Skip to content

[GTK][WPE] MemoryMappedGPUBuffer::mapIfNeeded() does not handle mmap() errors properly #47111

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

Conversation

nikolaszimmermann
Copy link
Contributor

@nikolaszimmermann nikolaszimmermann commented Jun 24, 2025

2111c09

[GTK][WPE] MemoryMappedGPUBuffer::mapIfNeeded() does not handle mmap() errors properly
https://bugs.webkit.org/show_bug.cgi?id=294896

Reviewed by Carlos Garcia Campos.

If mmap() fails MAP_FAILED is returned, which is a special constant
(e.g. (void*)-1, 0xfffff...) indicating the error. We currently return
false from mapIfNeeded() to indicate that the mapping failed, but later
on check if the mapping succeded, by invoking `isMapped()`, which checks
if m_mappedData is non-null, which is the case even if mmap() failed, as
we incorrectly store the special constant in m_mappedData -- the address
of the memory-mapped region. Fix that problem by properly resetting
m_mappedData to nullptr, if mmap() failed.

Covered by existing tests -- they currently crash if mmap() mode fails
instead of falling back to OpenGL texture updates.

* Source/WebCore/platform/graphics/gbm/MemoryMappedGPUBuffer.cpp:
(WebCore::MemoryMappedGPUBuffer::mapIfNeeded):

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

e1735f6

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@nikolaszimmermann nikolaszimmermann self-assigned this Jun 24, 2025
@nikolaszimmermann nikolaszimmermann added the WPE WebKit WebKit WPE component label Jun 24, 2025
@nikolaszimmermann nikolaszimmermann added the merge-queue Applied to send a pull request to merge-queue label Jun 24, 2025
…) errors properly

https://bugs.webkit.org/show_bug.cgi?id=294896

Reviewed by Carlos Garcia Campos.

If mmap() fails MAP_FAILED is returned, which is a special constant
(e.g. (void*)-1, 0xfffff...) indicating the error. We currently return
false from mapIfNeeded() to indicate that the mapping failed, but later
on check if the mapping succeded, by invoking `isMapped()`, which checks
if m_mappedData is non-null, which is the case even if mmap() failed, as
we incorrectly store the special constant in m_mappedData -- the address
of the memory-mapped region. Fix that problem by properly resetting
m_mappedData to nullptr, if mmap() failed.

Covered by existing tests -- they currently crash if mmap() mode fails
instead of falling back to OpenGL texture updates.

* Source/WebCore/platform/graphics/gbm/MemoryMappedGPUBuffer.cpp:
(WebCore::MemoryMappedGPUBuffer::mapIfNeeded):

Canonical link: https://commits.webkit.org/296562@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/GTK-WPE-MemoryMappedGPUBuffer-mapIfNeeded-does-not-handle-mmap-errors-properly branch from e1735f6 to 2111c09 Compare June 24, 2025 15:01
@webkit-commit-queue
Copy link
Collaborator

Committed 296562@main (2111c09): https://commits.webkit.org/296562@main

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

@webkit-commit-queue webkit-commit-queue merged commit 2111c09 into WebKit:main Jun 24, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WPE WebKit WebKit WPE component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants