Issue metadata
Sign in to add a comment
|
Heap-use-after-free in blink::PaintController::FinishCycle |
||||||||||||||||||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=5102865228759040 Fuzzer: bj_broddelwerk Job Type: windows_asan_chrome_no_sandbox Platform Id: windows Crash Type: Heap-use-after-free READ 8 Crash Address: 0x122ff317e000 Crash State: blink::PaintController::FinishCycle blink::GraphicsLayer::PaintRecursively blink::LocalFrameView::PaintTree Sanitizer: address (ASAN) Recommended Security Severity: High Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=571248:571253 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5102865228759040 Issue filed automatically. See https://github.com/google/clusterfuzz-tools for more information.
,
Jun 30 2018
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/f0498e2265442349ed2b837c09631fb3b9eb237c ([CI] Remove cache generation mechanism of DisplayItemClient). 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.
,
Jun 30 2018
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
,
Jun 30 2018
,
Jun 30 2018
The root cause is mismatching hierarchies of PaintLayer and compositing.
The DOM tree is like:
<svg style="backface-visibility: hidden">
<foreignObject>
some inline boxes
</foreignObject>
</svg>
In the PaintLayer tree, the layers for the svg root and the foreign object are siblings:
layer for HTML
normal flow list
layer for the svg
positive z-order list
layer for the foreignObject
while in the GraphicsLayer tree, the foreignObject is painted on the GraphicsLayer of the svg.
When the contents of the foreignObject changes, we mark the painting layer ancestors for NeedsRepaint, and miss the svg which is not in the painting layer ancestor path. In the next paint, the svg's GraphicsLayer is repainted. Then during FinishCycle() when we iterate the DisplayItemClients we access some deleted DisplayItemClients under the foreignObject.
I can change GraphicsLayer to call FinishCycle() for real repainted layers only to avoid the heap-use-after-free for the test case. That won't fix the under-invalidation though.
chrishtr@ can you look into the under-invalidation issue?
,
Jul 1
,
Jul 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e71a3846080ff6cc758f39c2fea5c5a903572e61 commit e71a3846080ff6cc758f39c2fea5c5a903572e61 Author: Xianzhu Wang <wangxianzhu@chromium.org> Date: Mon Jul 02 01:16:45 2018 [PE] Don't call PaintController::FinishCycle() if not repainted This is mainly a performance optimization because we don't need FinishCycle() if PaintController has nothing changed. By the way it can also avoid the immediate bad effect (but not the root cause) of bug 859294 . Bug: 859294 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Iad3e14ee303c08185922d3d6e495c3b25ead5434 Reviewed-on: https://chromium-review.googlesource.com/1121783 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Commit-Position: refs/heads/master@{#571803} [modify] https://crrev.com/e71a3846080ff6cc758f39c2fea5c5a903572e61/third_party/blink/renderer/core/frame/local_frame_view.cc [modify] https://crrev.com/e71a3846080ff6cc758f39c2fea5c5a903572e61/third_party/blink/renderer/core/frame/local_frame_view.h [modify] https://crrev.com/e71a3846080ff6cc758f39c2fea5c5a903572e61/third_party/blink/renderer/platform/graphics/graphics_layer.cc
,
Jul 2
ClusterFuzz has detected this issue as fixed in range 571802:571803. Detailed report: https://clusterfuzz.com/testcase?key=5102865228759040 Fuzzer: bj_broddelwerk Job Type: windows_asan_chrome_no_sandbox Platform Id: windows Crash Type: Heap-use-after-free READ 8 Crash Address: 0x122ff317e000 Crash State: blink::PaintController::FinishCycle blink::GraphicsLayer::PaintRecursively blink::LocalFrameView::PaintTree Sanitizer: address (ASAN) Recommended Security Severity: High Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=571248:571253 Fixed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=571802:571803 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5102865228759040 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 2
ClusterFuzz testcase 5102865228759040 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Jul 2
,
Jul 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/74780299ea1b72a9f8b6415943213c0cf0f7ee23 commit 74780299ea1b72a9f8b6415943213c0cf0f7ee23 Author: Xianzhu Wang <wangxianzhu@chromium.org> Date: Tue Jul 03 18:40:12 2018 Revert "[PE] Don't call PaintController::FinishCycle() if not repainted" This reverts commit e71a3846080ff6cc758f39c2fea5c5a903572e61. Reason for revert: Caused performance regressions because changed paint properties are not cleared changed flags. Original change's description: > [PE] Don't call PaintController::FinishCycle() if not repainted > > This is mainly a performance optimization because we don't need > FinishCycle() if PaintController has nothing changed. > > By the way it can also avoid the immediate bad effect (but not the > root cause) of bug 859294 . > > Bug: 859294 > Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel > Change-Id: Iad3e14ee303c08185922d3d6e495c3b25ead5434 > Reviewed-on: https://chromium-review.googlesource.com/1121783 > Reviewed-by: Chris Harrelson <chrishtr@chromium.org> > Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> > Cr-Commit-Position: refs/heads/master@{#571803} TBR=wangxianzhu@chromium.org,chrishtr@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 859294 , 859885 Change-Id: I2a4d2a7d09cc3bf5edfbf0c6dee275581030c2c9 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/1124701 Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Commit-Position: refs/heads/master@{#572302} [modify] https://crrev.com/74780299ea1b72a9f8b6415943213c0cf0f7ee23/third_party/blink/renderer/core/frame/local_frame_view.cc [modify] https://crrev.com/74780299ea1b72a9f8b6415943213c0cf0f7ee23/third_party/blink/renderer/core/frame/local_frame_view.h [modify] https://crrev.com/74780299ea1b72a9f8b6415943213c0cf0f7ee23/third_party/blink/renderer/platform/graphics/graphics_layer.cc
,
Jul 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d7209bb8a53bcc9636b2c12f18e7f7b74b0c02c8 commit d7209bb8a53bcc9636b2c12f18e7f7b74b0c02c8 Author: Xianzhu Wang <wangxianzhu@chromium.org> Date: Tue Jul 03 22:34:13 2018 [PE] Don't validate DisplayItemClients if PaintController changed nothing This is mainly a performance optimization because we don't need FinishCycle() if PaintController has nothing changed. By the way it can also avoid the immediate bad effect (but not the root cause) of bug 859294 . Bug: 859294 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I3a4d26253fef2b04adc805ef94b93c5479c88759 Reviewed-on: https://chromium-review.googlesource.com/1124980 Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#572376} [modify] https://crrev.com/d7209bb8a53bcc9636b2c12f18e7f7b74b0c02c8/third_party/blink/renderer/core/paint/paint_and_raster_invalidation_test.cc [modify] https://crrev.com/d7209bb8a53bcc9636b2c12f18e7f7b74b0c02c8/third_party/blink/renderer/platform/graphics/paint/paint_controller.cc [modify] https://crrev.com/d7209bb8a53bcc9636b2c12f18e7f7b74b0c02c8/third_party/blink/renderer/platform/graphics/paint/paint_controller.h
,
Jul 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5c301712a67bbe70026048a224b4608628199e12 commit 5c301712a67bbe70026048a224b4608628199e12 Author: Chris Harrelson <chrishtr@chromium.org> Date: Wed Jul 04 01:20:53 2018 Fix CompositingContainer for replaced normal-flow stacking contexts. For such PaintLayers, the CompositingContainer is the parent, not the containing stacking context. This is because such stacking contexts paint during the normal-flow of the ancestor layout object. This also allows us to enable subsequence caching; the previous attempt last week was still suffering from the under-invalidation reported in issue 859520 . The testcases that crash on ASAN from the referenced bugs are fixed; one such example is included in this CL. Bug:859294, 719835 ,723076, 859520 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I6ab5271d70bd834482c22b89f09b93907788ed0c Reviewed-on: https://chromium-review.googlesource.com/1125269 Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Commit-Position: refs/heads/master@{#572428} [add] https://crrev.com/5c301712a67bbe70026048a224b4608628199e12/third_party/WebKit/LayoutTests/svg/foreignObject/foreignObject-position-crash.html [modify] https://crrev.com/5c301712a67bbe70026048a224b4608628199e12/third_party/blink/renderer/core/paint/compositing/graphics_layer_updater.cc [modify] https://crrev.com/5c301712a67bbe70026048a224b4608628199e12/third_party/blink/renderer/core/paint/paint_invalidator.cc [modify] https://crrev.com/5c301712a67bbe70026048a224b4608628199e12/third_party/blink/renderer/core/paint/paint_layer.cc [modify] https://crrev.com/5c301712a67bbe70026048a224b4608628199e12/third_party/blink/renderer/core/paint/paint_layer.h [modify] https://crrev.com/5c301712a67bbe70026048a224b4608628199e12/third_party/blink/renderer/core/paint/paint_layer_test.cc
,
Aug 15
,
Oct 8
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 |
|||||||||||||||||||||||
Comment 1 by ClusterFuzz
, Jun 30 2018Labels: Test-Predator-Auto-Components