Fix heap overrun in texture readback for the model transparency scan#7616
Merged
Conversation
gr_opengl_get_bitmap_from_texture() sized its readback buffer from the texture cache slot dimensions and the bmpman frame count, but glGetTexImage() returns whatever the bound GL texture actually contains. When texture detail below maximum culls textures on upload, the two can disagree, and the readback overruns the buffer (observed as heap corruption when loading Zeus.pof with Texture Detail: Minimum). Size the readback from the dimensions and layer count OpenGL itself reports for the texture, and pad the staging buffer to 4x4 block granularity in case a driver writes tiny compressed mips back in whole blocks. The readback now also allocates the returned buffer and reports the dimensions of the data it returns, because the second half of the bug was on the consumer side: bitmap_lookup indexed the readback data by the bitmap's own dimensions, so with any culling in effect the transparency scan read mostly uninitialized memory. bitmap_lookup (and its cache) now carries the actual readback dimensions, and it skips the texture cleanly if the readback fails. Also pairs the cache's vm_malloc allocations with vm_free instead of free, and no longer ignores a failed tcache_set before reading back. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Goober5000
added
fix
Return fully opaque from bitmap_lookup::get_channel_alpha() when the bitmap has no alpha channel, since indexing channel 3 of a 3-channel buffer reads out of bounds (currently unreachable because the only caller checks bm_has_alpha_channel() first, but no longer a trap). Per review, also assert at maximum texture detail, where no culling occurs on upload, that the readback dimensions OpenGL reports match the texture cache slot, so any future slot mismatch is caught immediately in debug builds instead of surfacing as corruption. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BMagnu
approved these changes
Jul 18, 2026
The validation rejected the export only if both the width and the height mismatched the texture cache slot, so a texture wrong in one dimension passed through into opengl_get_texture, which trusts the caller-supplied dimensions for its mipmap loop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Goober5000
added
the
Point Release Candidate
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
gr_opengl_get_bitmap_from_texture() sized its readback buffer from the texture cache slot dimensions and the bmpman frame count, but glGetTexImage() returns whatever the bound GL texture actually contains. When texture detail below maximum culls textures on upload, the two can disagree, and the readback overruns the buffer (observed as heap corruption when loading Zeus.pof with Texture Detail: Minimum). Size the readback from the dimensions and layer count OpenGL itself reports for the texture, and pad the staging buffer to 4x4 block granularity in case a driver writes tiny compressed mips back in whole blocks.
The readback now also allocates the returned buffer and reports the dimensions of the data it returns, because the second half of the bug was on the consumer side: bitmap_lookup indexed the readback data by the bitmap's own dimensions, so with any culling in effect the transparency scan read mostly uninitialized memory. bitmap_lookup (and its cache) now carries the actual readback dimensions, and it skips the texture cleanly if the readback fails.
Also pairs the cache's vm_malloc allocations with vm_free instead of free, and no longer ignores a failed tcache_set before reading back.