[PATCH] vga: split text renderer geometry cache from graphics renderer

[PATCH] vga: split text renderer geometry cache from graphics renderer

[sin99xx]

Resending with the correct [PATCH] subject prefix; please ignore
the previous copy.  Sorry for the noise.

vga_draw_text() and vga_draw_graphic() share last_width/last_height
but store them in different units (chars vs pixels).  A graphics
frame leaving values equal to a following text frame's char counts
makes the text resize predicate compare equal, skipping the console
resize; the glyph loop then paints out of bounds of the surface.

Commit 95687639e6 (CVE-2026-17516) fixed the graphics-path consumer
of this confusion but not the text path.  Give vga_draw_text() its
own cache fields.

Fixes: CVE-2026-77913
Cc: qemu-stable@nongnu.org
Signed-off-by: Warisjeet Singh (sin99xx) <sinxx198@gmail.com>
---
 hw/display/vga.c     | 10 +++++++---
 hw/display/vga_int.h |  3 ++-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1241,7 +1241,7 @@
         return;
     }

-    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_depth) {
         s->last_scr_width = width * cw;
         s->last_scr_height = height * cheight;
@@ -1249,8 +1249,8 @@
         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 +1845,8 @@

     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 +1889,8 @@
     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
--- a/hw/display/vga_int.h
+++ b/hw/display/vga_int.h
@@ -122,7 +122,8 @@
     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;

Re: [PATCH] vga: split text renderer geometry cache from graphics renderer

[Marc-André Lureau]

Hi

On Fri, Aug 21, 2026 at 11:28 PM sin99xx <sinxx198@gmail.com> wrote:
>
> Resending with the correct [PATCH] subject prefix; please ignore
> the previous copy.  Sorry for the noise.

No worries, but drop it from the commit message, or use '---'
(three-dashes, see git-am(1)) section instead.

>
> vga_draw_text() and vga_draw_graphic() share last_width/last_height
> but store them in different units (chars vs pixels).  A graphics
> frame leaving values equal to a following text frame's char counts
> makes the text resize predicate compare equal, skipping the console
> resize; the glyph loop then paints out of bounds of the surface.
>
> Commit 95687639e6 (CVE-2026-17516) fixed the graphics-path consumer
> of this confusion but not the text path.  Give vga_draw_text() its
> own cache fields.
>
> Fixes: CVE-2026-77913

Also add
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4215

But the patch doesn't fix the test you reported though.. Can you
check? Compile qemu with ASAN. (I also patched
qemu_pixman_shareable_alloc()/free() to use g_malloc(), we may want to
make this an option to help tools..)


> Cc: qemu-stable@nongnu.org
> Signed-off-by: Warisjeet Singh (sin99xx) <sinxx198@gmail.com>
> ---
>  hw/display/vga.c     | 10 +++++++---
>  hw/display/vga_int.h |  3 ++-
>  2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -1241,7 +1241,7 @@
>          return;
>      }
>
> -    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_depth) {
>          s->last_scr_width = width * cw;
>          s->last_scr_height = height * cheight;
> @@ -1249,8 +1249,8 @@
>          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 +1845,8 @@
>
>      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 +1889,8 @@
>      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
> --- a/hw/display/vga_int.h
> +++ b/hw/display/vga_int.h
> @@ -122,7 +122,8 @@
>      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;
>

Re: [PATCH] vga: split text renderer geometry cache from graphics renderer

[Warisjeet Singh]

Hi Marc-André,

On Sat, Aug 22, 2026, Marc-André Lureau wrote:
> No worries, but drop it from the commit message, or use '---'
> (three-dashes, see git-am(1)) section instead.

Done — v2 keeps notes under '---' only.

> Also add
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4215

Added.

> But the patch doesn't fix the test you reported though.. Can you
> check? Compile qemu with ASAN.

You were right, and I found the exact reason. The split alone is
insufficient because the text predicate still can't observe the console
surface being replaced by the graphics renderer:

- G1 in my reproducer is a *legacy* (non-VBE) graphics mode, and
  vga_get_bpp() returns 0 there — so s->last_depth stays 0 and the
  "|| s->last_depth" term that normally forces a resize after a
  graphics frame never fires.
- With v1, last_text_width/last_text_height still hold (80, 25) from
  the first text frame, so the T2 predicate compares equal, the resize
  is skipped, and the glyph loop paints 720x400 px into the 80x25 px
  surface the graphics path left behind.

v2 keeps the split (units are now unambiguous) and adds the term that
actually catches the swap: the text path compares the pixel size it is
about to paint (width*cw x height*cheight) against the console
surface's real dimensions, and resizes on mismatch. Caches in either
unit can be stale wrt the surface; the surface cannot.

Verification (master @ eea8fe61b8 and v11.1.0, same qtest PoC as in
the report):
- unpatched: SIGSEGV in vga_draw_glyph9() during the T2 render.
- v2 patched: T2 forces the console resize, QEMU survives, subsequent
  screendumps work.
- ASAN build with qemu_pixman_shareable_alloc()/free() routed to
  g_malloc() as you suggested: unpatched, the PoC triggers
  "heap-buffer-overflow WRITE of size 4" in vga_draw_glyph9()
  (vga-helpers.h:80), 0 bytes after the 8000-byte surface region —
  fired through the ordinary console refresh BH, no screendump needed.
  With v2 applied the same run is clean (no ASAN report, all redraws
  succeed).

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

Regards,
Warisjeet