Skip to content

vulkan: Use 64-bit arithmetic for scaled readback buffer sizes#149

Merged
jmacnak merged 1 commit into
google:mainfrom
utzcoz:64-bit-arithmetic
Jun 23, 2026
Merged

vulkan: Use 64-bit arithmetic for scaled readback buffer sizes#149
jmacnak merged 1 commit into
google:mainfrom
utzcoz:64-bit-arithmetic

Conversation

@utzcoz

@utzcoz utzcoz commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

readColorBufferPixelsScaledCpu() and ResizeRGBAImage() computed pixel buffer sizes and indices as 'width * height * bpp' in 32-bit int before widening to the uint64_t/size_t they are stored in. The dimensions here come from the host screenshot path rather than the guest, so this is a robustness fix rather than a guest-reachable overflow, but the mixed arithmetic is inconsistent with the explicit (uint64_t) cast already used a few lines up at the simple-readback size computation.

Perform these multiplications in 64-bit: cast the scaled output size and the resize allocation to 64-bit, and widen ResizeRGBAImage's pixel index helper and index variables to int64_t so the index math cannot overflow independently of the allocation.

Test: bazel build //host/vulkan:gfxstream_vulkan_server

@utzcoz

utzcoz commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Hi @jmacnak PTAL. I think I will change the API structure a lot, so I didn't add unit tests for it.

@jmacnak jmacnak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall LGTM, 2 minor nits

Comment thread host/vulkan/vk_common_operations.cpp Outdated
Comment thread host/vulkan/vk_common_operations.cpp Outdated
readColorBufferPixelsScaledCpu() and ResizeRGBAImage() computed pixel
buffer sizes and indices as 'width * height * bpp' in 32-bit int before
widening to the uint64_t/size_t they are stored in. The dimensions here
come from the host screenshot path rather than the guest, so this is a
robustness fix rather than a guest-reachable overflow, but the mixed
arithmetic is inconsistent with the explicit (uint64_t) cast already
used a few lines up at the simple-readback size computation.

Perform these multiplications in 64-bit: cast the scaled output size and
the resize allocation to 64-bit, and widen ResizeRGBAImage's pixel index
helper and index variables to int64_t so the index math cannot overflow
independently of the allocation.

Test: bazel build //host/vulkan:gfxstream_vulkan_server

Change-Id: I71c06bf932ad5ee598b907bfb5daf46dde69f9c0
@utzcoz utzcoz force-pushed the 64-bit-arithmetic branch from ce859cb to 904aba6 Compare June 23, 2026 14:50
@utzcoz

utzcoz commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

@jmacnak Updated. PTAL.

@jmacnak jmacnak enabled auto-merge June 23, 2026 15:00
@jmacnak jmacnak added this pull request to the merge queue Jun 23, 2026
Merged via the queue into google:main with commit 74bb51f Jun 23, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants