Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WebGPU] RemoteBuffer::copy can result in an out of bounds write #29086

Conversation

mwyrzykowski
Copy link
Contributor

@mwyrzykowski mwyrzykowski commented May 24, 2024

0fa174e

[WebGPU] RemoteBuffer::copy can result in an out of bounds write
https://bugs.webkit.org/show_bug.cgi?id=274678
<radar://128591350>

Reviewed by Tadeu Zagallo.

RemoteBuffer::copy should use the actual buffer size, not the initial size.

* Source/WebCore/Modules/WebGPU/Implementation/WebGPUBufferImpl.cpp:
(WebCore::WebGPU::BufferImpl::getBufferContents):
* Source/WebGPU/WebGPU/Buffer.mm:
(wgpuBufferGetActualSize):
* Source/WebGPU/WebGPU/WebGPU.h:

* LayoutTests/fast/webgpu/regression/repro_274678-expected.txt: Added.
* LayoutTests/fast/webgpu/regression/repro_274678.html: Added.
Add regression test for running locally.

Canonical link: https://commits.webkit.org/279368@main

48d31a6

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@mwyrzykowski mwyrzykowski self-assigned this May 24, 2024
@mwyrzykowski mwyrzykowski added the WebGPU For bugs in WebGPU label May 24, 2024
@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-RemoteBuffercopy-can-result-in-an-out-of-bounds-write branch from f69b08c to 8a69264 Compare May 24, 2024 21:47
@mwyrzykowski mwyrzykowski requested a review from djg May 24, 2024 22:50
@@ -89,7 +89,7 @@ auto BufferImpl::getBufferContents() -> MappedRange
return { nullptr, 0 };

auto* pointer = wgpuBufferGetBufferContents(m_backing.get());
auto bufferSize = wgpuBufferGetSize(m_backing.get());
auto bufferSize = wgpuBufferGetActualSize(m_backing.get());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be clearer if these were called wgpuBufferGetInitialSize and wgpuBufferGetCurrentSize?

@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-RemoteBuffercopy-can-result-in-an-out-of-bounds-write branch from 8a69264 to 48d31a6 Compare May 28, 2024 04:31
@mwyrzykowski mwyrzykowski added the merge-queue Applied to send a pull request to merge-queue label May 28, 2024
@webkit-commit-queue webkit-commit-queue force-pushed the eng/WebGPU-RemoteBuffercopy-can-result-in-an-out-of-bounds-write branch from 48d31a6 to ef994b7 Compare May 28, 2024 06:53
https://bugs.webkit.org/show_bug.cgi?id=274678
<radar://128591350>

Reviewed by Tadeu Zagallo.

RemoteBuffer::copy should use the actual buffer size, not the initial size.

* Source/WebCore/Modules/WebGPU/Implementation/WebGPUBufferImpl.cpp:
(WebCore::WebGPU::BufferImpl::getBufferContents):
* Source/WebGPU/WebGPU/Buffer.mm:
(wgpuBufferGetActualSize):
* Source/WebGPU/WebGPU/WebGPU.h:

* LayoutTests/fast/webgpu/regression/repro_274678-expected.txt: Added.
* LayoutTests/fast/webgpu/regression/repro_274678.html: Added.
Add regression test for running locally.

Canonical link: https://commits.webkit.org/279368@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/WebGPU-RemoteBuffercopy-can-result-in-an-out-of-bounds-write branch from ef994b7 to 0fa174e Compare May 28, 2024 06:54
@webkit-commit-queue
Copy link
Collaborator

Committed 279368@main (0fa174e): https://commits.webkit.org/279368@main

Reviewed commits have been landed. Closing PR #29086 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 0fa174e into WebKit:main May 28, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebGPU For bugs in WebGPU
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants