Changeset 240061 in webkit

Timestamp:

Jan 16, 2019, 3:27:09 PM (8 years ago)

Author:

Alan Coon

Message:

Cherry-pick r239833. rdar://problem/47260274

Safari Crashing in Version 12.0.1 (14606.2.104.1.1) WebCore::GraphicsLayerCA::updateBackdropFilters
​https://bugs.webkit.org/show_bug.cgi?id=193309
<rdar://problem/45279224>

Reviewed by Antoine Quint.

A speculative fix for a CheckedArithmetic crash triggered in updateBackdropFilters.

The crash log indicates we crash in a Checked<> class that is not recording
overflow i.e. it is crashing due to an overflow. The only place in this function
where that could happen is when we convert the FloatRect for the backdrop
region into a Checked<unsigned> for width and height. This suggests that either
the width or height are negative, or the float values are too large for integers,
or the product of the two overflows.

Avoid this by using RecordOverflow, but also changing the code a little to
bail if the rectangle is incorrect.

• platform/graphics/ca/GraphicsLayerCA.cpp: (WebCore::GraphicsLayerCA::updateBackdropFilters):

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@239833 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:

branches/safari-607-branch/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/ca/GraphicsLayerCA.cpp (modified) (1 diff)

branches/safari-607-branch/Source/WebCore/ChangeLog

@@ +1 @@
+2019-01-15  Alan Coon  <alancoon@apple.com>
+
+        Cherry-pick r239833. rdar://problem/47260274
+
+    Safari Crashing in Version 12.0.1 (14606.2.104.1.1) WebCore::GraphicsLayerCA::updateBackdropFilters
+    https://bugs.webkit.org/show_bug.cgi?id=193309
+    <rdar://problem/45279224>
+    
+    Reviewed by Antoine Quint.
+    
+    A speculative fix for a CheckedArithmetic crash triggered in updateBackdropFilters.
+    
+    The crash log indicates we crash in a Checked<> class that is not recording
+    overflow i.e. it is crashing due to an overflow. The only place in this function
+    where that could happen is when we convert the FloatRect for the backdrop
+    region into a Checked<unsigned> for width and height. This suggests that either
+    the width or height are negative, or the float values are too large for integers,
+    or the product of the two overflows.
+    
+    Avoid this by using RecordOverflow, but also changing the code a little to
+    bail if the rectangle is incorrect.
+    
+    * platform/graphics/ca/GraphicsLayerCA.cpp:
+    (WebCore::GraphicsLayerCA::updateBackdropFilters):
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239833 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-01-09  Dean Jackson  <dino@apple.com>
+
+            Safari Crashing in Version 12.0.1 (14606.2.104.1.1) WebCore::GraphicsLayerCA::updateBackdropFilters
+            https://bugs.webkit.org/show_bug.cgi?id=193309
+            <rdar://problem/45279224>
+
+            Reviewed by Antoine Quint.
+
+            A speculative fix for a CheckedArithmetic crash triggered in updateBackdropFilters.
+
+            The crash log indicates we crash in a Checked<> class that is not recording
+            overflow i.e. it is crashing due to an overflow. The only place in this function
+            where that could happen is when we convert the FloatRect for the backdrop
+            region into a Checked<unsigned> for width and height. This suggests that either
+            the width or height are negative, or the float values are too large for integers,
+            or the product of the two overflows.
+
+            Avoid this by using RecordOverflow, but also changing the code a little to
+            bail if the rectangle is incorrect.
+
+            * platform/graphics/ca/GraphicsLayerCA.cpp:
+            (WebCore::GraphicsLayerCA::updateBackdropFilters):
+
 2019-01-15  Alan Coon  <alancoon@apple.com>
 

branches/safari-607-branch/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp

@@ -2148 +2148 @@
 void GraphicsLayerCA::updateBackdropFilters(CommitState& commitState)
 {
+    using CheckedUnsigned = Checked<unsigned, RecordOverflow>;
+
     bool canHaveBackdropFilters = needsBackdrop();
 
     if (canHaveBackdropFilters) {
-        Checked<unsigned, RecordOverflow> backdropFilterArea = Checked<unsigned>(static_cast<int>(m_backdropFiltersRect.rect().width())) * Checked<unsigned>(static_cast<int>(m_backdropFiltersRect.rect().height()));
-        if (backdropFilterArea.hasOverflowed())
-            canHaveBackdropFilters = false;
-        else {
-            Checked<unsigned, RecordOverflow> newTotalBackdropFilterArea = Checked<unsigned, RecordOverflow>(commitState.totalBackdropFilterArea) + backdropFilterArea;
-            if (newTotalBackdropFilterArea.hasOverflowed() || newTotalBackdropFilterArea.unsafeGet() > cMaxTotalBackdropFilterArea)
-                canHaveBackdropFilters = false;
-            else
-                commitState.totalBackdropFilterArea = newTotalBackdropFilterArea.unsafeGet();
+        canHaveBackdropFilters = false;
+        IntRect backdropFilterRect = enclosingIntRect(m_backdropFiltersRect.rect());
+        if (backdropFilterRect.width() > 0 && backdropFilterRect.height() > 0) {
+            CheckedUnsigned backdropFilterArea = CheckedUnsigned(backdropFilterRect.width()) * CheckedUnsigned(backdropFilterRect.height());
+            if (!backdropFilterArea.hasOverflowed()) {
+                CheckedUnsigned newTotalBackdropFilterArea = CheckedUnsigned(commitState.totalBackdropFilterArea) + backdropFilterArea;
+                if (!newTotalBackdropFilterArea.hasOverflowed() && newTotalBackdropFilterArea.unsafeGet() <= cMaxTotalBackdropFilterArea) {
+                    commitState.totalBackdropFilterArea = newTotalBackdropFilterArea.unsafeGet();
+                    canHaveBackdropFilters = true;
+                }
+            }
         }
     }