[PATCH v2] hw/display/vga: fix text-mode OOB write after a graphics surface switch

[PATCH v2] hw/display/vga: fix text-mode OOB write after a graphics surface switch

[Warisjeet Singh]

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.

Split the geometry cache per renderer so the units are unambiguous,
and 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: qemu-stable@nongnu.org
Signed-off-by: Warisjeet Singh (sin99xx) <sinxx198@gmail.com>
---
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     | 13 ++++++++++---
 hw/display/vga_int.h |  3 ++-
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index da0c331486..7a349dc738 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;
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

Re: [PATCH v2] hw/display/vga: fix text-mode OOB write after a graphics surface switch

[Marc-André Lureau]

Hi

On Sun, Aug 23, 2026 at 8:41 PM Warisjeet Singh <sinxx198@gmail.com> 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.
>
> Split the geometry cache per renderer so the units are unambiguous,
> and 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: qemu-stable@nongnu.org
> Signed-off-by: Warisjeet Singh (sin99xx) <sinxx198@gmail.com>
> ---
> 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     | 13 ++++++++++---
>  hw/display/vga_int.h |  3 ++-
>  2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index da0c331486..7a349dc738 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;
> 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) */

We should finish separating gfx and text state.
The text functions and paths should only manipulate
last_text_{width,height} (in vga_draw_text/update_text, and text path
in invalidate_display/common_reset)

>      uint32_t last_scr_width, last_scr_height; /* in pixels */
>      uint32_t last_depth; /* in bits */
>      bool last_byteswap;
> --
> 2.47.3
>

Re: [PATCH v2] hw/display/vga: fix text-mode OOB write after a graphics surface switch

[Warisjeet Singh]

Hi Marc-André,

On Mon, Aug 24, 2026, Marc-André Lureau wrote:
> We should finish separating gfx and text state.
> The text functions and paths should only manipulate
> last_text_{width,height} (in vga_draw_text/update_text, and text path
> in invalidate_display/common_reset)

Done in v3: vga_update_text() (predicate, cache stores, the -1
invalidate handshake, and the mode-message box) now uses
last_text_{width,height} too, so the text paths never touch the
graphics cache fields.  last_{width,height} are written by
vga_draw_graphic() and the shared reset/invalidate points only.

Re-verified on master @ eea8fe61b8: the qtest PoC (graphics 80x25 px
-> text 80x25 chars) is caught by the surface check, QEMU survives;
SeaBIOS text boot screendump still gives a clean 720x400 frame.

v3 sent as a new thread: [PATCH v3] hw/display/vga: fix text-mode OOB
write after a graphics surface switch.

Regards,
Warisjeet