Bug 206106

Summary: Null Ptr Deref READ @ WebCore::RenderMultiColumnFlow::lastMultiColumnSet const
Product: WebKit Reporter: Jack <shihchieh_lee>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, ews-feeder, ggaren, koivisto, product-security, rniwa, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch none

Jack
Reported 2020-01-10 15:43:35 PST
<rdar://problem/56685305>
Attachments
Patch (2.58 KB, patch)
2020-01-14 16:26 PST, Jack 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Jack
Comment 1 2020-01-14 16:26:06 PST
Created attachment 387725 [details] Patch
Jack
Comment 2 2020-01-24 11:03:22 PST
In this test case RenderMultiColumnFlowThread is being detached from LI RenderListItem, so the code tries to move its children to its parent (by searching for sibling and creating new RenderMultiColumnSet). However, because the nodes are being destroyed in preorder in function RenderTreeBuilder::destroy, no parent can be found for child insertion. Tried changing the destroy function to call detach in post-order, and the problem can be solved.
Jack
Comment 3 2020-01-24 11:04:02 PST
After discussing with Geoff
Jack
Comment 4 2020-01-24 11:12:21 PST
After discussing with Geoff, Alan and Antti, it was determined that the best approach is to check null multicolumn container (parent) and just exit the column processing functions. Doing so help expedite destroy process. If later other functions also try to refer container in destroy process, we should exit the function immediately. Ideally we should avoid moving children altogether, but that will require some refactoring, so we put null check for now. (In reply to Jack from comment #3) > After discussing with Geoff
Jack
Comment 5 2020-01-24 11:15:59 PST
Alan also verified with setting multi-column to 2 then 1 to make sure an element will be correctly attached back to multi-column container when RenderMultiColumnFlowThread is detached. Below is the html to verify RenderMultiColumnFlowThread attach/detach: <div id = container> <div>foo</div> <div>bar</div> </div> <script> container.style.webkitColumnCount = "2"; setTimeout(function() { container.style.webkitColumnCount = "1"; }, 5000); </script>
Jack
Comment 6 2020-01-24 11:20:59 PST
Change the bug to non-security since the parent pointer is correctly set to null when a render element is detached. The pointer will not point to random or freed address.
WebKit Commit Bot
Comment 7 2020-01-24 17:36:49 PST
The commit-queue encountered the following flaky tests while processing attachment 387725 [details]: editing/spelling/spellcheck-attribute.html bug 206178 (authors: g.czajkowski@samsung.com, mark.lam@apple.com, and rniwa@webkit.org) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 8 2020-01-24 17:37:25 PST
Comment on attachment 387725 [details] Patch Clearing flags on attachment: 387725 Committed r255113: <https://trac.webkit.org/changeset/255113>
WebKit Commit Bot
Comment 9 2020-01-24 17:37:27 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.