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

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 <pbonzini@redhat.com>
Signed-off-by: Warisjeet Singh <sinxx198@gmail.com>
[ Marc- André - drop realloc() resize condition & commit message ]
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 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
-- 
2.55.0

On 28/7/26 17:14, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> 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 <pbonzini@redhat.com>
> Signed-off-by: Warisjeet Singh <sinxx198@gmail.com>
> [ Marc- André - drop realloc() resize condition & commit message ]
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   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é <philmd@oss.qualcomm.com>

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;

On Tue, Jul 28, 2026 at 7:16 PM <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> 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 <pbonzini@redhat.com>
> Signed-off-by: Warisjeet Singh <sinxx198@gmail.com>
> [ Marc- André - drop realloc() resize condition & commit message ]
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

ping

> ---
>  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
> --
> 2.55.0
>
>