This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

+awhalley@, could you ptal and triage?

To V8 clusterfuzz sheriff.

+some other sheriffs, given the holiday week in the us

M63 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge  into the release branch ASAP. Thank you.

Plan is to cut M63 Stable RC on this Friday (12/01) @ 5:00 PM PT. Please plan to have fix landed/merged to M63 before then. Thank you.

Any update here as we're getting closer to M63 stable RC cut?

Pushing to M64 as we're out of time for 63. Sad to see the regression make stable :(

I reproduced the problem, still debugging.

Bisected to f0acede9bb05155c25ee87e81b4b587e8a76f690. According to CF it still reproduces on TOT.
The problem is that Array.prototype.join (a JS builtin) lost it's feedback vector at some point and therefore we crash when this builtin is called.

jarin: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Michael, could you have a look? I think you have the best experience in hunting rogue feedback vectors.

Cobbled together a d8 repro with mstarzinger@:

---

function f() {
  function g(arg) { return arg; }
  // The closure contains a call IC slot.
  return function() { return g(42); };
}

const a = Realm.create();
const b = Realm.create();

// Create two closures in different contexts sharing the same
// SharedFunctionInfo.
const x = Realm.eval(a, f.toString() + " f()");
const y = Realm.eval(b, f.toString() + " f()");

// Run the first closure to create SFI::code.
x();

// At this point, SFI::code is set and `x` has a feedback vector (`y` has not).

// Enabling block code coverage deoptimizes all functions and triggers the
// buggy code path in which we'd unconditionally replace JSFunction::code with
// its SFI::code (but skip feedback vector setup).
%DebugToggleBlockCoverage(true);

// Still no feedback vector set on `y` but it now contains code. Run it to
// trigger the crash when attempting to write into the non-existent feedback
// vector.
y();

---

Inline comments should explain the issue. The problem is that [0] unconditionally replaces all JSFunction's code objects with the associated SFI::code object when enabling code coverage. That can result in a situation where we execute a JSFunction without ever initializing its feedback vector.

Got a fix in-flight.


[0] https://crrev.com/f0acede9bb05155c25ee87e81b4b587e8a76f690

Note that this can only happen when code coverage or type profiling (both debugging features in DevTools) are turned on explicitly by the user.

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/8303dc531b54bdb1ee979111d87715698ef20382

commit 8303dc531b54bdb1ee979111d87715698ef20382
Author: jgruber <jgruber@chromium.org>
Date: Thu Dec 07 13:48:31 2017

[coverage] Do not reset JSFunction::code post-deoptimization

When enabling any coverage mode (other than best-effort), we trigger
deoptimization of all functions on the heap.

Prior to the recent removal of the weak list of optimized functions [0],
we'd unlink optimized code from all relevant JSFunctions during the call
to DeoptimizeAll.

After the weak-list-removal, this was no longer the case, hence this [1]
change which attempts to reset the code object from the
SharedFunctionInfo for all found JSFunction objects.

But this can create a situation in which JSFunctions are set up
incorrectly s.t. they have unoptimized code but no feedback vector.

This CL fixes that by leaving JSFunction objects untouched and relying
on self-healing mechanisms (CompileLazyDeoptimizedCode) to fix up
JSFunction::code.

[0] https://crrev.com/f0acede9bb05155c25ee87e81b4b587e8a76f690
[1] https://crrev.com/c/647596/5/src/debug/debug-coverage.cc

Bug:  chromium:786784 , chromium:791940,  v8:6637 
Change-Id: I13191f4c8800a0d72894b959105189dc09ca693e
Reviewed-on: https://chromium-review.googlesource.com/813615
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49932}
[modify] https://crrev.com/8303dc531b54bdb1ee979111d87715698ef20382/src/isolate.cc
[add] https://crrev.com/8303dc531b54bdb1ee979111d87715698ef20382/test/mjsunit/regress/regress-786784.js

Thanks for taking this bug Jakob, I appreciate it! :)

ClusterFuzz has detected this issue as fixed in range 522491:522544.

Detailed report: https://clusterfuzz.com/testcase?key=5455700821278720

Fuzzer: inferno_twister
Job Type: mac_asan_content_shell
Platform Id: mac

Crash Type: UNKNOWN READ
Crash Address: 0x000500000007
Crash State:
  v8::internal::Invoke
  v8::internal::Execution::Call
  v8::Function::Call
  
Sanitizer: address (ASAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=mac_asan_content_shell&range=499408:499633
Fixed: https://clusterfuzz.com/revisions?job=mac_asan_content_shell&range=522491:522544

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5455700821278720

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 5455700821278720 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/918b7fba95d8890fab08cf4a6a622f1c1f9df8cf

commit 918b7fba95d8890fab08cf4a6a622f1c1f9df8cf
Author: jgruber <jgruber@chromium.org>
Date: Mon Dec 11 11:48:30 2017

Merged: [coverage] Do not reset JSFunction::code post-deoptimization

When enabling any coverage mode (other than best-effort), we trigger
deoptimization of all functions on the heap.

Prior to the recent removal of the weak list of optimized functions [0],
we'd unlink optimized code from all relevant JSFunctions during the call
to DeoptimizeAll.

After the weak-list-removal, this was no longer the case, hence this [1]
change which attempts to reset the code object from the
SharedFunctionInfo for all found JSFunction objects.

But this can create a situation in which JSFunctions are set up
incorrectly s.t. they have unoptimized code but no feedback vector.

This CL fixes that by leaving JSFunction objects untouched and relying
on self-healing mechanisms (CompileLazyDeoptimizedCode) to fix up
JSFunction::code.

[0] https://crrev.com/f0acede9bb05155c25ee87e81b4b587e8a76f690
[1] https://crrev.com/c/647596/5/src/debug/debug-coverage.cc

NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
TBR=mstarzinger@chromium.org

Bug:  chromium:786784 , chromium:791940,  v8:6637 
Change-Id: I13191f4c8800a0d72894b959105189dc09ca693e
Reviewed-on: https://chromium-review.googlesource.com/813615
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#49932}(cherry picked from commit 8303dc531b54bdb1ee979111d87715698ef20382)
Reviewed-on: https://chromium-review.googlesource.com/818346
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.4@{#8}
Cr-Branched-From: 0407506af3d9d7e2718be1d8759296165b218fcf-refs/heads/6.4.388@{#1}
Cr-Branched-From: a5fc4e085ee543cb608eb11034bc8f147ba388e1-refs/heads/master@{#49724}
[modify] https://crrev.com/918b7fba95d8890fab08cf4a6a622f1c1f9df8cf/src/isolate.cc
[add] https://crrev.com/918b7fba95d8890fab08cf4a6a622f1c1f9df8cf/test/mjsunit/regress/regress-786784.js

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/36f26478b473d2a98b81c5c95ce2d6c568f0c8e0

commit 36f26478b473d2a98b81c5c95ce2d6c568f0c8e0
Author: jgruber <jgruber@chromium.org>
Date: Wed Dec 13 09:02:48 2017

[coverage] Do not reset JSFunction::code post-deoptimization

When enabling any coverage mode (other than best-effort), we trigger
deoptimization of all functions on the heap.

Prior to the recent removal of the weak list of optimized functions [0],
we'd unlink optimized code from all relevant JSFunctions during the call
to DeoptimizeAll.

After the weak-list-removal, this was no longer the case, hence this [1]
change which attempts to reset the code object from the
SharedFunctionInfo for all found JSFunction objects.

But this can create a situation in which JSFunctions are set up
incorrectly s.t. they have unoptimized code but no feedback vector.

This CL fixes that by leaving JSFunction objects untouched and relying
on self-healing mechanisms (CompileLazyDeoptimizedCode) to fix up
JSFunction::code.

[0] https://crrev.com/f0acede9bb05155c25ee87e81b4b587e8a76f690
[1] https://crrev.com/c/647596/5/src/debug/debug-coverage.cc

NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
TBR=mstarzinger@chromium.org

Bug:  chromium:786784 , chromium:791940,  v8:6637 
Change-Id: I13191f4c8800a0d72894b959105189dc09ca693e
Reviewed-on: https://chromium-review.googlesource.com/813615
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#49932}
Reviewed-on: https://chromium-review.googlesource.com/822616
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.3@{#103}
Cr-Branched-From: 094a7c93dcdcd921de3883ba4674b7e1a0feffbe-refs/heads/6.3.292@{#1}
Cr-Branched-From: 18b8fbb528a8021e04a029e06eafee50b918bce0-refs/heads/master@{#48432}
[modify] https://crrev.com/36f26478b473d2a98b81c5c95ce2d6c568f0c8e0/src/debug/debug-coverage.cc
[add] https://crrev.com/36f26478b473d2a98b81c5c95ce2d6c568f0c8e0/test/mjsunit/regress/regress-786784.js

 Issue 782546  has been merged into this issue.

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot