jsc_fuz/wktr: null ptr deref in WebCore::GraphicsLayerAsyncContentsDisplayDelegateCocoa::tryCopyToLayer(WebCore::ImageBuffer&)

mattwoodrow · mattwoodrow · commit bdc9ba2c424d · 2023-12-20T12:00:05.000-08:00
https://bugs.webkit.org/show_bug.cgi?id=262640 <rdar://115497296> Reviewed by Kimmo Kinnunen. This adds support for setDelegatedContents on a PlatformCALayerRemote having a generic ImageBufferBackendHandle (which includes shared memory), instead of only MachSendRight. Adds an explicit copy constructor to SharedMemoryHandle, UnixFileDescriptor and CGDisplayList to match MachSendRight and make this possible. Also switches Protection::ReadWrite to Protection::ReadOnly for the RemoteLayerBackingStore callers, since we were already using this for tryCopyToLayer, and we need the ::map() call in the UI process to not try ask for extra permissions. * Source/WTF/wtf/unix/UnixFileDescriptor.h: (WTF::UnixFileDescriptor::UnixFileDescriptor): * Source/WebKit/Platform/SharedMemory.h: * Source/WebKit/Shared/RemoteLayerTree/CGDisplayList.h: * Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.h: * Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm: (WebKit::RemoteLayerBackingStore::encode const): (WebKit::RemoteLayerBackingStore::setDelegatedContents): (WebKit::RemoteLayerBackingStoreProperties::layerContentsBufferFromBackendHandle): * Source/WebKit/Shared/ShareableBitmap.h: * Source/WebKit/WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.mm: * Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.h: * Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.mm: (WebKit::PlatformCALayerRemote::setDelegatedContents): (WebKit::PlatformCALayerRemote::setRemoteDelegatedContents): Originally-landed-as: 267815.262@safari-7617-branch (8ac1946). rdar://119570861 Canonical link: https://commits.webkit.org/272365@main
diff --git a/LayoutTests/fast/canvas/offscreen-giant-expected.html b/LayoutTests/fast/canvas/offscreen-giant-expected.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<body>
+<canvas id="onscreen" width="100" height="10""></canvas>
+<script>
+const canvas = document.getElementById('onscreen');
+
+const context = canvas.getContext('2d');
+
+const square = new Path2D();
+square.rect(0, 0, 100, 10);
+context.fillStyle = 'green';
+context.fill(square);
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/canvas/offscreen-giant.html b/LayoutTests/fast/canvas/offscreen-giant.html
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta name="fuzzy" content="maxDifference=0-64; totalPixels=0-10" />
+</head>
+<body style="overflow:hidden">
+<canvas id="offscreen"></canvas>
+<script>
+const canvas = document.getElementById('offscreen');
+
+const offscreenCanvas = canvas.transferControlToOffscreen();
+offscreenCanvas.width = 200000;
+offscreenCanvas.height = 10;
+
+const offscreenContext = offscreenCanvas.getContext('2d');
+
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+requestAnimationFrame(function() {
+    const square = new Path2D();
+    square.rect(0, 0, 100, 10);
+    offscreenContext.fillStyle = 'green';
+    offscreenContext.fill(square);
+    offscreenContext.commit();
+
+    requestAnimationFrame(function() {
+        if (window.testRunner)
+            testRunner.notifyDone();
+    });
+});
+</script>
+</body>
+</html>
diff --git a/LayoutTests/platform/glib/TestExpectations b/LayoutTests/platform/glib/TestExpectations
@@ -3757,6 +3757,8 @@ webkit.org/b/266636 fast/multicol/last-set-crash.html [ Skip ]
 
 webkit.org/b/266708 imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/loading-the-media-resource/resource-selection-currentSrc.html [ Pass Crash ]
 
+webkit.org/b/266719 fast/canvas/offscreen-giant.html [ ImageOnlyFailure ]
+
 # End: Common failures between GTK and WPE.
 
 #////////////////////////////////////////////////////////////////////////////////////////
diff --git a/LayoutTests/platform/mac-monterey/TestExpectations b/LayoutTests/platform/mac-monterey/TestExpectations
@@ -7,3 +7,5 @@
 
 # Failing after OS migration rdar://112624778 (Migrate macOS Sonoma test expectations to OpenSource, add expectation files to Down-Levels (259373))
 http/tests/appcache/fail-on-update-2.html [ DumpJSConsoleLogInStdErr Timeout Failure ]
+
+fast/canvas/offscreen-giant.html [ ImageOnlyFailure ]
diff --git a/Source/WTF/wtf/unix/UnixFileDescriptor.h b/Source/WTF/wtf/unix/UnixFileDescriptor.h
@@ -34,7 +34,6 @@
 namespace WTF {
 
 class UnixFileDescriptor {
-    WTF_MAKE_NONCOPYABLE(UnixFileDescriptor);
 public:
     UnixFileDescriptor() = default;
 
@@ -55,6 +54,12 @@ class UnixFileDescriptor {
         m_value = o.release();
     }
 
+    explicit UnixFileDescriptor(const UnixFileDescriptor& o)
+    {
+        if (o.m_value >= 0)
+            m_value = dupCloseOnExec(o.m_value);
+    }
+
     UnixFileDescriptor& operator=(UnixFileDescriptor&& o)
     {
         if (&o == this)
diff --git a/Source/WebCore/platform/graphics/ca/cocoa/GraphicsLayerAsyncContentsDisplayDelegateCocoa.mm b/Source/WebCore/platform/graphics/ca/cocoa/GraphicsLayerAsyncContentsDisplayDelegateCocoa.mm
@@ -43,6 +43,8 @@
 bool GraphicsLayerAsyncContentsDisplayDelegateCocoa::tryCopyToLayer(ImageBuffer& image)
 {
     m_image = ImageBuffer::sinkIntoNativeImage(image.clone());
+    if (!m_image)
+        return false;
 
     [CATransaction begin];
     [CATransaction setDisableActions:YES];
diff --git a/Source/WebCore/platform/graphics/cocoa/DynamicContentScalingDisplayList.h b/Source/WebCore/platform/graphics/cocoa/DynamicContentScalingDisplayList.h
@@ -33,13 +33,13 @@
 namespace WebCore {
 
 class DynamicContentScalingDisplayList {
-    WTF_MAKE_NONCOPYABLE(DynamicContentScalingDisplayList);
 public:
     DynamicContentScalingDisplayList(Ref<WebCore::SharedBuffer> displayList, Vector<MachSendRight>&& surfaces)
         : m_displayList(WTFMove(displayList))
         , m_surfaces(WTFMove(surfaces))
     {
     }
+    explicit CGDisplayList(const CGDisplayList&) = default;
 
     DynamicContentScalingDisplayList(DynamicContentScalingDisplayList&&) = default;
     DynamicContentScalingDisplayList& operator=(DynamicContentScalingDisplayList&&) = default;
diff --git a/Source/WebKit/Platform/SharedMemory.h b/Source/WebKit/Platform/SharedMemory.h
@@ -60,7 +60,6 @@ namespace WebKit {
 enum class MemoryLedger { None, Default, Network, Media, Graphics, Neural };
 
 class SharedMemoryHandle {
-    WTF_MAKE_NONCOPYABLE(SharedMemoryHandle);
 public:
     using Type =
 #if USE(UNIX_DOMAIN_SOCKETS)
@@ -72,6 +71,7 @@ class SharedMemoryHandle {
 #endif
 
     SharedMemoryHandle(SharedMemoryHandle&&) = default;
+    explicit SharedMemoryHandle(const SharedMemoryHandle&) = default;
     SharedMemoryHandle(SharedMemoryHandle::Type&&, size_t);
 
     SharedMemoryHandle& operator=(SharedMemoryHandle&&) = default;
diff --git a/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.h b/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.h
@@ -54,6 +54,7 @@ class RemoteLayerBackingStoreCollection;
 class RemoteLayerTreeNode;
 class RemoteLayerTreeHost;
 enum class SwapBuffersDisplayRequirement : uint8_t;
+struct PlatformCALayerRemoteDelegatedContents;
 
 enum class BackingStoreNeedsDisplayReason : uint8_t {
     None,
@@ -102,7 +103,7 @@ class RemoteLayerBackingStore : public CanMakeWeakPtr<RemoteLayerBackingStore> {
     void setNeedsDisplay(const WebCore::IntRect);
     void setNeedsDisplay();
 
-    void setDelegatedContents(const WebCore::PlatformCALayerDelegatedContents&);
+    void setDelegatedContents(const PlatformCALayerRemoteDelegatedContents&);
 
     // Returns true if we need to encode the buffer.
     bool layerWillBeDisplayed();
@@ -178,7 +179,7 @@ class RemoteLayerBackingStore : public CanMakeWeakPtr<RemoteLayerBackingStore> {
 
     // FIXME: This should be removed and m_bufferHandle should be used to ref the buffer once ShareableBitmap::Handle
     // can be encoded multiple times. http://webkit.org/b/234169
-    std::optional<MachSendRight> m_contentsBufferHandle;
+    std::optional<ImageBufferBackendHandle> m_contentsBufferHandle;
     std::optional<WebCore::RenderingResourceIdentifier> m_contentsRenderingResourceIdentifier;
 
     Vector<std::unique_ptr<WebCore::ThreadSafeImageBufferFlusher>> m_frontBufferFlushers;
diff --git a/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm b/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm
@@ -161,7 +161,7 @@ static bool hasValue(const ImageBufferBackendHandle& backendHandle)
     std::optional<ImageBufferBackendHandle> handle;
     if (m_contentsBufferHandle) {
         ASSERT(m_parameters.type == Type::IOSurface);
-        handle = MachSendRight { *m_contentsBufferHandle };
+        handle = ImageBufferBackendHandle { *m_contentsBufferHandle };
     } else
         handle = frontBufferHandle();
 
@@ -311,9 +311,9 @@ static bool hasValue(const ImageBufferBackendHandle& backendHandle)
     return !m_parameters.isOpaque && !m_layer->owner()->platformCALayerShouldPaintUsingCompositeCopy();
 }
 
-void RemoteLayerBackingStore::setDelegatedContents(const WebCore::PlatformCALayerDelegatedContents& contents)
+void RemoteLayerBackingStore::setDelegatedContents(const PlatformCALayerRemoteDelegatedContents& contents)
 {
-    m_contentsBufferHandle = MachSendRight { contents.surface };
+    m_contentsBufferHandle = ImageBufferBackendHandle { contents.surface };
     if (contents.finishedFence)
         m_frontBufferFlushers.append(DelegatedContentsFenceFlusher::create(Ref { *contents.finishedFence }));
     if (contents.surfaceIdentifier)
@@ -478,7 +478,7 @@ static bool hasValue(const ImageBufferBackendHandle& backendHandle)
     RetainPtr<id> contents;
     WTF::switchOn(backendHandle,
         [&] (ShareableBitmap::Handle& handle) {
-            if (auto bitmap = ShareableBitmap::create(WTFMove(handle)))
+            if (auto bitmap = ShareableBitmap::create(WTFMove(handle), SharedMemory::Protection::ReadOnly))
                 contents = bridge_id_cast(bitmap->makeCGImageCopy());
         },
         [&] (MachSendRight& machSendRight) {
diff --git a/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerWithInProcessRenderingBackingStore.mm b/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerWithInProcessRenderingBackingStore.mm
@@ -71,7 +71,7 @@
 {
     auto* sharing = buffer.toBackendSharing();
     if (is<ImageBufferBackendHandleSharing>(sharing))
-        return downcast<ImageBufferBackendHandleSharing>(*sharing).takeBackendHandle();
+        return downcast<ImageBufferBackendHandleSharing>(*sharing).takeBackendHandle(SharedMemory::Protection::ReadOnly);
 
     return std::nullopt;
 }
diff --git a/Source/WebKit/Shared/ShareableBitmap.h b/Source/WebKit/Shared/ShareableBitmap.h
@@ -95,9 +95,9 @@ class ShareableBitmapConfiguration {
 };
 
 class ShareableBitmapHandle  {
-    WTF_MAKE_NONCOPYABLE(ShareableBitmapHandle);
 public:
     ShareableBitmapHandle(ShareableBitmapHandle&&) = default;
+    explicit ShareableBitmapHandle(const ShareableBitmapHandle&) = default;
     ShareableBitmapHandle(SharedMemory::Handle&&, const ShareableBitmapConfiguration&);
 
     ShareableBitmapHandle& operator=(ShareableBitmapHandle&&) = default;
diff --git a/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.mm b/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.mm
@@ -135,11 +135,11 @@ bool tryCopyToLayer(ImageBuffer& buffer) final
             return false;
 
         auto backendHandle = sharing->createBackendHandle(SharedMemory::Protection::ReadOnly);
-        ASSERT(backendHandle && std::holds_alternative<MachSendRight>(*backendHandle));
+        ASSERT(backendHandle);
 
         {
             Locker locker { m_surfaceLock };
-            m_surfaceSendRight = MachSendRight { std::get<MachSendRight>(*backendHandle) };
+            m_surfaceBackendHandle = ImageBufferBackendHandle { *backendHandle };
             m_surfaceIdentifier = clone->renderingResourceIdentifier();
         }
 
@@ -151,8 +151,8 @@ bool tryCopyToLayer(ImageBuffer& buffer) final
     void display(PlatformCALayer& layer) final
     {
         Locker locker { m_surfaceLock };
-        if (m_surfaceSendRight)
-            layer.setDelegatedContents({ MachSendRight { *m_surfaceSendRight }, { }, std::optional<RenderingResourceIdentifier>(m_surfaceIdentifier) });
+        if (m_surfaceBackendHandle)
+            downcast<PlatformCALayerRemote>(layer).setRemoteDelegatedContents({ ImageBufferBackendHandle { *m_surfaceBackendHandle }, { }, std::optional<RenderingResourceIdentifier>(m_surfaceIdentifier) });
     }
 
     void setDestinationLayerID(WebCore::PlatformLayerIdentifier layerID)
@@ -167,7 +167,7 @@ void setDestinationLayerID(WebCore::PlatformLayerIdentifier layerID)
     DrawingAreaIdentifier m_drawingArea;
     WebCore::PlatformLayerIdentifier m_layerID;
     Lock m_surfaceLock;
-    std::optional<MachSendRight> m_surfaceSendRight WTF_GUARDED_BY_LOCK(m_surfaceLock);
+    std::optional<ImageBufferBackendHandle> m_surfaceBackendHandle WTF_GUARDED_BY_LOCK(m_surfaceLock);
     WebCore::RenderingResourceIdentifier m_surfaceIdentifier WTF_GUARDED_BY_LOCK(m_surfaceLock);
 };
 
diff --git a/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.h b/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.h
@@ -29,6 +29,7 @@
 #include "RemoteLayerTreeTransaction.h"
 #include <WebCore/HTMLMediaElementIdentifier.h>
 #include <WebCore/PlatformCALayer.h>
+#include <WebCore/PlatformCALayerDelegatedContents.h>
 #include <WebCore/PlatformLayer.h>
 #include <wtf/WeakPtr.h>
 
@@ -44,6 +45,12 @@ namespace WebKit {
 
 class RemoteLayerTreeContext;
 
+struct PlatformCALayerRemoteDelegatedContents {
+    ImageBufferBackendHandle surface;
+    RefPtr<WebCore::PlatformCALayerDelegatedContentsFence> finishedFence;
+    std::optional<WebCore::RenderingResourceIdentifier> surfaceIdentifier;
+};
+
 class PlatformCALayerRemote : public WebCore::PlatformCALayer, public CanMakeWeakPtr<PlatformCALayerRemote> {
 public:
     static Ref<PlatformCALayerRemote> create(WebCore::PlatformCALayer::LayerType, WebCore::PlatformCALayerClient*, RemoteLayerTreeContext&);
@@ -146,6 +153,7 @@ class PlatformCALayerRemote : public WebCore::PlatformCALayer, public CanMakeWea
     CFTypeRef contents() const override;
     void setContents(CFTypeRef) override;
     void setDelegatedContents(const WebCore::PlatformCALayerDelegatedContents&) override;
+    void setRemoteDelegatedContents(const PlatformCALayerRemoteDelegatedContents&);
     void setContentsRect(const WebCore::FloatRect&) override;
 
     void setMinificationFilter(WebCore::PlatformCALayer::FilterType) override;
diff --git a/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.mm b/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.mm
@@ -742,6 +742,11 @@ static inline bool isEquivalentLayer(const PlatformCALayer* layer, const std::op
 }
 
 void PlatformCALayerRemote::setDelegatedContents(const PlatformCALayerDelegatedContents& contents)
+{
+    setRemoteDelegatedContents({ ImageBufferBackendHandle { MachSendRight { contents.surface } }, contents.finishedFence, contents.surfaceIdentifier });
+}
+
+void PlatformCALayerRemote::setRemoteDelegatedContents(const PlatformCALayerRemoteDelegatedContents& contents)
 {
     ASSERT(m_acceleratesDrawing);
     ensureBackingStore();

Original file line number	Diff line number	Diff line change
`@@ -43,6 +43,8 @@`
`43`	`43`	`bool GraphicsLayerAsyncContentsDisplayDelegateCocoa::tryCopyToLayer(ImageBuffer& image)`
`44`	`44`	`{`
`45`	`45`	`m_image = ImageBuffer::sinkIntoNativeImage(image.clone());`
	`46`	`+ if (!m_image)`
	`47`	`+ return false;`
`46`	`48`
`47`	`49`	`[CATransaction begin];`
`48`	`50`	`[CATransaction setDisableActions:YES];`
Original file line number	Diff line number	Diff line change
`@@ -33,13 +33,13 @@`
`33`	`33`	`namespace WebCore {`
`34`	`34`
`35`	`35`	`class DynamicContentScalingDisplayList {`
`36`		`- WTF_MAKE_NONCOPYABLE(DynamicContentScalingDisplayList);`
`37`	`36`	`public:`
`38`	`37`	`DynamicContentScalingDisplayList(Ref<WebCore::SharedBuffer> displayList, Vector<MachSendRight>&& surfaces)`
`39`	`38`	`: m_displayList(WTFMove(displayList))`
`40`	`39`	`, m_surfaces(WTFMove(surfaces))`
`41`	`40`	`{`
`42`	`41`	`}`
	`42`	`+ explicit CGDisplayList(const CGDisplayList&) = default;`
`43`	`43`
`44`	`44`	`DynamicContentScalingDisplayList(DynamicContentScalingDisplayList&&) = default;`
`45`	`45`	`DynamicContentScalingDisplayList& operator=(DynamicContentScalingDisplayList&&) = default;`