On Mon, Aug 24, 2026 at 8:47 PM Warisjeet Singh <[email protected]> wrote: > > vga_draw_text() decides whether the console surface needs a resize from > its geometry cache, but none of the cache terms observe the graphics > renderer having replaced the console surface in between: > > - last_width/last_height are shared with vga_draw_graphic(), which > stores them in pixels while the text path stores characters; > - last_depth stays 0 for legacy (non-VBE) graphics modes, because > vga_get_bpp() only reports a depth when VBE is enabled, so the > "s->last_depth" term that normally forces a resize after a graphics > frame does not fire. > > So a graphics frame that shrinks the console surface (e.g. 80x25 > pixels) followed by a text frame with matching character geometry > (80x25 chars) skips the resize, and the glyph loop then paints > width*cw x height*cheight pixels into the smaller surface, out of > bounds, with guest-controlled (DAC palette) values, on every display > refresh. > > Separate the geometry cache per renderer: text paths (vga_draw_text, > vga_update_text, and the text handling in vga_invalidate_display / > vga_common_reset) now only manipulate last_text_{width,height}, in > characters; last_{width,height} become graphics-only, in pixels. > Additionally, make the text path compare the pixel size it is about > to paint against the console surface's actual dimensions. The > surface check is the load-bearing term: caches in either unit cannot > see the other renderer swapping the surface, the surface can. > > Fixes: CVE-2026-77913 > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4215 > Cc: [email protected] > Signed-off-by: Warisjeet Singh (sin99xx) <[email protected]> > --- > Changes v2 -> v3: > - Finish the gfx/text state separation, as requested: vga_update_text() > (predicate, cache stores, and the mode-message box) and the > last_width == -1 invalidate handshake now use last_text_{width,height} > too, so the text paths never touch the graphics cache fields. > > Changes v1 -> v2: > - v1 (split caches only) did not fix the reported reproducer: after a > legacy graphics frame (depth 0) the text predicate still compared > equal, because nothing in it noticed the console surface had been > replaced. Keep the split for clarity, and add the surface-size > check which actually catches it (verified with the qtest PoC and an > ASAN build; without this patch ASAN reports a heap-buffer-overflow > in vga_draw_glyph9(), with it the run is clean). > --- > hw/display/vga.c | 37 ++++++++++++++++++++++--------------- > hw/display/vga_int.h | 3 ++- > 2 files changed, 24 insertions(+), 16 deletions(-)
Reviewed-by: Marc-André Lureau <[email protected]> > > diff --git a/hw/display/vga.c b/hw/display/vga.c > index da0c331486..cb0e28b79b 100644 > --- a/hw/display/vga.c > +++ b/hw/display/vga.c > @@ -1241,7 +1241,10 @@ static void vga_draw_text(VGACommonState *s, int > full_update) > return; > } > > - if (width != s->last_width || height != s->last_height || > + if (surface == NULL || > + surface_width(surface) != width * cw || > + surface_height(surface) != height * cheight || > + width != s->last_text_width || height != s->last_text_height || > cw != s->last_cw || cheight != s->last_ch || s->last_depth) { > s->last_scr_width = width * cw; > s->last_scr_height = height * cheight; > @@ -1249,8 +1252,8 @@ static void vga_draw_text(VGACommonState *s, int > full_update) > surface = qemu_console_surface(s->con); > qemu_console_text_resize(s->con, width, height); > s->last_depth = 0; > - s->last_width = width; > - s->last_height = height; > + s->last_text_width = width; > + s->last_text_height = height; > s->last_ch = cheight; > s->last_cw = cw; > full_update = 1; > @@ -1845,6 +1848,8 @@ static void vga_invalidate_display(void *opaque) > > s->last_width = -1; > s->last_height = -1; > + s->last_text_width = -1; > + s->last_text_height = -1; > } > > void vga_common_reset(VGACommonState *s) > @@ -1887,6 +1892,8 @@ void vga_common_reset(VGACommonState *s) > s->last_ch = 0; > s->last_width = 0; > s->last_height = 0; > + s->last_text_width = 0; > + s->last_text_height = 0; > s->last_scr_width = 0; > s->last_scr_height = 0; > s->cursor_start = 0; > @@ -1938,8 +1945,8 @@ static void vga_update_text(void *opaque, uint32_t > *chardata) > s->graphic_mode = graphic_mode; > full_update = 1; > } > - if (s->last_width == -1) { > - s->last_width = 0; > + if (s->last_text_width == -1) { > + s->last_text_width = 0; > full_update = 1; > } > > @@ -1978,15 +1985,15 @@ static void vga_update_text(void *opaque, uint32_t > *chardata) > break; > } > > - if (width != s->last_width || height != s->last_height || > + if (width != s->last_text_width || height != s->last_text_height || > cw != s->last_cw || cheight != s->last_ch) { > s->last_scr_width = width * cw; > s->last_scr_height = height * cheight; > qemu_console_resize(s->con, s->last_scr_width, > s->last_scr_height); > qemu_console_text_resize(s->con, width, height); > s->last_depth = 0; > - s->last_width = width; > - s->last_height = height; > + s->last_text_width = width; > + s->last_text_height = height; > s->last_ch = cheight; > s->last_cw = cw; > full_update = 1; > @@ -2071,22 +2078,22 @@ static void vga_update_text(void *opaque, uint32_t > *chardata) > } > > /* Display a message */ > - s->last_width = 60; > - s->last_height = height = 3; > + s->last_text_width = 60; > + s->last_text_height = height = 3; > qemu_console_text_set_cursor(s->con, -1, -1); > - qemu_console_text_resize(s->con, s->last_width, height); > + qemu_console_text_resize(s->con, s->last_text_width, height); > > - for (dst = chardata, i = 0; i < s->last_width * height; i ++) > + for (dst = chardata, i = 0; i < s->last_text_width * height; i ++) > *dst++ = ' '; > > size = strlen(msg_buffer); > - width = (s->last_width - size) / 2; > - dst = chardata + s->last_width + width; > + width = (s->last_text_width - size) / 2; > + dst = chardata + s->last_text_width + width; > for (i = 0; i < size; i ++) > *dst++ = ATTR2CHTYPE(msg_buffer[i], QEMU_COLOR_BLUE, > QEMU_COLOR_BLACK, 1); > > - qemu_console_text_update(s->con, 0, 0, s->last_width, height); > + qemu_console_text_update(s->con, 0, 0, s->last_text_width, height); > } > > static uint64_t vga_mem_read(void *opaque, hwaddr addr, > diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h > index 5664317ecd..ca69ae9815 100644 > --- a/hw/display/vga_int.h > +++ b/hw/display/vga_int.h > @@ -122,7 +122,8 @@ typedef struct VGACommonState { > uint32_t plane_updated; > uint32_t last_line_offset; > uint8_t last_cw, last_ch; > - uint32_t last_width, last_height; /* in chars or pixels */ > + uint32_t last_width, last_height; /* in pixels (graphics renderer) */ > + uint32_t last_text_width, last_text_height; /* in chars (text renderer) > */ > uint32_t last_scr_width, last_scr_height; /* in pixels */ > uint32_t last_depth; /* in bits */ > bool last_byteswap; > -- > 2.47.3 >
