New issue
Advanced search Search tips

Issue 797298 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Security



Sign in to add a comment

Heap-use-after-free in blink::PaintLayerScrollableArea::UpdateScrollOffset

Project Member Reported by ClusterFuzz, Dec 22 2017

Issue description

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

Fuzzer: inferno_twister
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x60f00000ef60
Crash State:
  blink::PaintLayerScrollableArea::UpdateScrollOffset
  blink::ScrollableArea::ScrollOffsetChanged
  blink::ProgrammaticScrollAnimator::ScrollToOffsetWithoutAnimation
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=434178:434255

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Dec 22 2017

Components: Blink>Paint Platform
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, Dec 22 2017

Labels: Test-Predator-Auto-Owner
Owner: flackr@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/79b54819da38f8047e234a647e21998e09b75e5f (Reland "Only promote opaque scrollers which are stacking contexts.").

If this is incorrect, please remove the owner and apply the Test-Predator-Wrong-CLs label.
Project Member

Comment 3 by sheriffbot@chromium.org, Dec 23 2017

Labels: M-63
Project Member

Comment 4 by sheriffbot@chromium.org, Dec 23 2017

Labels: Pri-1
Components: -Blink>Paint -Platform Blink>Compositing
Project Member

Comment 6 by sheriffbot@chromium.org, Jan 6 2018

flackr: 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
Project Member

Comment 7 by sheriffbot@chromium.org, Jan 20 2018

flackr: Uh oh! This issue still open and hasn't been updated in the last 28 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

Comment 8 by flackr@chromium.org, Jan 21 2018

Status: Started (was: Assigned)
This looks like it may have been around for a long time, I'll see if I can find the leaked memory.
Project Member

Comment 10 by bugdroid1@chromium.org, Jan 23 2018

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

commit 3a5a63076cf74c07b7cd0b167ecb6d92fe67e67c
Author: Robert Flack <flackr@chromium.org>
Date: Tue Jan 23 07:00:02 2018

Detect when scrollable area is removed during scroll update.

The layout object for a scrollable area can be detached as a result of running
the accessibility scroll update notification handler. This results in the
layer being deleted but the scrollable area is not immediately deleted as it
is garbage collected so we need to detect that we can no longer access the
layout object or paint layer.

BUG= 797298 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ib31df5f6da71ed5f020a5cc65a94f7678ea256ee
Reviewed-on: https://chromium-review.googlesource.com/878063
Commit-Queue: Robert Flack <flackr@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531171}
[add] https://crrev.com/3a5a63076cf74c07b7cd0b167ecb6d92fe67e67c/third_party/WebKit/LayoutTests/fast/layers/scrollable-area-removed-on-scroll-crash-expected.txt
[add] https://crrev.com/3a5a63076cf74c07b7cd0b167ecb6d92fe67e67c/third_party/WebKit/LayoutTests/fast/layers/scrollable-area-removed-on-scroll-crash.html
[modify] https://crrev.com/3a5a63076cf74c07b7cd0b167ecb6d92fe67e67c/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
[modify] https://crrev.com/3a5a63076cf74c07b7cd0b167ecb6d92fe67e67c/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
[modify] https://crrev.com/3a5a63076cf74c07b7cd0b167ecb6d92fe67e67c/third_party/WebKit/Source/platform/scroll/ScrollableArea.cpp
[modify] https://crrev.com/3a5a63076cf74c07b7cd0b167ecb6d92fe67e67c/third_party/WebKit/Source/platform/scroll/ScrollableArea.h

Project Member

Comment 11 by ClusterFuzz, Jan 23 2018

ClusterFuzz has detected this issue as fixed in range 531170:531171.

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

Fuzzer: inferno_twister
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x60f00000ef60
Crash State:
  blink::PaintLayerScrollableArea::UpdateScrollOffset
  blink::ScrollableArea::ScrollOffsetChanged
  blink::ProgrammaticScrollAnimator::ScrollToOffsetWithoutAnimation
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=434178:434255
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=531170:531171

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

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 12 by ClusterFuzz, Jan 23 2018

Labels: ClusterFuzz-Verified
Status: Verified (was: Started)
ClusterFuzz testcase 6276543369445376 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 13 by bugdroid1@chromium.org, Jan 23 2018

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

commit e7a9977f4d7072cd809992208d875a8ecd6ef5e8
Author: Ramin Halavati <rhalavati@chromium.org>
Date: Tue Jan 23 12:14:37 2018

Revert "Detect when scrollable area is removed during scroll update."

This reverts commit 3a5a63076cf74c07b7cd0b167ecb6d92fe67e67c.

Reason for revert: There are flaky tests in site_per_process_webkit_layout_tests 
Please check out  crbug.com/804813 .

Original change's description:
> Detect when scrollable area is removed during scroll update.
> 
> The layout object for a scrollable area can be detached as a result of running
> the accessibility scroll update notification handler. This results in the
> layer being deleted but the scrollable area is not immediately deleted as it
> is garbage collected so we need to detect that we can no longer access the
> layout object or paint layer.
> 
> BUG= 797298 
> 
> Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Change-Id: Ib31df5f6da71ed5f020a5cc65a94f7678ea256ee
> Reviewed-on: https://chromium-review.googlesource.com/878063
> Commit-Queue: Robert Flack <flackr@chromium.org>
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#531171}

TBR=flackr@chromium.org,chrishtr@chromium.org

Change-Id: Ic0ef59659dfecea0c246f7c8a9407ec4524e328d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  797298 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/880902
Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531212}
[delete] https://crrev.com/4b4f64d7c3f27dfa2ae19a7fbbd74f35293871f7/third_party/WebKit/LayoutTests/fast/layers/scrollable-area-removed-on-scroll-crash-expected.txt
[delete] https://crrev.com/4b4f64d7c3f27dfa2ae19a7fbbd74f35293871f7/third_party/WebKit/LayoutTests/fast/layers/scrollable-area-removed-on-scroll-crash.html
[modify] https://crrev.com/e7a9977f4d7072cd809992208d875a8ecd6ef5e8/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
[modify] https://crrev.com/e7a9977f4d7072cd809992208d875a8ecd6ef5e8/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
[modify] https://crrev.com/e7a9977f4d7072cd809992208d875a8ecd6ef5e8/third_party/WebKit/Source/platform/scroll/ScrollableArea.cpp
[modify] https://crrev.com/e7a9977f4d7072cd809992208d875a8ecd6ef5e8/third_party/WebKit/Source/platform/scroll/ScrollableArea.h

Status: Assigned (was: Verified)
Please reland your patch with any fixes/flake fixes needed.
Project Member

Comment 15 by sheriffbot@chromium.org, Feb 8 2018

Labels: -M-63 M-64
 Issue 810680  has been merged into this issue.
Cc: flackr@chromium.org schenney@chromium.org dmazz...@chromium.org
 Issue 805324  has been merged into this issue.
Oops, I should have posted a link in the bug. I have a reland up for review at:
https://chromium-review.googlesource.com/c/chromium/src/+/887908

However, the specific cause of this use-after-free was resolved by https://chromium-review.googlesource.com/902775. I still think we should reland changing to a PaintLayer pointer to catch other potential use after free cases but this bug should still be resolved.
Cc: -flackr@chromium.org
So, both this one and  bug 810680  are fixed. However,  bug 805324  is still reproducing, see testcase in https://clusterfuzz.com/v2/testcase-detail/6292372572078080. Lets see if your reland fixes it.

Project Member

Comment 20 by bugdroid1@chromium.org, Feb 15 2018

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

commit 3ab971569afab25c6e330b3898fba29288b68732
Author: Robert Flack <flackr@chromium.org>
Date: Thu Feb 15 16:58:01 2018

Use PaintLayer pointer from PaintLayerScrollableArea as PaintLayer is destructed first.

This is a partial reland of e7a9977f4d7072cd809992208d875a8ecd6ef5e8 as flakiness observed on
 https://crbug.com/804813  was determined to be likely unrelated.

The accessibility lifetime issue was partially resolved by https://chromium-review.googlesource.com/902775 but there are still accesses after destruction from ScrollableArea and the potential for others to a deleted / freed PaintLayer. Using a pointer which we explicitly null out on dispose ensures that we will crash instead of using freed memory if other accesses occur.

Bug:  797298 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ia4bce7393a1f77a44fa76614305f2992485f3e88
Reviewed-on: https://chromium-review.googlesource.com/887908
Commit-Queue: Robert Flack <flackr@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537049}
[add] https://crrev.com/3ab971569afab25c6e330b3898fba29288b68732/third_party/WebKit/LayoutTests/fast/layers/scrollable-area-removed-on-scroll-crash-expected.txt
[add] https://crrev.com/3ab971569afab25c6e330b3898fba29288b68732/third_party/WebKit/LayoutTests/fast/layers/scrollable-area-removed-on-scroll-crash.html
[modify] https://crrev.com/3ab971569afab25c6e330b3898fba29288b68732/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
[modify] https://crrev.com/3ab971569afab25c6e330b3898fba29288b68732/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
[modify] https://crrev.com/3ab971569afab25c6e330b3898fba29288b68732/third_party/WebKit/Source/platform/scroll/ScrollableArea.cpp
[modify] https://crrev.com/3ab971569afab25c6e330b3898fba29288b68732/third_party/WebKit/Source/platform/scroll/ScrollableArea.h

Project Member

Comment 21 by bugdroid1@chromium.org, Feb 16 2018

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

commit 14fb4bf71bc1d4e0693b588724c8d6f8ed2d6207
Author: Robert Flack <flackr@chromium.org>
Date: Fri Feb 16 16:35:34 2018

Fix flakiness in fast/layers/scrollable-area-removed-on-scroll-crash.html

There are multiple accessibility notifications but the test should only
continue when it receives the one for the programmatic scroll.

This patch also fixes another use after free from trying to use the smooth
scroll coordinator.

Bug:  797298 , 812702 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I1340d8cd1d5d3e53e75d9ab0c5f50b523041b187
Reviewed-on: https://chromium-review.googlesource.com/922685
Commit-Queue: Robert Flack <flackr@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537317}
[modify] https://crrev.com/14fb4bf71bc1d4e0693b588724c8d6f8ed2d6207/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/14fb4bf71bc1d4e0693b588724c8d6f8ed2d6207/third_party/WebKit/LayoutTests/fast/layers/scrollable-area-removed-on-scroll-crash.html
[modify] https://crrev.com/14fb4bf71bc1d4e0693b588724c8d6f8ed2d6207/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp

Status: Fixed (was: Assigned)
https://bugs.chromium.org/p/chromium/issues/detail?id=805324 is now fixed, Closing.
Project Member

Comment 23 by sheriffbot@chromium.org, Feb 17 2018

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -M-64 M-66
Project Member

Comment 25 by sheriffbot@chromium.org, Mar 16 2018

Labels: Merge-Request-66
Project Member

Comment 26 by sheriffbot@chromium.org, Mar 16 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-66 Merge-Rejected-66
Since CLs landed on Feb 16th, this is already in M66. Branch for M66 was on March 1st. Unless there is something else to merge?
Labels: Release-0-M66
Project Member

Comment 29 by sheriffbot@chromium.org, May 26 2018

Labels: -Restrict-View-SecurityNotify allpublic
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

Sign in to add a comment