Null-dereference READ in blink::BaselineContext::FindCompatibleSharedGroup |
||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=5830458616315904 Fuzzer: inferno_twister_c Job Type: linux_asan_chrome_media Platform Id: linux Crash Type: Null-dereference READ Crash Address: 0x000000000000 Crash State: blink::BaselineContext::FindCompatibleSharedGroup blink::BaselineContext::GetSharedGroup blink::LayoutGrid::GetBaselineGroupForChild Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_media&range=472654:472713 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5830458616315904 Issue filed automatically. See https://github.com/google/clusterfuzz-tools for more information.
,
Aug 12 2017
,
Aug 21 2017
I'll take a look ASAP
,
Aug 23 2017
Attached a minimized test case to reproduce the crash.
,
Aug 23 2017
Setting a longer timeout makes more noticeable what action triggers the crash. It seems that adding the c29 CSS class, which sets the 'content' property value to an empty string (any string will produce the same result), triggers some internal actions that lead to a crash during layout.
,
Aug 23 2017
Still speculating about the root cause of the crash, but it seems clear that the problem comes from the fact that both grid have auto-sized tracks and the nested grid has indefinite width.
,
Aug 23 2017
Another interesting point is that the nested grid's child is orthogonal. Otherwise the crash is not reproducible.
,
Aug 23 2017
I think I finally found out the actual root cause of the crash. It seems that changes in the 'content' property implies a Style reattach. When applied to the nested grid's child, it makes such grid is marked as dirty, hence, needing to execute again the items placement logic. After Attaching the new style, a new layout is performed on the whole tree, which implies computing again the nested grid's intrinsic size. This computation involves a new run of the track sizing algorithm. Since we have auto-sized tracks, the MinContentForChild logic is executed, which ends up trying to reuse the Baseline extent computed in the previous layout; we only do this when dealing with orthogonal items while computing the column track width. In order to figure out the Baseline extent for a child, we need to know which Baseline grout it belongs to. For this, we need to know which grid area it's placed into. Since the grid has been marked as dirty, and it still need to be laid out, the items placement logic has not been executed yet, hence there is no way to gather the Baseline related information. Actually, the grid logic has an assertion to verify the grid area structures are not used before the item's placement logic is executed. This is precisely the assert triggered by the attached test case.
,
Aug 23 2017
,
Aug 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1b9a649524e74b55b4752fa7f5a545c9079bc541 commit 1b9a649524e74b55b4752fa7f5a545c9079bc541 Author: Javier Fernandez <jfernandez@igalia.com> Date: Thu Aug 31 16:10:13 2017 [css-grid] Clearing baseline context structures when grid is dirty When the grid is marked as dirty the Grid data structures are cleared and it must execute again the items placement logic. Style changes applied to certain properties will cause a style Reattach,hence the affected layout tree must be built again. This obviously implies the grid must be marked as dirty. The LayoutGrid class holds some data structures to implement the logic for the Baseline Alignment of its Grid items. We are not clearing these structures when the grid is marked as dirty; instead, we do it at the beginning of the layout operation. This is enough in most of the cases, however, this issue proves otherwise. Another important factor to understand the issue is that we use the track sizing algorithm to compute the grid's intrinsic size. This operation has to be done without doing a layout of the grid itself. Additionally, when dealing with orthogonal items we use the baseline offsets if they are available, because they affect to the grid's intrinsic size. In order to gather the Baseline data of a grid item it's needed to determine the Baseline Group such item belongs to. For that, it's also necessary to know which grid area the item is placed in. When the above mentioned style changes are applied to a nested grid we mark it as dirty, hence its basic Grid structures are cleared. As it's also a grid item, we may need to compute its intrinsic size as part of the layout logic on its parent grid. This computation will imply running the whole track sizing algorithm (under intrinsic size mode), which finally tries to use the baseline data (because of the orthogonal children). Since we haven't performed the layout of the nested grid, we still have the baseline data structures. This will lead to trigger the assertion described in the bug. Bug: 750479 Change-Id: If70ed652cb86ed4e86c1033a605ca276d1344c51 Reviewed-on: https://chromium-review.googlesource.com/630456 Commit-Queue: Javier Fernandez <jfernandez@igalia.com> Reviewed-by: Sergio Villar <svillar@igalia.com> Cr-Commit-Position: refs/heads/master@{#498879} [add] https://crrev.com/1b9a649524e74b55b4752fa7f5a545c9079bc541/third_party/WebKit/LayoutTests/fast/css-grid-layout/changing-content-property-on-nested-grid-should-not-crash-expected.txt [add] https://crrev.com/1b9a649524e74b55b4752fa7f5a545c9079bc541/third_party/WebKit/LayoutTests/fast/css-grid-layout/changing-content-property-on-nested-grid-should-not-crash.html [modify] https://crrev.com/1b9a649524e74b55b4752fa7f5a545c9079bc541/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
,
Aug 31 2017
This issue should be FIXED now.
,
Sep 1 2017
ClusterFuzz has detected this issue as fixed in range 498874:498922. Detailed report: https://clusterfuzz.com/testcase?key=5830458616315904 Fuzzer: inferno_twister_c Job Type: linux_asan_chrome_media Platform Id: linux Crash Type: Null-dereference READ Crash Address: 0x000000000000 Crash State: blink::BaselineContext::FindCompatibleSharedGroup blink::BaselineContext::GetSharedGroup blink::LayoutGrid::GetBaselineGroupForChild Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_media&range=472654:472713 Fixed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_media&range=498874:498922 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5830458616315904 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.
,
Sep 1 2017
ClusterFuzz testcase 5830458616315904 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 |
||||||
Comment 1 by msrchandra@chromium.org
, Jul 31 2017Labels: M-62 Test-Predator-Wrong
Owner: jfernandez@chromium.org
Status: Assigned (was: Untriaged)