From: Marc-André Lureau <marcandre.lureau@redhat.com>

virtio-vga has two renderers (VGA core and virtio-gpu) sharing a single
QemuConsole, with no invalidation protocol between them. When virtio-gpu
replaces the surface (SET_SCANOUT) or destroys it (reset via
qemu_console_set_surface(con, NULL)), the VGA renderer's cached geometry
(last_scr_width, last_height, last_depth, etc.) is never cleared:
vga_common_reset() does not run during a virtio-gpu reset.

The text draw path was fixed by 418396be80 (CVE-2026-77913), but
vga_draw_blank() and vga_draw_graphic() still trust stale cache against
a foreign surface:

vga_draw_blank() uses last_scr_width/height to memset the display
surface. When the surface has been replaced by a smaller one (e.g.
after a text-to-graphics mode switch, or a virtio-gpu reset installing
a 16x16 placeholder), the memset writes past the surface buffer. Use
the actual surface dimensions for the memset bounds instead of the
cached last_scr values.

vga_draw_graphic() decides whether to recreate the DisplaySurface based
solely on VGA register state (line_offset, width, height, depth,
byteswap, allocated). When virtio-gpu replaces the surface with a
smaller one, the predicate sees no change and the drawing loop writes a
full VGA-geometry frame into the undersized surface. Add surface
dimension checks to the predicate so that a foreign surface whose size
does not match the VGA geometry triggers a resize before drawing.

Ideally the two renderers could coordinate through a console-level
invalidation callback, but these defensive checks are needed regardless
as a safety net against any path that replaces the surface.

Fixes: CVE-2026-97889
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4579
Reported-by: david korczynski (@david1766)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/display/vga.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index cb0e28b79b6a..b9b8c529419d 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1641,7 +1641,9 @@ static void vga_draw_graphic(VGACommonState *s, int full_update)
         height != s->last_height ||
         s->last_depth != depth ||
         s->last_byteswap != byteswap ||
-        allocate_surface != surface_is_allocated(surface)) {
+        allocate_surface != surface_is_allocated(surface) ||
+        disp_width != surface_width(surface) ||
+        height != surface_height(surface)) {
         /* display parameters changed -> need new display surface */
         s->last_scr_width = disp_width;
         s->last_scr_height = height;
@@ -1789,9 +1791,9 @@ static void vga_draw_blank(VGACommonState *s, int full_update)
         qemu_console_set_surface(s->con, surface);
     }
 
-    w = s->last_scr_width * surface_bytes_per_pixel(surface);
+    w = surface_width(surface) * surface_bytes_per_pixel(surface);
     d = surface_data(surface);
-    for(i = 0; i < s->last_scr_height; i++) {
+    for (i = 0; i < surface_height(surface); i++) {
         memset(d, 0, w);
         d += surface_stride(surface);
     }
-- 
2.56.0.rc0.29.g47ce80527c56

On 1/10/26 12:41, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> virtio-vga has two renderers (VGA core and virtio-gpu) sharing a single
> QemuConsole, with no invalidation protocol between them. When virtio-gpu
> replaces the surface (SET_SCANOUT) or destroys it (reset via
> qemu_console_set_surface(con, NULL)), the VGA renderer's cached geometry
> (last_scr_width, last_height, last_depth, etc.) is never cleared:
> vga_common_reset() does not run during a virtio-gpu reset.
> 
> The text draw path was fixed by 418396be80 (CVE-2026-77913), but
> vga_draw_blank() and vga_draw_graphic() still trust stale cache against
> a foreign surface:
> 
> vga_draw_blank() uses last_scr_width/height to memset the display
> surface. When the surface has been replaced by a smaller one (e.g.
> after a text-to-graphics mode switch, or a virtio-gpu reset installing
> a 16x16 placeholder), the memset writes past the surface buffer. Use
> the actual surface dimensions for the memset bounds instead of the
> cached last_scr values.
> 
> vga_draw_graphic() decides whether to recreate the DisplaySurface based
> solely on VGA register state (line_offset, width, height, depth,
> byteswap, allocated). When virtio-gpu replaces the surface with a
> smaller one, the predicate sees no change and the drawing loop writes a
> full VGA-geometry frame into the undersized surface. Add surface
> dimension checks to the predicate so that a foreign surface whose size
> does not match the VGA geometry triggers a resize before drawing.
> 
> Ideally the two renderers could coordinate through a console-level
> invalidation callback, but these defensive checks are needed regardless
> as a safety net against any path that replaces the surface.
> 
> Fixes: CVE-2026-97889
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4579
> Reported-by: david korczynski (@david1766)
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   hw/display/vga.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index cb0e28b79b6a..b9b8c529419d 100644
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -1641,7 +1641,9 @@ static void vga_draw_graphic(VGACommonState *s, int full_update)
>           height != s->last_height ||
>           s->last_depth != depth ||
>           s->last_byteswap != byteswap ||
> -        allocate_surface != surface_is_allocated(surface)) {
> +        allocate_surface != surface_is_allocated(surface) ||
> +        disp_width != surface_width(surface) ||
> +        height != surface_height(surface)) {

This if() condition became very complex and is hideous to read,
could we split as a helper for readability? (improvement that can
be done on top).

>           /* display parameters changed -> need new display surface */
>           s->last_scr_width = disp_width;
>           s->last_scr_height = height;
> @@ -1789,9 +1791,9 @@ static void vga_draw_blank(VGACommonState *s, int full_update)
>           qemu_console_set_surface(s->con, surface);
>       }
>   
> -    w = s->last_scr_width * surface_bytes_per_pixel(surface);
> +    w = surface_width(surface) * surface_bytes_per_pixel(surface);
>       d = surface_data(surface);
> -    for(i = 0; i < s->last_scr_height; i++) {
> +    for (i = 0; i < surface_height(surface); i++) {
>           memset(d, 0, w);
>           d += surface_stride(surface);
>       }

Changes LGTM and make sense but I'd rather another review on top
since this is a CVE fix (Cc'ed Chad and Zoltan in case...).

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>