Issue metadata
Sign in to add a comment
|
Security CHECK failure: !cached_item->IsTombstone() in paint_controller.cc |
||||||||||||||||||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=6335079606124544 Fuzzer: ochang_domfuzzer Job Type: windows_asan_content_shell Platform Id: windows Crash Type: Security CHECK failure Crash Address: Crash State: !cached_item->IsTombstone() in paint_controller.cc blink::PaintController::CopyCachedSubsequence blink::PaintController::UseCachedSubsequenceIfPossible Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=windows_asan_content_shell&range=537543:537544 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6335079606124544 Issue filed automatically. See https://github.com/google/clusterfuzz-tools for more information.
,
Apr 22 2018
Narjane99@gmail.com
,
Apr 23 2018
Predator and CL could not provide any possible suspects. Using Code Search for the file, "paint_controller.cc" suspecting the below Cl might have caused this issue Suspect CL: https://chromium.googlesource.com/chromium/src/+/d5803f330a4355c1d11e86e388d1bbb2e25b2b08%5E%21/third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp wangxianzhu@ -- Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. Thanks!
,
Apr 27 2018
Reproduced this with out/Release/content_shell --content-shell-host-window-size=1830x361 --run-layout-test fuzz-112-viewspec-viewtargetparams.html
,
Apr 27 2018
Reduced test case attached. This is a weird case: - The HTML element is multi-column; - The BODY element is paged and column-span:all; - The OBJECT element is painted multi-paged, but it doesn't know because its PaginationContainer() is nullptr, and the content HTML creates subsequences. The OBJECT element's layer has null PaginationContainer() because we found it's under a column span: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/layout/layout_flow_thread.cc?rcl=b41c63db5b5efa88c4205225cd329a44a18b3492&l=69. Should we continue the outer loop because there might be paged-y container?
,
Apr 29 2018
Neat! I haven't debugged it, but here's what it looks like to me: Paged overflow on BODY is propagated to the viewport, just like any other overflow value. So the paged container should actually end up being established by LayoutView, not by BODY. So the paged container actually becomes the ancestor of the of the multicol container, not the other way around. BODY should still be remain a spanner (as a regular block, not a paged container), though. If my guess is correct, you should be able to create the same situation with regular DIVs (rather than BODY and HTML): A paged container with a multicol child with a spanner child, with this OBJECT thing inside. Regarding the code you point to: The containing flow thread of BODY and its children should ideally be the paged container. This is bug 757947 (that we just return nullptr like this), which I'm not sure if it's a good idea to fix. Just assign this one to me if you don't have any reasonable work-around on the paint side for it.
,
Apr 30 2018
Is it the HTML element which creates multiple subsequences? How exactly?
,
Apr 30 2018
It's the HTML element in the subdocument in the OBJECT element creating multiple subsequences. We have the mechanism to prevent the issue in subdocuments painted in fragments: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/embedded_content_painter.cc?rcl=73a64ffde3f3b64df576aa1f2b5baebf7ec964ba&l=35, but in this case, the OBJECT element doesn't know it's painted in fragments because its EnclosingPaginationLayer() is null.
,
Apr 30 2018
I see...nested multicol plus column-span: all.. I'd prefer to fix the bug by plumbing the fact that there is some ancestor with multicol to EmbeddedContentPainter, even if the parent is not. It seems we could do that more easily and safer than fully fixing issue 757947.
,
Apr 30 2018
Do you mean we always walk up the ancestor chain to see if the object has a LayoutFlowThread ancestor? I think that would degrade performance. I think regardless the root cause, https://chromium-review.googlesource.com/c/chromium/src/+/1034114 can be a fail-safe for duplicated subsequences. It can avoid the security CHECK, though still fails DCHECKs. As the test case is unlikely to exist in real world, I think DCHECK failure is fine, and I'd prefer to leave the root cause to bug 757947.
,
Apr 30 2018
Ok.
,
May 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ecad646aff6cc66dbb18e785d3b203bdde7a27e0 commit ecad646aff6cc66dbb18e785d3b203bdde7a27e0 Author: Xianzhu Wang <wangxianzhu@chromium.org> Date: Tue May 01 04:40:46 2018 [PE] Avoid SECURITY_CHECK failure on duplicated subsequences For now in rare cases a client may create multiple subsequences, e.g. the bug case. Previously this led to a SECURITY_CHECK failure. Bug: 835631 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: Ifb3ef4b9e00d3ef00b4f3666120b7399733cfb9b Reviewed-on: https://chromium-review.googlesource.com/1034114 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Commit-Position: refs/heads/master@{#554990} [modify] https://crrev.com/ecad646aff6cc66dbb18e785d3b203bdde7a27e0/third_party/blink/renderer/platform/graphics/paint/paint_controller.cc [modify] https://crrev.com/ecad646aff6cc66dbb18e785d3b203bdde7a27e0/third_party/blink/renderer/platform/graphics/paint/paint_controller_test.cc
,
May 1 2018
ClusterFuzz has detected this issue as fixed in range 554989:554991. Detailed report: https://clusterfuzz.com/testcase?key=6335079606124544 Fuzzer: ochang_domfuzzer Job Type: windows_asan_content_shell Platform Id: windows Crash Type: Security CHECK failure Crash Address: Crash State: !cached_item->IsTombstone() in paint_controller.cc blink::PaintController::CopyCachedSubsequence blink::PaintController::UseCachedSubsequenceIfPossible Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=windows_asan_content_shell&range=537543:537544 Fixed: https://clusterfuzz.com/revisions?job=windows_asan_content_shell&range=554989:554991 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6335079606124544 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.
,
May 1 2018
ClusterFuzz testcase 6335079606124544 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Jun 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5f5e98ab81a127dbc59fe87205e0082dcf41773b commit 5f5e98ab81a127dbc59fe87205e0082dcf41773b Author: Xianzhu Wang <wangxianzhu@chromium.org> Date: Tue Jun 05 21:34:22 2018 [PE] Avoid SECURITY_CHECK failure on duplicated subsequences For now in rare cases a client may create multiple subsequences, e.g. the bug case. Previously this led to a SECURITY_CHECK failure. TBR=wangxianzhu@chromium.org (cherry picked from commit ecad646aff6cc66dbb18e785d3b203bdde7a27e0) Bug: 835631 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: Ifb3ef4b9e00d3ef00b4f3666120b7399733cfb9b Reviewed-on: https://chromium-review.googlesource.com/1034114 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#554990} Reviewed-on: https://chromium-review.googlesource.com/1087790 Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#754} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/5f5e98ab81a127dbc59fe87205e0082dcf41773b/third_party/blink/renderer/platform/graphics/paint/paint_controller.cc [modify] https://crrev.com/5f5e98ab81a127dbc59fe87205e0082dcf41773b/third_party/blink/renderer/platform/graphics/paint/paint_controller_test.cc
,
Jun 5 2018
Merged for bug 847120. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by ClusterFuzz
, Apr 22 2018Labels: Test-Predator-Auto-Components