On 28/7/26 17:14, [email protected] wrote:
From: Marc-André Lureau <[email protected]>
The fields last_width and last_height serve two purposes: the text
renderer counts in characters, the graphics renderer in pixels.
panning_buf reallocation is guarded by geometry-change check, so the
unit mismatch can trick it into thinking nothing changed when the
resolution actually grew.
A guest can trigger this by switching graphics -> text -> graphics:
1. Enter graphics mode with a small width (CR01=0x00, 8 pixels).
The predicate fires and panning_buf is allocated for that width.
2. Switch to text mode with a large width (CR01=0xFF, 256 chars).
The text renderer stores 256 into last_width. The text path
never touches panning_buf.
3. Switch back to graphics with a width that happens to equal 256
in pixels (CR01=0x1F, 32*8 = 256). The predicate sees
256 == 256 and skips the realloc. With horizontal pel panning
enabled, vga_draw_line4() then writes a full 256-pixel scanline
into the buffer still sized for 8 pixels -- a 960-byte heap
overflow on every scanline, every refresh.
Fix it by reallocating unconditionally panning_buf on
vga_draw_graphic().
Fixes: CVE-2026-17516
Fixes: 973a724eb006 ("vga: implement horizontal pel panning in graphics modes")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4085
Cc: Paolo Bonzini <[email protected]>
Signed-off-by: Warisjeet Singh <[email protected]>
[ Marc- André - drop realloc() resize condition & commit message ]
Signed-off-by: Marc-André Lureau <[email protected]>
---
hw/display/vga.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/display/vga.c b/hw/display/vga.c
index abe3f8e07758..da0c331486eb 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1647,11 +1647,12 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
s->last_line_offset = s->params.line_offset;
s->last_depth = depth;
s->last_byteswap = byteswap;
- /* 16 extra pixels are needed for double-width planar modes. */
- s->panning_buf = g_realloc(s->panning_buf,
- (disp_width + 16) * sizeof(uint32_t));
full_update = 1;
}
+
+ /* 16 extra pixels are needed for double-width planar modes. */
+ s->panning_buf = g_realloc(s->panning_buf,
+ (disp_width + 16) * sizeof(uint32_t));
if (surface_data(surface) != s->vram_ptr + (s->params.start_addr * 4)
&& !surface_is_allocated(surface)) {
/* base address changed (page flip) -> shared display surfaces
This function body is huge.
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Orthogonal but since reviewing, should panning_buf be declared as
uint32_t*? We could then call:
s->panning_buf = g_renew(uint32_t *, s->panning_buf, disp_width + 16);
and
return hpel ? &vga->panning_buf[n * hpel] : NULL;
