Title: [286866] trunk
Revision
286866
Author
commit-qu...@webkit.org
Date
2021-12-10 11:37:38 -0800 (Fri, 10 Dec 2021)

Log Message

nullptr deref in ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded
https://bugs.webkit.org/show_bug.cgi?id=234018

Patch by Gabriel Nava Marino <gnavamar...@apple.com> on 2021-12-10
Reviewed by Alan Bujtas.

Source/WebCore:

Test: fast/rendering/floating-object-renderer-crash.html

When destroying a given renderer, we first remove floats and out-of-flow positioned objects
from their containing block before detaching the renderer from the tree. We do this by obtaining
the renderer’s outermost block containing a floating object and recursively marking all siblings
and descendants for layout.

The criteria for continuing down the list of children require the current block to contain floats
or be able to shrink to avoid floats. However, we can have a scenario where the current child block
doesn’t have a float, but one of its descendants does. In this case, although we should continue to
that descendant and remove the float, we do not.

The proposal in this patch will instead check whether the child block contains a float, or any of its descendants do.
If so we should continue traversing towards that descendant.

* rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::subtreeContainsFloat const):
(WebCore::RenderBlockFlow::subtreeContainsFloats const):
(WebCore::RenderBlockFlow::markAllDescendantsWithFloatsForLayout):
* rendering/RenderBlockFlow.h:

LayoutTests:

* fast/rendering/floating-object-renderer-crash-expected.txt: Added.
* fast/rendering/floating-object-renderer-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (286865 => 286866)


--- trunk/LayoutTests/ChangeLog	2021-12-10 19:34:01 UTC (rev 286865)
+++ trunk/LayoutTests/ChangeLog	2021-12-10 19:37:38 UTC (rev 286866)
@@ -1,3 +1,13 @@
+2021-12-10  Gabriel Nava Marino  <gnavamar...@apple.com>
+
+        nullptr deref in ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded
+        https://bugs.webkit.org/show_bug.cgi?id=234018
+
+        Reviewed by Alan Bujtas.
+
+        * fast/rendering/floating-object-renderer-crash-expected.txt: Added.
+        * fast/rendering/floating-object-renderer-crash.html: Added.
+
 2021-12-10  Commit Queue  <commit-qu...@webkit.org>
 
         Unreviewed, reverting r286836.

Added: trunk/LayoutTests/fast/rendering/floating-object-renderer-crash-expected.txt (0 => 286866)


--- trunk/LayoutTests/fast/rendering/floating-object-renderer-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/rendering/floating-object-renderer-crash-expected.txt	2021-12-10 19:37:38 UTC (rev 286866)
@@ -0,0 +1,3 @@
+PASS if this doesn't crash
+
+

Added: trunk/LayoutTests/fast/rendering/floating-object-renderer-crash.html (0 => 286866)


--- trunk/LayoutTests/fast/rendering/floating-object-renderer-crash.html	                        (rev 0)
+++ trunk/LayoutTests/fast/rendering/floating-object-renderer-crash.html	2021-12-10 19:37:38 UTC (rev 286866)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<style>
+  video {
+    block-size: 400px;
+  }
+  input {
+    float: left;
+    content: url();
+  }
+</style>
+<script>
+  _onload_ = () => {
+    document.documentElement.prepend(document.createElement('span'));
+    document.body.append(document.createElement('div'));
+    document.body.append(document.createElement('span'));
+    document.documentElement.append(document.createElement('div'));
+    let html2 = document.createElement('html');
+    html2.style.blockSize = '0';
+    document.body.appendChild(html2);
+    html2.appendChild(document.createElement('q'));
+    html2.appendChild(document.createElement('input'));
+    html2.appendChild(document.createElement('video'));
+    document.body.offsetTop;
+    document.styleSheets[0].insertRule(`html, body, video { float: left; }`);
+    if (window.testRunner)
+        testRunner.dumpAsText();
+  };
+</script>
+PASS if this doesn't crash

Modified: trunk/Source/WebCore/ChangeLog (286865 => 286866)


--- trunk/Source/WebCore/ChangeLog	2021-12-10 19:34:01 UTC (rev 286865)
+++ trunk/Source/WebCore/ChangeLog	2021-12-10 19:37:38 UTC (rev 286866)
@@ -1,3 +1,31 @@
+2021-12-10  Gabriel Nava Marino  <gnavamar...@apple.com>
+
+        nullptr deref in ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded
+        https://bugs.webkit.org/show_bug.cgi?id=234018
+
+        Reviewed by Alan Bujtas.
+
+        Test: fast/rendering/floating-object-renderer-crash.html
+
+        When destroying a given renderer, we first remove floats and out-of-flow positioned objects
+        from their containing block before detaching the renderer from the tree. We do this by obtaining
+        the renderer’s outermost block containing a floating object and recursively marking all siblings
+        and descendants for layout.
+
+        The criteria for continuing down the list of children require the current block to contain floats
+        or be able to shrink to avoid floats. However, we can have a scenario where the current child block
+        doesn’t have a float, but one of its descendants does. In this case, although we should continue to
+        that descendant and remove the float, we do not.
+
+        The proposal in this patch will instead check whether the child block contains a float, or any of its descendants do.
+        If so we should continue traversing towards that descendant.
+
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::RenderBlockFlow::subtreeContainsFloat const):
+        (WebCore::RenderBlockFlow::subtreeContainsFloats const):
+        (WebCore::RenderBlockFlow::markAllDescendantsWithFloatsForLayout):
+        * rendering/RenderBlockFlow.h:
+
 2021-12-10  Said Abou-Hallawa  <s...@apple.com>
 
         [GPU Process] [Filters] Make FilterEffectVector a Vector of Ref<FilterEffect>

Modified: trunk/Source/WebCore/rendering/RenderBlockFlow.cpp (286865 => 286866)


--- trunk/Source/WebCore/rendering/RenderBlockFlow.cpp	2021-12-10 19:34:01 UTC (rev 286865)
+++ trunk/Source/WebCore/rendering/RenderBlockFlow.cpp	2021-12-10 19:37:38 UTC (rev 286866)
@@ -2109,6 +2109,30 @@
     return m_floatingObjects && m_floatingObjects->set().contains<FloatingObjectHashTranslator>(renderer);
 }
 
+bool RenderBlockFlow::subtreeContainsFloat(RenderBox& renderer) const
+{
+    bool contains = m_floatingObjects && m_floatingObjects->set().contains<FloatingObjectHashTranslator>(renderer);
+    for (auto& block : childrenOfType<RenderBlock>(*this)) {
+        if (!is<RenderBlockFlow>(block))
+            continue;
+        auto& blockFlow = downcast<RenderBlockFlow>(block);
+        contains |= blockFlow.subtreeContainsFloat(renderer);
+    }
+    return contains;
+}
+
+bool RenderBlockFlow::subtreeContainsFloats() const
+{
+    bool contains = m_floatingObjects && !m_floatingObjects->set().isEmpty();
+    for (auto& block : childrenOfType<RenderBlock>(*this)) {
+        if (!is<RenderBlockFlow>(block))
+            continue;
+        auto& blockFlow = downcast<RenderBlockFlow>(block);
+        contains |= blockFlow.subtreeContainsFloats();
+    }
+    return contains;
+}
+
 void RenderBlockFlow::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
 {
     RenderBlock::styleDidChange(diff, oldStyle);
@@ -2878,7 +2902,7 @@
             continue;
         }
         auto& blockFlow = downcast<RenderBlockFlow>(block);
-        if ((floatToRemove ? blockFlow.containsFloat(*floatToRemove) : blockFlow.containsFloats()) || blockFlow.shrinkToAvoidFloats())
+        if ((floatToRemove ? blockFlow.subtreeContainsFloat(*floatToRemove) : blockFlow.subtreeContainsFloats()) || blockFlow.shrinkToAvoidFloats())
             blockFlow.markAllDescendantsWithFloatsForLayout(floatToRemove, inLayout);
     }
 }

Modified: trunk/Source/WebCore/rendering/RenderBlockFlow.h (286865 => 286866)


--- trunk/Source/WebCore/rendering/RenderBlockFlow.h	2021-12-10 19:34:01 UTC (rev 286865)
+++ trunk/Source/WebCore/rendering/RenderBlockFlow.h	2021-12-10 19:37:38 UTC (rev 286866)
@@ -278,6 +278,8 @@
 
     bool containsFloats() const override { return m_floatingObjects && !m_floatingObjects->set().isEmpty(); }
     bool containsFloat(RenderBox&) const;
+    bool subtreeContainsFloats() const;
+    bool subtreeContainsFloat(RenderBox&) const;
 
     void deleteLines() override;
     void computeOverflow(LayoutUnit oldClientAfterEdge, bool recomputeFloats = false) override;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to