[WebGPU] UAF in GPUBuffer::getMappedRange

https://bugs.webkit.org/show_bug.cgi?id=273685 <radar://127490690> Reviewed by Dan Glastonbury. Fix UAF by using a callback and update test expectations to run in Debug which would have likely caught this issue. * LayoutTests/platform/mac-wk2/TestExpectations: * Source/WebCore/Modules/WebGPU/GPUBuffer.cpp: (WebCore::GPUBuffer::getMappedRange): * Source/WebCore/Modules/WebGPU/Implementation/WebGPUBufferImpl.cpp: (WebCore::WebGPU::BufferImpl::getMappedRange): * Source/WebCore/Modules/WebGPU/Implementation/WebGPUBufferImpl.h: * Source/WebCore/Modules/WebGPU/InternalAPI/WebGPUBuffer.h: * Source/WebKit/WebProcess/GPU/graphics/WebGPU/RemoteBufferProxy.cpp: (WebKit::WebGPU::RemoteBufferProxy::getMappedRange): * Source/WebKit/WebProcess/GPU/graphics/WebGPU/RemoteBufferProxy.h: Canonical link: https://commits.webkit.org/278392@main
WebKit · May 6, 2024 · cf23d24 · cf23d24
1 parent 9764163
commit cf23d24
Show file tree

Hide file tree

Showing 8 changed files with 37 additions and 26 deletions.
diff --git a/LayoutTests/platform/mac-wk2/TestExpectations b/LayoutTests/platform/mac-wk2/TestExpectations
@@ -1783,15 +1783,16 @@ webkit.org/b/263396 css3/color/text.html [ Skip ]
 
 # WebGPU
 
-[ Release ] http/tests/webgpu/webgpu/api/operation/buffers/map.html [ Pass ]
+http/tests/webgpu/webgpu/api/operation/buffers/map.html [ Pass ]
+# skipped debug due to https://bugs.webkit.org/show_bug.cgi?id=273716
 [ Release ] http/tests/webgpu/webgpu/api/operation/buffers/map_ArrayBuffer.html [ Pass ]
-[ Release ] http/tests/webgpu/webgpu/api/operation/buffers/map_detach.html [ Pass ]
-[ Release ] http/tests/webgpu/webgpu/api/operation/buffers/map_oom.html [ Pass ]
-[ Release ] http/tests/webgpu/webgpu/api/operation/buffers/threading.html [ Pass ]
+http/tests/webgpu/webgpu/api/operation/buffers/map_detach.html [ Pass ]
+http/tests/webgpu/webgpu/api/operation/buffers/map_oom.html [ Pass ]
+http/tests/webgpu/webgpu/api/operation/buffers/threading.html [ Pass ]
 
-[ Release ] http/tests/webgpu/webgpu/api/validation/buffer/create.html [ Pass ]
-[ Release ] http/tests/webgpu/webgpu/api/validation/buffer/destroy.html [ Pass ]
-[ Release ] http/tests/webgpu/webgpu/api/validation/buffer/mapping.html [ Pass ]
+http/tests/webgpu/webgpu/api/validation/buffer/create.html [ Pass ]
+http/tests/webgpu/webgpu/api/validation/buffer/destroy.html [ Pass ]
+http/tests/webgpu/webgpu/api/validation/buffer/mapping.html [ Pass ]
 
 # Skip tests until they can be validated to consistently pass in EWS
 http/tests/webgpu/webgpu/idl/constructable.html [ Skip ]

diff --git a/Source/WebCore/Modules/WebGPU/GPUBuffer.cpp b/Source/WebCore/Modules/WebGPU/GPUBuffer.cpp
@@ -171,16 +171,23 @@ ExceptionOr<Ref<JSC::ArrayBuffer>> GPUBuffer::getMappedRange(std::optional<GPUSi
         m_mappedRanges.compact();
     }
 
-    auto mappedRange = m_backing->getMappedRange(offset, size);
-    if (!mappedRange.source) {
-        m_arrayBuffers.clear();
-        if (m_mappedAtCreation || !size)
-            return makeArrayBuffer(static_cast<size_t>(0U), 0, 1, m_arrayBuffers, m_device, *this);
+    RefPtr<JSC::ArrayBuffer> result;
+    m_backing->getMappedRange(offset, size, [&] (auto mappedRange) {
+        if (!mappedRange.source) {
+            m_arrayBuffers.clear();
+            if (m_mappedAtCreation || !size)
+                result = makeArrayBuffer(static_cast<size_t>(0U), 0, 1, m_arrayBuffers, m_device, *this);
 
+            return;
+        }
+
+        result = makeArrayBuffer(mappedRange.source, offset, size, m_arrayBuffers, m_device, *this);
+    });
+
+    if (!result)
         return Exception { ExceptionCode::OperationError, "getMappedRange failed"_s };
-    }
 
-    return makeArrayBuffer(mappedRange.source, offset, size, m_arrayBuffers, m_device, *this);
+    return result.releaseNonNull();
 }
 
 void GPUBuffer::unmap(ScriptExecutionContext& scriptExecutionContext)

diff --git a/Source/WebCore/Modules/WebGPU/Implementation/WebGPUBufferImpl.cpp b/Source/WebCore/Modules/WebGPU/Implementation/WebGPUBufferImpl.cpp
@@ -70,7 +70,7 @@ void BufferImpl::mapAsync(MapModeFlags mapModeFlags, Size64 offset, std::optiona
     wgpuBufferMapAsync(m_backing.get(), backingMapModeFlags, static_cast<size_t>(offset), static_cast<size_t>(usedSize), &mapAsyncCallback, Block_copy(blockPtr.get())); // Block_copy is matched with Block_release above in mapAsyncCallback().
 }
 
-auto BufferImpl::getMappedRange(Size64 offset, std::optional<Size64> size) -> MappedRange
+void BufferImpl::getMappedRange(Size64 offset, std::optional<Size64> size, Function<void(MappedRange)>&& callback)
 {
     auto usedSize = getMappedSize(m_backing.get(), size, offset);
 
@@ -80,7 +80,7 @@ auto BufferImpl::getMappedRange(Size64 offset, std::optional<Size64> size) -> Ma
     auto bufferSize = wgpuBufferGetSize(m_backing.get());
     size_t actualSize = pointer ? static_cast<size_t>(bufferSize) : 0;
     size_t actualOffset = pointer ? static_cast<size_t>(offset) : 0;
-    return { static_cast<uint8_t*>(pointer) - actualOffset, actualSize };
+    callback({ static_cast<uint8_t*>(pointer) - actualOffset, actualSize });
 }
 
 auto BufferImpl::getBufferContents() -> MappedRange

diff --git a/Source/WebCore/Modules/WebGPU/Implementation/WebGPUBufferImpl.h b/Source/WebCore/Modules/WebGPU/Implementation/WebGPUBufferImpl.h
@@ -59,7 +59,7 @@ class BufferImpl final : public Buffer {
     WGPUBuffer backing() const { return m_backing.get(); }
 
     void mapAsync(MapModeFlags, Size64 offset, std::optional<Size64> sizeForMap, CompletionHandler<void(bool)>&&) final;
-    MappedRange getMappedRange(Size64 offset, std::optional<Size64>) final;
+    void getMappedRange(Size64 offset, std::optional<Size64>, Function<void(MappedRange)>&&) final;
     MappedRange getBufferContents() final;
     void unmap() final;
     void copy(Vector<uint8_t>&&, size_t) final;

diff --git a/Source/WebCore/Modules/WebGPU/InternalAPI/WebGPUBuffer.h b/Source/WebCore/Modules/WebGPU/InternalAPI/WebGPUBuffer.h
@@ -54,7 +54,7 @@ class Buffer : public RefCounted<Buffer>, public CanMakeWeakPtr<Buffer> {
         uint8_t* source { nullptr };
         size_t byteLength { 0 };
     };
-    virtual MappedRange getMappedRange(Size64 offset, std::optional<Size64>) = 0;
+    virtual void getMappedRange(Size64 offset, std::optional<Size64>, Function<void(MappedRange)>&&) = 0;
     virtual void unmap() = 0;
 
     virtual void destroy() = 0;

diff --git a/Source/WebKit/GPUProcess/graphics/WebGPU/RemoteBuffer.cpp b/Source/WebKit/GPUProcess/graphics/WebGPU/RemoteBuffer.cpp
@@ -71,10 +71,11 @@ void RemoteBuffer::mapAsync(WebCore::WebGPU::MapModeFlags mapModeFlags, WebCore:
 
 void RemoteBuffer::getMappedRange(WebCore::WebGPU::Size64 offset, std::optional<WebCore::WebGPU::Size64> size, CompletionHandler<void(std::optional<Vector<uint8_t>>&&)>&& callback)
 {
-    auto mappedRange = m_backing->getMappedRange(offset, size);
-    m_isMapped = true;
+    m_backing->getMappedRange(offset, size, [&] (auto mappedRange) {
+        m_isMapped = true;
 
-    callback(Vector(std::span { static_cast<const uint8_t*>(mappedRange.source), mappedRange.byteLength }));
+        callback(Vector(std::span { static_cast<const uint8_t*>(mappedRange.source), mappedRange.byteLength }));
+    });
 }
 
 void RemoteBuffer::unmap()

diff --git a/Source/WebKit/WebProcess/GPU/graphics/WebGPU/RemoteBufferProxy.cpp b/Source/WebKit/WebProcess/GPU/graphics/WebGPU/RemoteBufferProxy.cpp
@@ -63,16 +63,18 @@ static bool offsetOrSizeExceedsBounds(size_t dataSize, WebCore::WebGPU::Size64 o
     return offset >= dataSize || (requestedSize.has_value() && requestedSize.value() + offset > dataSize);
 }
 
-auto RemoteBufferProxy::getMappedRange(WebCore::WebGPU::Size64 offset, std::optional<WebCore::WebGPU::Size64> size) -> MappedRange
+void RemoteBufferProxy::getMappedRange(WebCore::WebGPU::Size64 offset, std::optional<WebCore::WebGPU::Size64> size, Function<void(MappedRange)>&& callback)
 {
     // FIXME: Implement error handling.
     auto sendResult = sendSync(Messages::RemoteBuffer::GetMappedRange(offset, size));
     auto [data] = sendResult.takeReplyOr(std::nullopt);
 
-    if (!data || !data->data() || offsetOrSizeExceedsBounds(data->size(), offset, size))
-        return { };
+    if (!data || !data->data() || offsetOrSizeExceedsBounds(data->size(), offset, size)) {
+        callback({ });
+        return;
+    }
 
-    return { data->data() + offset, static_cast<size_t>(size.value_or(data->size() - offset)) };
+    callback({ data->data() + offset, static_cast<size_t>(size.value_or(data->size() - offset)) });
 }
 
 auto RemoteBufferProxy::getBufferContents() -> MappedRange

diff --git a/Source/WebKit/WebProcess/GPU/graphics/WebGPU/RemoteBufferProxy.h b/Source/WebKit/WebProcess/GPU/graphics/WebGPU/RemoteBufferProxy.h
@@ -79,7 +79,7 @@ class RemoteBufferProxy final : public WebCore::WebGPU::Buffer {
     }
 
     void mapAsync(WebCore::WebGPU::MapModeFlags, WebCore::WebGPU::Size64 offset, std::optional<WebCore::WebGPU::Size64> sizeForMap, CompletionHandler<void(bool)>&&) final;
-    MappedRange getMappedRange(WebCore::WebGPU::Size64 offset, std::optional<WebCore::WebGPU::Size64>) final;
+    void getMappedRange(WebCore::WebGPU::Size64 offset, std::optional<WebCore::WebGPU::Size64>, Function<void(MappedRange)>&&) final;
     MappedRange getBufferContents() final;
     void unmap() final;
     void copy(Vector<uint8_t>&&, size_t offset) final;