New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 798918 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference READ in blink::StyleInvalidator::PushInvalidationSetsForContainerNode

Project Member Reported by ClusterFuzz, Jan 4 2018

Issue description

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

Fuzzer: mbarbella_webcomponents
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000010
Crash State:
  blink::StyleInvalidator::PushInvalidationSetsForContainerNode
  blink::StyleInvalidator::InvalidateShadowRootChildren
  blink::StyleInvalidator::InvalidateChildren
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=473072:473106

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

Additional requirements: Requires Gestures

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Jan 4 2018

Components: Blink>CSS
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, Jan 4 2018

Cc: nainar@chromium.org rlanday@chromium.org xiaoche...@chromium.org
Labels: Test-Predator-Auto-CC
Automatically adding ccs based on suspected regression changelists:

Rename DMC::SetMarkersActive() to SetTextMatchMarkersActive() by rlanday@chromium.org - https://chromium.googlesource.com/chromium/src/+/97cf10ffd89f897b67f489c4b072886e186c9944

Add class level comments to LayoutTreeBuilder by nainar@chromium.org - https://chromium.googlesource.com/chromium/src/+/f24a21c25dd45feb95fdf9bece7a589ab1126532

Make user-triggered SelectAll act as if there is no selection for hidden selection by xiaochengh@chromium.org - https://chromium.googlesource.com/chromium/src/+/bd5cd151162312c124b976bc822bbd0e6f4f2d39

If this is incorrect, please apply the Test-Predator-Wrong-CLs label.
Owner: futhark@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 4 2018

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

commit aa00f1943ede6fc272b377260ebe8e86a3883161
Author: Rune Lillesveen <futhark@chromium.org>
Date: Thu Jan 04 16:11:57 2018

Don't style recalc shadow root descendants if not necessary.

Sibling invalidation sets may be (re-)scheduled on shadow roots on dom
changes. If the removed node was the one that needed style invalidation,
we may end up with a style-dirty shadow root with no style-dirty
children. There is a DCHECK in RecalcDescendantStyles which triggers if
we call it unnecessarily. We called it unconditionally in ShadowRoot.

Discovered while minimizing the fuzzer case for the mentioned Bug.

Bug:  798918 
Change-Id: I4f59b1a77af6a89ed304abd88d7f8cca84775127
Reviewed-on: https://chromium-review.googlesource.com/850395
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526995}
[add] https://crrev.com/aa00f1943ede6fc272b377260ebe8e86a3883161/third_party/WebKit/LayoutTests/fast/css/invalidation/shadow-root-sibling-invalidation-crash.html
[modify] https://crrev.com/aa00f1943ede6fc272b377260ebe8e86a3883161/third_party/WebKit/Source/core/dom/ShadowRoot.cpp

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 5 2018

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

commit ac12e4fe4d31c4446589f1d3df44acaa2f12b9e6
Author: Rune Lillesveen <futhark@chromium.org>
Date: Fri Jan 05 13:21:55 2018

Use V1 instead of V0 shadow root in crash test.

Either way of creating a shadow root would cause the crasher here.
Prefer the V1 way.

Bug:  798918 
Change-Id: I90992b07236caa1cdda30d63a96b2e9601ae8b60
Reviewed-on: https://chromium-review.googlesource.com/852095
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527267}
[modify] https://crrev.com/ac12e4fe4d31c4446589f1d3df44acaa2f12b9e6/third_party/WebKit/LayoutTests/fast/css/invalidation/shadow-root-sibling-invalidation-crash.html

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 9 2018

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

commit ca6fa74c5abb1dbeb3de22453d1dd68fd4533174
Author: Rune Lillesveen <futhark@chromium.org>
Date: Tue Jan 09 02:48:15 2018

Clear style invalidation detaching ElementShadow.

DetachLayoutTree for ElementShadow did not set the clear_invalidation
bit of the AttachContext. Sibling invalidation sets are sometimes
(re-)scheduled on the parent node as descendant invalidations to make
sure invalidations still happens correctly after DOM changes. This
parent can be the shadow root. DetachLayoutTree clears scheduled
invalidations on all elements but the root in the detached subtree. The
root is not cleared because sibling invalidation sets may still apply
to siblings of the detached root. This is why the clear_invalidation
flag in the AttachContents is only set for descendants of the root of
the DetachLayoutTree. Normally, this is done in
ContainerNode::DetachLayoutTree before detaching children. For shadow
hosts we did not enable the flag entering the ElementShadow and the
shadow root(s).

Bug:  798918 
Change-Id: I0ce9ba2fdb89500d5a3093a376c291d329fab176
Reviewed-on: https://chromium-review.googlesource.com/853954
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527877}
[add] https://crrev.com/ca6fa74c5abb1dbeb3de22453d1dd68fd4533174/third_party/WebKit/LayoutTests/fast/css/invalidation/shadow-root-sibling-invalidation-crash-2.html
[modify] https://crrev.com/ca6fa74c5abb1dbeb3de22453d1dd68fd4533174/third_party/WebKit/Source/core/dom/ElementShadow.cpp

Status: Fixed (was: Started)
Project Member

Comment 8 by ClusterFuzz, Jan 9 2018

ClusterFuzz has detected this issue as fixed in range 527876:527878.

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

Fuzzer: mbarbella_webcomponents
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000010
Crash State:
  blink::StyleInvalidator::PushInvalidationSetsForContainerNode
  blink::StyleInvalidator::InvalidateShadowRootChildren
  blink::StyleInvalidator::InvalidateChildren
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=473072:473106
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=527876:527878

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

Additional requirements: Requires Gestures

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

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

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

Sign in to add a comment