No subject
Fri Mar 7 15:32:22 PST 2014
4582">r154582</a> by <jchaffraix at chromium.org>
Source/WebCore:
This is a regression from the changes in OrderIterator. The issue is
that we don't invalidate our iterator when a child is removed. This
means that we could hold onto free'd memory until the next layout
when we will recompute the iterator.
The solution is simple: just clear the memory when we remove a child.
Note that RenderGrid is not impacted by this bug as we don't use the
iterator outside layout yet, but if we do it at some point the very same
problem will arise, so the same treatment was applied to the class.
Test: fast/flexbox/order-iterator-crash.html
* rendering/OrderIterator.cpp:
(WebCore::OrderIterator::invalidate): Clear m_children Vector.
* rendering/OrderIterator.h:
(WebCore::OrderIteratorPopulator::OrderIteratorPopulator): Use
invalidate() method.
* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::removeChild): Invalidate m_orderIterator.
* rendering/RenderFlexibleBox.h: Add removeChild() signature.
* rendering/RenderGrid.cpp: Invalidate m_orderIterator.
(WebCore::RenderGrid::removeChild):
* rendering/RenderGrid.h: Add removeChild() header.
LayoutTests:
Add new layout test to check that removing a child doesn't cause a crash
in OrderIterator.
* fast/flexbox/order-iterator-crash-expected.txt: Added.
* fast/flexbox/order-iterator-crash.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href=3D"#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a=
></li>
<li><a href=3D"#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeL=
og</a></li>
<li><a href=3D"#trunkSourceWebCorerenderingOrderIteratorcpp">trunk/Source=
/WebCore/rendering/OrderIterator.cpp</a></li>
<li><a href=3D"#trunkSourceWebCorerenderingOrderIteratorh">trunk/Source/W=
ebCore/rendering/OrderIterator.h</a></li>
<li><a href=3D"#trunkSourceWebCorerenderingRenderFlexibleBoxcpp">trunk/So=
urce/WebCore/rendering/RenderFlexibleBox.cpp</a></li>
<li><a href=3D"#trunkSourceWebCorerenderingRenderFlexibleBoxh">trunk/Sour=
ce/WebCore/rendering/RenderFlexibleBox.h</a></li>
<li><a href=3D"#trunkSourceWebCorerenderingRenderGridcpp">trunk/Source/We=
bCore/rendering/RenderGrid.cpp</a></li>
<li><a href=3D"#trunkSourceWebCorerenderingRenderGridh">trunk/Source/WebC=
ore/rendering/RenderGrid.h</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href=3D"#trunkLayoutTestsfastflexboxorderiteratorcrashexpectedtxt"=
>trunk/LayoutTests/fast/flexbox/order-iterator-crash-expected.txt</a></li=
>
<li><a href=3D"#trunkLayoutTestsfastflexboxorderiteratorcrashhtml">trunk/=
LayoutTests/fast/flexbox/order-iterator-crash.html</a></li>
</ul>
</div>
<div id=3D"patch">
<h3>Diff</h3>
<a id=3D"trunkLayoutTestsChangeLog"></a>
<div class=3D"modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (167941 =
=3D> 167942)</h4>
<pre class=3D"diff"><span>
<span class=3D"info">--- trunk/LayoutTests/ChangeLog 2014-04-29 17:28:52 =
UTC (rev 167941)
+++ trunk/LayoutTests/ChangeLog 2014-04-29 17:34:34 UTC (rev 167942)
</span><span class=3D"lines">@@ -1,3 +1,18 @@
</span><ins>+2014-04-29 Manuel Rego Casasnovas <rego at igalia.com>
+
+ REGRESSION (r167879): Heap-use-after-free in WebCore::RenderFlex=
ibleBox
+ https://bugs.webkit.org/show_bug.cgi?id=3D132337
+
+ Reviewed by Simon Fraser.
+
+ From Blink r154582 by <jchaffraix at chromium.org>
+
+ Add new layout test to check that removing a child doesn't cause=
a crash
+ in OrderIterator.
+
+ * fast/flexbox/order-iterator-crash-expected.txt: Added.
+ * fast/flexbox/order-iterator-crash.html: Added.
+
</ins><span class=3D"cx"> 2014-04-29 Hans Muller <hmuller at adobe.com&=
gt;
</span><span class=3D"cx">=20
</span><span class=3D"cx"> [CSS Shapes] off-by-one error in Shape=
::createRasterShape()
</span></span></pre></div>
<a id=3D"trunkLayoutTestsfastflexboxorderiteratorcrashexpectedtxt"></a>
<div class=3D"addfile"><h4>Added: trunk/LayoutTests/fast/flexbox/order-it=
erator-crash-expected.txt (0 =3D> 167942)</h4>
<pre class=3D"diff"><span>
<span class=3D"info">--- trunk/LayoutTests/fast/flexbox/order-iterator-cr=
ash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/flexbox/order-iterator-crash-expected.txt 2014=
-04-29 17:34:34 UTC (rev 167942)
</span><span class=3D"lines">@@ -0,0 +1,3 @@
</span><ins>+This test has passed if it doesn't crash.
+* { display: -webkit-flex; }
+if (window.testRunner) testRunner.dumpAsText(); crashy.offsetLeft; crash=
y.parentNode.removeChild(crashy);
</ins></span></pre></div>
<a id=3D"trunkLayoutTestsfastflexboxorderiteratorcrashhtml"></a>
<div class=3D"addfile"><h4>Added: trunk/LayoutTests/fast/flexbox/order-it=
erator-crash.html (0 =3D> 167942)</h4>
<pre class=3D"diff"><span>
<span class=3D"info">--- trunk/LayoutTests/fast/flexbox/order-iterator-cr=
ash.html (rev 0)
+++ trunk/LayoutTests/fast/flexbox/order-iterator-crash.html 2014-04-29 1=
7:34:34 UTC (rev 167942)
</span><span class=3D"lines">@@ -0,0 +1,12 @@
</span><ins>+<div>This test has passed if it doesn't crash.</div=
>
+<style>
+* { display: -webkit-flex; }
+</style>
+<table><td id=3D"crashy"></td></table>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+crashy.offsetLeft;
+crashy.parentNode.removeChild(crashy);
+</script>
</ins></span></pre></div>
<a id=3D"trunkSourceWebCoreChangeLog"></a>
<div class=3D"modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (1679=
41 =3D> 167942)</h4>
<pre class=3D"diff"><span>
<span class=3D"info">--- trunk/Source/WebCore/ChangeLog 2014-04-29 17:28:=
52 UTC (rev 167941)
+++ trunk/Source/WebCore/ChangeLog 2014-04-29 17:34:34 UTC (rev 167942)
</span><span class=3D"lines">@@ -1,3 +1,37 @@
</span><ins>+2014-04-29 Manuel Rego Casasnovas <rego at igalia.com>
+
+ REGRESSION (r167879): Heap-use-after-free in WebCore::RenderFlex=
ibleBox
+ https://bugs.webkit.org/show_bug.cgi?id=3D132337
+
+ Reviewed by Simon Fraser.
+
+ From Blink r154582 by <jchaffraix at chromium.org>
+
+ This is a regression from the changes in OrderIterator. The issu=
e is
+ that we don't invalidate our iterator when a child is removed. T=
his
+ means that we could hold onto free'd memory until the next layou=
t
+ when we will recompute the iterator.
+
+ The solution is simple: just clear the memory when we remove a c=
hild.
+
+ Note that RenderGrid is not impacted by this bug as we don't use=
the
+ iterator outside layout yet, but if we do it at some point the v=
ery same
+ problem will arise, so the same treatment was applied to the cla=
ss.
+
+ Test: fast/flexbox/order-iterator-crash.html
+
+ * rendering/OrderIterator.cpp:
+ (WebCore::OrderIterator::invalidate): Clear m_children Vector.
+ * rendering/OrderIterator.h:
+ (WebCore::OrderIteratorPopulator::OrderIteratorPopulator): Use
+ invalidate() method.
+ * rendering/RenderFlexibleBox.cpp:
+ (WebCore::RenderFlexibleBox::removeChild): Invalidate m_orderIte=
rator.
+ * rendering/RenderFlexibleBox.h: Add removeChild() signature.
+ * rendering/RenderGrid.cpp: Invalidate m_orderIterator.
+ (WebCore::RenderGrid::removeChild):
+ * rendering/RenderGrid.h: Add removeChild() header.
+
</ins><span class=3D"cx"> 2014-04-29 Enrica Casucci <enrica at apple.co=
m>
</span><span class=3D"cx">=20
</span><span class=3D"cx"> iOS build fix after http://trac.webkit=
.org/changeset/167937.
</span></span></pre></div>
<a id=3D"trunkSourceWebCorerenderingOrderIteratorcpp"></a>
<div class=3D"modfile"><h4>Modified: trunk/Source/WebCore/rendering/Order=
Iterator.cpp (167941 =3D> 167942)</h4>
<pre class=3D"diff"><span>
<span class=3D"info">--- trunk/Source/WebCore/rendering/OrderIterator.cpp=
2014-04-29 17:28:52 UTC (rev 167941)
+++ trunk/Source/WebCore/rendering/OrderIterator.cpp 2014-04-29 17:34:34 =
UTC (rev 167942)
</span><span class=3D"lines">@@ -56,6 +56,11 @@
</span><span class=3D"cx"> return currentChild();
</span><span class=3D"cx"> }
</span><span class=3D"cx">=20
</span><ins>+void OrderIterator::invalidate()
+{
+ m_children.clear();
+}
+
</ins><span class=3D"cx"> static bool compareByOrderValueAndIndex(std::pa=
ir<RenderBox*, int> childAndIndex1, std::pair<RenderBox*, int>=
; childAndIndex2)
</span><span class=3D"cx"> {
</span><span class=3D"cx"> if (childAndIndex1.first->style().order=
() !=3D childAndIndex2.first->style().order())
</span></span></pre></div>
<a id=3D"trunkSourceWebCorerenderingOrderIteratorh"></a>
<div class=3D"modfile"><h4>Modified: trunk/Source/WebCore/rendering/Order=
Iterator.h (167941 =3D> 167942)</h4>
<pre class=3D"diff"><span>
<span class=3D"info">--- trunk/Source/WebCore/rendering/OrderIterator.h 2=
014-04-29 17:28:52 UTC (rev 167941)
+++ trunk/Source/WebCore/rendering/OrderIterator.h 2014-04-29 17:34:34 UT=
C (rev 167942)
</span><span class=3D"lines">@@ -47,6 +47,8 @@
</span><span class=3D"cx"> RenderBox* first();
</span><span class=3D"cx"> RenderBox* next();
</span><span class=3D"cx">=20
</span><ins>+ void invalidate();
+
</ins><span class=3D"cx"> private:
</span><span class=3D"cx"> void reset();
</span><span class=3D"cx">=20
</span><span class=3D"lines">@@ -61,7 +63,7 @@
</span><span class=3D"cx"> , m_childIndex(0)
</span><span class=3D"cx"> , m_allChildrenHaveDefaultOrderValue(t=
rue)
</span><span class=3D"cx"> {
</span><del>- m_iterator.m_children.clear();
</del><ins>+ m_iterator.invalidate();
</ins><span class=3D"cx"> }
</span><span class=3D"cx">=20
</span><span class=3D"cx"> ~OrderIteratorPopulator();
</span></span></pre></div>
<a id=3D"trunkSourceWebCorerenderingRenderFlexibleBoxcpp"></a>
<div class=3D"modfile"><h4>Modified: trunk/Source/WebCore/rendering/Rende=
rFlexibleBox.cpp (167941 =3D> 167942)</h4>
<pre class=3D"diff"><span>
<span class=3D"info">--- trunk/Source/WebCore/rendering/RenderFlexibleBox=
.cpp 2014-04-29 17:28:52 UTC (rev 167941)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp 2014-04-29 17:34=
:34 UTC (rev 167942)
</span><span class=3D"lines">@@ -1392,4 +1392,10 @@
</span><span class=3D"cx"> return isHorizontalFlow();
</span><span class=3D"cx"> }
</span><span class=3D"cx">=20
</span><ins>+void RenderFlexibleBox::removeChild(RenderObject& child)
+{
+ RenderBlock::removeChild(child);
+ m_orderIterator.invalidate();
</ins><span class=3D"cx"> }
</span><ins>+
+}
</ins></span></pre></div>
<a id=3D"trunkSourceWebCorerenderingRenderFlexibleBoxh"></a>
<div class=3D"modfile"><h4>Modified: trunk/Source/WebCore/rendering/Rende=
rFlexibleBox.h (167941 =3D> 167942)</h4>
<pre class=3D"diff"><span>
<span class=3D"info">--- trunk/Source/WebCore/rendering/RenderFlexibleBox=
.h 2014-04-29 17:28:52 UTC (rev 167941)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.h 2014-04-29 17:34:3=
4 UTC (rev 167942)
</span><span class=3D"lines">@@ -60,6 +60,8 @@
</span><span class=3D"cx"> bool isTopLayoutOverflowAllowed() const ov=
erride;
</span><span class=3D"cx"> bool isLeftLayoutOverflowAllowed() const o=
verride;
</span><span class=3D"cx">=20
</span><ins>+ void removeChild(RenderObject&) override;
+
</ins><span class=3D"cx"> protected:
</span><span class=3D"cx"> virtual void computeIntrinsicLogicalWidths=
(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const =
override;
</span><span class=3D"cx"> virtual void computePreferredLogicalWidths=
() override;
</span></span></pre></div>
<a id=3D"trunkSourceWebCorerenderingRenderGridcpp"></a>
<div class=3D"modfile"><h4>Modified: trunk/Source/WebCore/rendering/Rende=
rGrid.cpp (167941 =3D> 167942)</h4>
<pre class=3D"diff"><span>
<span class=3D"info">--- trunk/Source/WebCore/rendering/RenderGrid.cpp 20=
14-04-29 17:28:52 UTC (rev 167941)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp 2014-04-29 17:34:34 UTC=
(rev 167942)
</span><span class=3D"lines">@@ -1186,6 +1186,12 @@
</span><span class=3D"cx"> return "RenderGrid";
</span><span class=3D"cx"> }
</span><span class=3D"cx">=20
</span><ins>+void RenderGrid::removeChild(RenderObject& child)
+{
+ RenderBlock::removeChild(child);
+ m_orderIterator.invalidate();
+}
+
</ins><span class=3D"cx"> } // namespace WebCore
</span><span class=3D"cx">=20
</span><span class=3D"cx"> #endif /* ENABLE(CSS_GRID_LAYOUT) */
</span></span></pre></div>
<a id=3D"trunkSourceWebCorerenderingRenderGridh"></a>
<div class=3D"modfile"><h4>Modified: trunk/Source/WebCore/rendering/Rende=
rGrid.h (167941 =3D> 167942)</h4>
<pre class=3D"diff"><span>
<span class=3D"info">--- trunk/Source/WebCore/rendering/RenderGrid.h 2014=
-04-29 17:28:52 UTC (rev 167941)
+++ trunk/Source/WebCore/rendering/RenderGrid.h 2014-04-29 17:34:34 UTC (=
rev 167942)
</span><span class=3D"lines">@@ -58,6 +58,8 @@
</span><span class=3D"cx"> const Vector<LayoutUnit>& column=
Positions() const { return m_columnPositions; }
</span><span class=3D"cx"> const Vector<LayoutUnit>& rowPos=
itions() const { return m_rowPositions; }
</span><span class=3D"cx">=20
</span><ins>+ void removeChild(RenderObject&) override;
+
</ins><span class=3D"cx"> private:
</span><span class=3D"cx"> virtual const char* renderName() const ove=
rride;
</span><span class=3D"cx"> virtual bool isRenderGrid() const override=
{ return true; }
</span></span></pre>
</div>
</div>
</body>
</html>
More information about the webkit-changes
mailing list