Changeset 287867 in webkit

Timestamp:

Jan 10, 2022, 7:21:20 PM (5 years ago)

Author:

Alan Bujtas

Message:

null ptr deref in WebCore::LayoutIntegration::LineLayout::collectOverflow()
​https://bugs.webkit.org/show_bug.cgi?id=234654
<rdar://problem/86571571>

Reviewed by Antti Koivisto.

Source/WebCore:

needsLayout() check in invalidateLineLayoutPath is insufficient for modern line layout.

m_lineLayout = std::monostate() does not only destroy the line layout object but it also nukes all the IFC geometries.
It is equivalent to having all the child boxes dirty, since in order to re-generate the geometry information,
we have to layout _all_ the boxes (note that nuking the legacy line layout object does not destroy the inline tree).
The bug here is that needsLayout() returns true for cases (e.g. posChildNeedsLayout) when
while the geometry is all gone, we are going to take a special layout codepath which expects pre-computed geometries.

Test: fast/block/line-layout/line-layout-collect-overflow-crash.html

• rendering/RenderBlockFlow.cpp:

(WebCore::RenderBlockFlow::invalidateLineLayoutPath):

LayoutTests:

• fast/block/line-layout/line-layout-collect-overflow-crash-expected.txt: Added.
• fast/block/line-layout/line-layout-collect-overflow-crash.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/block/line-layout/line-layout-collect-overflow-crash-expected.txt (added)
• LayoutTests/fast/block/line-layout/line-layout-collect-overflow-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBlockFlow.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2022-01-10  Alan Bujtas  <zalan@apple.com>
+
+        null ptr deref in WebCore::LayoutIntegration::LineLayout::collectOverflow()
+        https://bugs.webkit.org/show_bug.cgi?id=234654
+        <rdar://problem/86571571>
+
+        Reviewed by Antti Koivisto.
+
+        * fast/block/line-layout/line-layout-collect-overflow-crash-expected.txt: Added.
+        * fast/block/line-layout/line-layout-collect-overflow-crash.html: Added.
+
 2022-01-10  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2022-01-10  Alan Bujtas  <zalan@apple.com>
+
+        null ptr deref in WebCore::LayoutIntegration::LineLayout::collectOverflow()
+        https://bugs.webkit.org/show_bug.cgi?id=234654
+        <rdar://problem/86571571>
+
+        Reviewed by Antti Koivisto.
+
+        needsLayout() check in invalidateLineLayoutPath is insufficient for modern line layout.
+
+        m_lineLayout = std::monostate() does not only destroy the line layout object but it also nukes all the IFC geometries.
+        It is equivalent to having all the child boxes dirty, since in order to re-generate the geometry information,
+        we have to layout _all_ the boxes (note that nuking the legacy line layout object does not destroy the inline tree).
+        The bug here is that needsLayout() returns true for cases (e.g. posChildNeedsLayout) when
+        while the geometry is all gone, we are going to take a special layout codepath which expects pre-computed geometries.
+
+        Test: fast/block/line-layout/line-layout-collect-overflow-crash.html
+
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::RenderBlockFlow::invalidateLineLayoutPath):
+
 2022-01-10  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebCore/rendering/RenderBlockFlow.cpp

@@ -3563 +3563 @@
         m_lineLayout = std::monostate();
         setLineLayoutPath(path);
-        if (needsLayout())
+        if (selfNeedsLayout() || normalChildNeedsLayout())
             return;
         // FIXME: We should just kick off a subtree layout here (if needed at all) see webkit.org/b/172947.