Null-dereference READ in v8::Context::Global |
||||||||||||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=4823416929779712 Fuzzer: inferno_layout_test_unmodified Job Type: windows_asan_chrome_no_sandbox Platform Id: windows Crash Type: Null-dereference READ Crash Address: 0x00000000 Crash State: v8::Context::Global blink::ToExecutionContext blink::ExecutionContext::From Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=483281:483366 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4823416929779712 Issue filed automatically. See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
,
Jul 3 2017
,
Jul 14 2017
The result is a list of CLs that change the crashed files. Author: Yuki Shiino Project: chromium Changelist: https://chromium.googlesource.com/chromium/src/+/a5418786187f6af3327c9342845db1a32adfbbaf Time: Wed Jun 28 10:22:26 2017 Lines 722-739 of file V8BindingForCore.cpp which potentially caused crash are changed in this cl (frame #1, "blink::ToExecutionContext"). Minimum distance from crash line to modified line: 0. (file: V8BindingForCore.cpp, crashed on: 722, modified: 722). @yukishiino -- Could you please look into the issue, kindly re-assign if this is not related to your changes. Thank You
,
Jul 16 2017
,
Jul 20 2017
Issue 739061 has been merged into this issue.
,
Jul 20 2017
I'm now working on https://chromium-review.googlesource.com/c/574509 as a fix.
,
Jul 20 2017
Users experienced this crash on the following builds: Win Dev 61.0.3153.0 - 0.55 CPM, 96 reports, 95 clients (signature v8::Context::Global) Mac Dev 61.0.3153.4 - 0.76 CPM, 9 reports, 5 clients (signature v8::Context::Global) If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates. - Go/Fracas
,
Jul 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/659015bc5eb341e9d3ccce40ba0293079f2c2e31 commit 659015bc5eb341e9d3ccce40ba0293079f2c2e31 Author: Yuki Shiino <yukishiino@chromium.org> Date: Fri Jul 21 08:08:30 2017 v8binding: Fixes MutationCallback not to access the empty context. MutationCallback holds a ScriptState and accesses it assynchronously. So, MutationCallback should check the context if it's alive before the uses. Bug: 738299 Change-Id: Ib16b37b2e90e5ba87117d7c6e1218f9df6d69a41 Reviewed-on: https://chromium-review.googlesource.com/574509 Commit-Queue: Yuki Shiino <yukishiino@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#488611} [modify] https://crrev.com/659015bc5eb341e9d3ccce40ba0293079f2c2e31/third_party/WebKit/Source/bindings/core/v8/V8MutationCallback.h
,
Jul 25 2017
,
Jul 25 2017
yukishiino@ can you please request CL from comment#9 to get merged to M61(Branch 3163). So far there are just 4 crashes on latest Chrome Canary. Please find details from here : https://goto.google.com/unhcx
,
Jul 26 2017
,
Jul 27 2017
Thank pbommana@, it turned out that we need another fix. Working at: https://chromium-review.googlesource.com/c/589047/ We may need to merge both of CLs (already submitted one and another one linked above).
,
Jul 27 2017
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9d2a22bba0533f6607120816ddd88f1cb00858ae commit 9d2a22bba0533f6607120816ddd88f1cb00858ae Author: Yuki Shiino <yukishiino@chromium.org> Date: Thu Jul 27 14:17:42 2017 v8binding: Fixes MutationCallback not to access the empty context. MutationCallback holds a ScriptState and accesses it assynchronously. So, MutationCallback should check the context if it's alive before the uses. TBR=yukishiino@chromium.org (cherry picked from commit 659015bc5eb341e9d3ccce40ba0293079f2c2e31) Bug: 738299 Change-Id: Ib16b37b2e90e5ba87117d7c6e1218f9df6d69a41 Reviewed-on: https://chromium-review.googlesource.com/574509 Commit-Queue: Yuki Shiino <yukishiino@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#488611} Reviewed-on: https://chromium-review.googlesource.com/589468 Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#78} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/9d2a22bba0533f6607120816ddd88f1cb00858ae/third_party/WebKit/Source/bindings/core/v8/V8MutationCallback.h
,
Jul 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4e112e84ae592c82aef95065fa4d5def8ecea3bb commit 4e112e84ae592c82aef95065fa4d5def8ecea3bb Author: Yuki Shiino <yukishiino@chromium.org> Date: Thu Jul 27 17:23:06 2017 v8binding: Fixes a misuse of ScriptState at V8MutationCallback. V8MutationCallback is using a ScriptState without confirming that the ScriptState is valid or not. This CL fixes it to check it before the actual use. callback_.IsEmpty() and script_state_->ContextIdValid() are moved to be the same order as callback_function.cpp.tmpl. V8MutationCallback will be replaced with the generated version in near future (hopefully). This CL aims to fix a crash issue in a short term and will be merged to M61. Bug: 738299 Change-Id: I7a17203ad13a86d62a327a48cfffaca223ca5d8f Reviewed-on: https://chromium-review.googlesource.com/589047 Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Yuki Shiino <yukishiino@chromium.org> Cr-Commit-Position: refs/heads/master@{#490004} [modify] https://crrev.com/4e112e84ae592c82aef95065fa4d5def8ecea3bb/third_party/WebKit/Source/bindings/core/v8/V8MutationCallback.cpp
,
Jul 28 2017
May I request another merge (of CL of #c16) to M61?
,
Jul 28 2017
Before we approve merge to M61 for CL listed at #16, could you please confirm change is well baked/verified in Canary, having enough automation tests coverage and will be a safe merge to M61?
,
Jul 28 2017
Yes, will do. I should have done it before asking. Thanks. :)
,
Jul 28 2017
ClusterFuzz has detected this issue as fixed in range 489985:490017. Detailed report: https://clusterfuzz.com/testcase?key=4823416929779712 Fuzzer: inferno_layout_test_unmodified Job Type: windows_asan_chrome_no_sandbox Platform Id: windows Crash Type: Null-dereference READ Crash Address: 0x00000000 Crash State: v8::Context::Global blink::ToExecutionContext blink::ExecutionContext::From Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=483281:483366 Fixed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=489985:490017 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4823416929779712 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.
,
Jul 28 2017
ClusterFuzz testcase 4823416929779712 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Jul 29 2017
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 30 2017
Pls merge you change to M61 branch 3163 before 3:00 PM PT on Monday so we can take it in for next week last M61 Dev release. Thank you.
,
Jul 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/57cba57f14ec3c80d85ac8630b676846696319f9 commit 57cba57f14ec3c80d85ac8630b676846696319f9 Author: Yuki Shiino <yukishiino@chromium.org> Date: Mon Jul 31 06:26:07 2017 v8binding: Fixes a misuse of ScriptState at V8MutationCallback. V8MutationCallback is using a ScriptState without confirming that the ScriptState is valid or not. This CL fixes it to check it before the actual use. callback_.IsEmpty() and script_state_->ContextIdValid() are moved to be the same order as callback_function.cpp.tmpl. V8MutationCallback will be replaced with the generated version in near future (hopefully). This CL aims to fix a crash issue in a short term and will be merged to M61. TBR=yukishiino@chromium.org (cherry picked from commit 4e112e84ae592c82aef95065fa4d5def8ecea3bb) Bug: 738299 Change-Id: I7a17203ad13a86d62a327a48cfffaca223ca5d8f Reviewed-on: https://chromium-review.googlesource.com/589047 Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Yuki Shiino <yukishiino@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#490004} Reviewed-on: https://chromium-review.googlesource.com/593408 Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#143} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/57cba57f14ec3c80d85ac8630b676846696319f9/third_party/WebKit/Source/bindings/core/v8/V8MutationCallback.cpp |
||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||
Comment 1 by ClusterFuzz
, Jun 30 2017