New issue
Advanced search Search tips

Issue 872320 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Aug 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 1
Type: Bug

Blocking:
issue 868592



Sign in to add a comment

Null-dereference READ in content::RenderFrameImpl::CreateServiceWorkerProvider

Project Member Reported by ClusterFuzz, Aug 8

Issue description

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

Fuzzer: inferno_webbot
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  content::RenderFrameImpl::CreateServiceWorkerProvider
  content::RenderFrameImpl::CreateServiceWorkerProvider
  blink::LocalFrameClientImpl::CreateServiceWorkerProvider
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=581123:581124

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Aug 8

Components: Internals>Core
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 2 by ClusterFuzz, Aug 8

Labels: Test-Predator-Auto-Owner
Owner: kouhei@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/38894f66a9f1d3142746572da766de1b02f8e2ce (Flush microtask queue before commit).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.
Issue 872636 has been merged into this issue.
Cc: ligim...@chromium.org
Labels: RegressedIn-70 Target-70 ReleaseBlock-Beta FoundIn-70
Regressed during M70, stats below., #2 crash in latest canary- 70.0.3517.0.

70.0.3517.0	30.00%	36
70.0.3516.1	5.83%	7
70.0.3516.0	55.00%	66
Status: Started (was: Assigned)
Labels: OS-Windows
Blocking: 868592
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d995adf5879e71074e33983b0b038d21b3f3be43

commit d995adf5879e71074e33983b0b038d21b3f3be43
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Fri Aug 10 12:33:21 2018

NavigatorServiceWorker: Avoid instantiating if being navigated away.

This CL fixes a clusterfuzz crash which fails to minimize.

Bug:  872320 
Change-Id: Ied4ba2d6143573a4b66fc85fc4fc0fd3b2fbc0ec
Reviewed-on: https://chromium-review.googlesource.com/1170160
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582126}
[modify] https://crrev.com/d995adf5879e71074e33983b0b038d21b3f3be43/third_party/blink/renderer/modules/service_worker/navigator_service_worker.cc

Project Member

Comment 9 by ClusterFuzz, Aug 11

ClusterFuzz has detected this issue as fixed in range 582125:582126.

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

Fuzzer: inferno_webbot
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  content::RenderFrameImpl::CreateServiceWorkerProvider
  content::RenderFrameImpl::CreateServiceWorkerProvider
  blink::LocalFrameClientImpl::CreateServiceWorkerProvider
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=581123:581124
Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=582125:582126

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

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.
Project Member

Comment 10 by ClusterFuzz, Aug 11

Labels: ClusterFuzz-Verified
Status: Verified (was: Started)
ClusterFuzz testcase 6423077332975616 is verified as fixed, so closing issue as verified.

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

Comment 11 by bugdroid1@chromium.org, Aug 22

Labels: merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/222c9ba7c66822eb0f2e6294443da26ccf9b798e

commit 222c9ba7c66822eb0f2e6294443da26ccf9b798e
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Wed Aug 22 01:31:50 2018

Speculative fix for History::ScrollRestorationInternal null deref

This is a speculative fix for crash reported on crbug.com/872672 .

There is no guarantee that the DocumentLoader is always attached [1],
so let's introduce a null check.

[1] The DocumentLoader may be detached while FrameLoader::PrepareForCommit.

Bug: 872672
Change-Id: I015651506a891c3344f1bdbf40ea013ce988a95f
Reviewed-on: https://chromium-review.googlesource.com/1171972
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582509}
(cherry picked from commit a2a107d712d0fb754cc03ccb36630fa3ddc90f14)

NavigatorServiceWorker: Avoid instantiating if being navigated away.

This CL fixes a clusterfuzz crash which fails to minimize.

Bug:  872320 
Change-Id: Ied4ba2d6143573a4b66fc85fc4fc0fd3b2fbc0ec
Reviewed-on: https://chromium-review.googlesource.com/1170160
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582126}
(cherry picked from commit d995adf5879e71074e33983b0b038d21b3f3be43)

Flush microtask queue before commit

Bug:  868592 
Change-Id: Ia1d17f0b1d07d27a1665d6871545f051ee2eed87
Reviewed-on: https://chromium-review.googlesource.com/1164148
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581124}
(cherry picked from commit 38894f66a9f1d3142746572da766de1b02f8e2ce)

Prevent promise reject to be sync scheduled during DocumentLoader detach
(% mod: revert fetch_manager.cc change)

(cherry picked from commit c415acc1e0cd8ec75e43bcb596f7bd76321f72f8)

Bug:  868592 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I5cff4653a62c357e8eb9d5a82a11b8018653b712
Reviewed-on: https://chromium-review.googlesource.com/1163235
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#580814}
Reviewed-on: https://chromium-review.googlesource.com/1184122
Cr-Commit-Position: refs/branch-heads/3497@{#760}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/222c9ba7c66822eb0f2e6294443da26ccf9b798e/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees
[delete] https://crrev.com/b9e41b5b5522039b2d6bce08ae7ed6f375b0c54e/third_party/WebKit/LayoutTests/performance/performance-observer-crash-expected.txt
[delete] https://crrev.com/b9e41b5b5522039b2d6bce08ae7ed6f375b0c54e/third_party/WebKit/LayoutTests/performance/performance-observer-crash.html
[modify] https://crrev.com/222c9ba7c66822eb0f2e6294443da26ccf9b798e/third_party/blink/renderer/core/frame/history.cc
[modify] https://crrev.com/222c9ba7c66822eb0f2e6294443da26ccf9b798e/third_party/blink/renderer/core/loader/frame_loader.cc
[modify] https://crrev.com/222c9ba7c66822eb0f2e6294443da26ccf9b798e/third_party/blink/renderer/modules/service_worker/navigator_service_worker.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/53746ec1f57069de42beeeafb38d1ed2da56e148

commit 53746ec1f57069de42beeeafb38d1ed2da56e148
Author: Nasko Oskov <nasko@chromium.org>
Date: Fri Aug 31 21:27:07 2018

Revert "Speculative fix for History::ScrollRestorationInternal null deref"

This reverts commit 222c9ba7c66822eb0f2e6294443da26ccf9b798e.

Reason for revert: Causing crashes on Mac.

Original change's description:
> Speculative fix for History::ScrollRestorationInternal null deref
> 
> This is a speculative fix for crash reported on crbug.com/872672 .
> 
> There is no guarantee that the DocumentLoader is always attached [1],
> so let's introduce a null check.
> 
> [1] The DocumentLoader may be detached while FrameLoader::PrepareForCommit.
> 
> Bug: 872672
> Change-Id: I015651506a891c3344f1bdbf40ea013ce988a95f
> Reviewed-on: https://chromium-review.googlesource.com/1171972
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#582509}
> (cherry picked from commit a2a107d712d0fb754cc03ccb36630fa3ddc90f14)
> 
> NavigatorServiceWorker: Avoid instantiating if being navigated away.
> 
> This CL fixes a clusterfuzz crash which fails to minimize.
> 
> Bug:  872320 
> Change-Id: Ied4ba2d6143573a4b66fc85fc4fc0fd3b2fbc0ec
> Reviewed-on: https://chromium-review.googlesource.com/1170160
> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
> Reviewed-by: Matt Falkenhagen <falken@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#582126}
> (cherry picked from commit d995adf5879e71074e33983b0b038d21b3f3be43)
> 
> Flush microtask queue before commit
> 
> Bug:  868592 
> Change-Id: Ia1d17f0b1d07d27a1665d6871545f051ee2eed87
> Reviewed-on: https://chromium-review.googlesource.com/1164148
> Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
> Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#581124}
> (cherry picked from commit 38894f66a9f1d3142746572da766de1b02f8e2ce)
> 
> Prevent promise reject to be sync scheduled during DocumentLoader detach
> (% mod: revert fetch_manager.cc change)
> 
> (cherry picked from commit c415acc1e0cd8ec75e43bcb596f7bd76321f72f8)
> 
> Bug:  868592 
> Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
> Change-Id: I5cff4653a62c357e8eb9d5a82a11b8018653b712
> Reviewed-on: https://chromium-review.googlesource.com/1163235
> Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
> Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
> Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#580814}
> Reviewed-on: https://chromium-review.googlesource.com/1184122
> Cr-Commit-Position: refs/branch-heads/3497@{#760}
> Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}

TBR=yukishiino@chromium.org,yhirano@chromium.org,haraken@chromium.org,kouhei@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 872672,  872320 ,  868592 
Change-Id: I9f03a1b330dab1ccc68e123212b68bff336bf3ed
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1200371
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#862}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/53746ec1f57069de42beeeafb38d1ed2da56e148/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees
[add] https://crrev.com/53746ec1f57069de42beeeafb38d1ed2da56e148/third_party/WebKit/LayoutTests/performance/performance-observer-crash-expected.txt
[add] https://crrev.com/53746ec1f57069de42beeeafb38d1ed2da56e148/third_party/WebKit/LayoutTests/performance/performance-observer-crash.html
[modify] https://crrev.com/53746ec1f57069de42beeeafb38d1ed2da56e148/third_party/blink/renderer/core/frame/history.cc
[modify] https://crrev.com/53746ec1f57069de42beeeafb38d1ed2da56e148/third_party/blink/renderer/core/loader/frame_loader.cc
[modify] https://crrev.com/53746ec1f57069de42beeeafb38d1ed2da56e148/third_party/blink/renderer/modules/service_worker/navigator_service_worker.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f3c120ac4416a678e18ee28294c52cd517c147c9

commit f3c120ac4416a678e18ee28294c52cd517c147c9
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Fri Sep 14 17:42:50 2018

M69 Mega-patch for 868592 fix

This CL is a collection of cherry-picks related to  crbug.com/868592  fix.
Specifically, this is:
- The original mega-patch crrev.com/222c9ba7c6
- creis@ follow-up fix crrev.com/27986c7c955
- kouhei@ follow-up fix crrev.com/6be8b5a07bdf

The original change descriptions are captured below % Change-Id lines

---

Speculative crash fix for navigator.serviceworker access during unload

This should fix crash/caab6eb137e58385

This CL addresses the unhandled case in crrev.com/582126

TBR=falken@chromium.org

Bug: 881126,  868592 
Reviewed-on: https://chromium-review.googlesource.com/1207781
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#589419}(cherry picked from commit 6be8b5a07bdfa95c37e2da9cace7d7d4b69b31b5)
Reviewed-on: https://chromium-review.googlesource.com/1212368
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/branch-heads/3545@{#2}
Cr-Branched-From: a2bbe9dedf867fccce6d8073dc8e9c864c662bfe-refs/heads/master@{#589377}

Speculative fix for additional History DocumentLoader crashes.

There is no guarantee that the DocumentLoader is always attached [1],
so let's introduce null checks in StateInternal and setScrollRestoration.

[1] The DocumentLoader may be detached while FrameLoader::PrepareForCommit.

BUG=879477, 872672

Reviewed-on: https://chromium-review.googlesource.com/1200075
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588227}
Speculative fix for History::ScrollRestorationInternal null deref

This is a speculative fix for crash reported on crbug.com/872672 .

There is no guarantee that the DocumentLoader is always attached [1],
so let's introduce a null check.

[1] The DocumentLoader may be detached while FrameLoader::PrepareForCommit.

Bug: 872672
Reviewed-on: https://chromium-review.googlesource.com/1171972
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582509}
(cherry picked from commit a2a107d712d0fb754cc03ccb36630fa3ddc90f14)

NavigatorServiceWorker: Avoid instantiating if being navigated away.

This CL fixes a clusterfuzz crash which fails to minimize.

Bug:  872320 
Reviewed-on: https://chromium-review.googlesource.com/1170160
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582126}
(cherry picked from commit d995adf5879e71074e33983b0b038d21b3f3be43)

Flush microtask queue before commit

Bug:  868592 
Reviewed-on: https://chromium-review.googlesource.com/1164148
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581124}
(cherry picked from commit 38894f66a9f1d3142746572da766de1b02f8e2ce)

Prevent promise reject to be sync scheduled during DocumentLoader detach
(% mod: revert fetch_manager.cc change)

(cherry picked from commit c415acc1e0cd8ec75e43bcb596f7bd76321f72f8)

Bug:  868592 
Change-Id: I50029416f0441a9f09c538716684a01cb8af93e1
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1163235
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#580814}
Reviewed-on: https://chromium-review.googlesource.com/1184122
Cr-Original-Commit-Position: refs/branch-heads/3497@{#760}
Cr-Original-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
Reviewed-on: https://chromium-review.googlesource.com/1218183
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#938}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/f3c120ac4416a678e18ee28294c52cd517c147c9/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/f3c120ac4416a678e18ee28294c52cd517c147c9/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees
[delete] https://crrev.com/47b99a368e694ae38346630ce4c621622de4b8b0/third_party/WebKit/LayoutTests/performance/performance-observer-crash-expected.txt
[delete] https://crrev.com/47b99a368e694ae38346630ce4c621622de4b8b0/third_party/WebKit/LayoutTests/performance/performance-observer-crash.html
[modify] https://crrev.com/f3c120ac4416a678e18ee28294c52cd517c147c9/third_party/blink/renderer/core/frame/history.cc
[modify] https://crrev.com/f3c120ac4416a678e18ee28294c52cd517c147c9/third_party/blink/renderer/core/loader/frame_loader.cc
[modify] https://crrev.com/f3c120ac4416a678e18ee28294c52cd517c147c9/third_party/blink/renderer/modules/service_worker/navigator_service_worker.cc

Sign in to add a comment