[PATCH v2] hw/display/qxl: validate primary surface stride against width

[PATCH v2] hw/display/qxl: validate primary surface stride against width

[marcandre.lureau]

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

The existing validation in qxl_create_guest_primary() checks that
abs(stride) * height fits in vgamem_size and that stride is 4-byte
aligned, but never checks that abs(stride) is large enough to hold one
row of pixels for the declared width and format.

A malicious guest can create a primary surface with a stride much
smaller than width * bytes_per_pixel (e.g. stride=4 for a 64-wide 32bpp
surface). The spice server rejects this via red_validate_surface(), but
the return is void and QEMU unconditionally proceeds to set up the local
rendering state. On the next display refresh, VNC or SDL reads width *
bytes_pp per scanline from a region backed by only stride bytes per
row, causing a host-side out-of-bounds read.

Add three checks in qxl_create_guest_primary() before creating the
surface:
 - reject unknown surface formats
 - reject zero width or height
 - reject surfaces where abs(stride) < width * bytes_per_pixel

Also fix two related issues in qxl-render.c:
 - qxl_blit() used abs_stride to advance the dst pointer into the
   DisplaySurface, but when stride is negative the DisplaySurface is a
   packed buffer whose stride may be smaller. Use surface_stride()
   instead.
 - qxl_render_update_area_unlocked() uses guest_head0_width (set via
   QXL_IO_MONITORS_CONFIG_ASYNC) without validating it against
   abs_stride, bypassing the new validation. Clamp the effective width
   to abs_stride / bytes_pp to prevent out-of-bounds access while
   tolerating the normal transient where the monitor config arrives
   before the primary surface is resized to match.

Fixes: CVE-2026-16271
Fixes: 3761abb16784 ("hw/display/qxl: fix signed to unsigned comparison")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3637
Reported-by: huntr bubble
Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
---
 hw/display/qxl-render.c | 11 +++++++++--
 hw/display/qxl.c        | 29 +++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index 4799c9e8befd..231cad17ee3e 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -27,6 +27,7 @@
 static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
 {
     DisplaySurface *surface = qemu_console_surface(qxl->vga.con);
+    int dst_stride = surface_stride(surface);
     uint8_t *dst = surface_data(surface);
     uint8_t *src;
     int len, i;
@@ -45,14 +46,14 @@ static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
     } else {
         src += rect->top * qxl->guest_primary.abs_stride;
     }
-    dst += rect->top  * qxl->guest_primary.abs_stride;
+    dst += rect->top  * dst_stride;
     src += rect->left * qxl->guest_primary.bytes_pp;
     dst += rect->left * qxl->guest_primary.bytes_pp;
     len  = (rect->right - rect->left) * qxl->guest_primary.bytes_pp;
 
     for (i = rect->top; i < rect->bottom; i++) {
         memcpy(dst, src, len);
-        dst += qxl->guest_primary.abs_stride;
+        dst += dst_stride;
         src += qxl->guest_primary.qxl_stride;
     }
 }
@@ -103,6 +104,12 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
     int height = qxl->guest_head0_height ?: qxl->guest_primary.surface.height;
     int i;
 
+    if (qxl->guest_primary.bytes_pp > 0) {
+        int max_width = qxl->guest_primary.abs_stride
+                        / qxl->guest_primary.bytes_pp;
+        width = MIN(width, max_width);
+    }
+
     if (qxl->guest_primary.resized) {
         qxl->guest_primary.resized = 0;
         qxl->guest_primary.data = qxl_phys2virt(qxl,
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index b7d871b9ee33..41673c369bca 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1496,6 +1496,7 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
     QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
     uint32_t requested_height = le32_to_cpu(sc->height);
     int requested_stride = le32_to_cpu(sc->stride);
+    uint32_t bytes_pp;
 
     if (requested_stride == INT32_MIN ||
         abs(requested_stride) * (uint64_t)requested_height
@@ -1532,6 +1533,34 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
         return;
     }
 
+    switch (surface.format) {
+    case SPICE_SURFACE_FMT_16_555:
+    case SPICE_SURFACE_FMT_16_565:
+        bytes_pp = 2;
+        break;
+    case SPICE_SURFACE_FMT_32_xRGB:
+    case SPICE_SURFACE_FMT_32_ARGB:
+        bytes_pp = 4;
+        break;
+    default:
+        qxl_set_guest_bug(qxl, "%s: unhandled format %d",
+                          __func__, surface.format);
+        return;
+    }
+
+    if (surface.width == 0 || surface.height == 0) {
+        qxl_set_guest_bug(qxl, "%s: zero dimension %ux%u",
+                          __func__, surface.width, surface.height);
+        return;
+    }
+
+    if ((uint64_t)surface.width * bytes_pp > abs(surface.stride)) {
+        qxl_set_guest_bug(qxl, "%s: stride too small for width:"
+                          " stride %d width %u bpp %u",
+                          __func__, surface.stride, surface.width, bytes_pp);
+        return;
+    }
+
     surface.mouse_mode = true;
     surface.group_id   = MEMSLOT_GROUP_GUEST;
     if (loadvm) {
-- 
2.55.0

Re: [PATCH v2] hw/display/qxl: validate primary surface stride against width

[Akihiko Odaki]

On 2026/07/23 22:59, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> The existing validation in qxl_create_guest_primary() checks that
> abs(stride) * height fits in vgamem_size and that stride is 4-byte
> aligned, but never checks that abs(stride) is large enough to hold one
> row of pixels for the declared width and format.
> 
> A malicious guest can create a primary surface with a stride much
> smaller than width * bytes_per_pixel (e.g. stride=4 for a 64-wide 32bpp
> surface). The spice server rejects this via red_validate_surface(), but
> the return is void and QEMU unconditionally proceeds to set up the local
> rendering state. On the next display refresh, VNC or SDL reads width *
> bytes_pp per scanline from a region backed by only stride bytes per
> row, causing a host-side out-of-bounds read.
> 
> Add three checks in qxl_create_guest_primary() before creating the
> surface:
>   - reject unknown surface formats
>   - reject zero width or height
>   - reject surfaces where abs(stride) < width * bytes_per_pixel
> 
> Also fix two related issues in qxl-render.c:
>   - qxl_blit() used abs_stride to advance the dst pointer into the
>     DisplaySurface, but when stride is negative the DisplaySurface is a
>     packed buffer whose stride may be smaller. Use surface_stride()
>     instead.
>   - qxl_render_update_area_unlocked() uses guest_head0_width (set via
>     QXL_IO_MONITORS_CONFIG_ASYNC) without validating it against
>     abs_stride, bypassing the new validation. Clamp the effective width
>     to abs_stride / bytes_pp to prevent out-of-bounds access while
>     tolerating the normal transient where the monitor config arrives
>     before the primary surface is resized to match.

The height is also problematic. For the same reason, guest_head0_height 
can bypass the check in qxl_create_guest_primary(), overrunning vgamem_size.

qxl_phys2virt(), called in qxl_render_update_area_unlocked(), is meant 
to prevent overrunning the containing MemoryRegion, but it is also 
ineffective because the size parameter passed to the function may have 
overflowed; the value is calculated as abs_stride * height, so it will 
overflow when abs_stride == 16 MiB and height == 256.

Regards,
Akihiko Odaki

> 
> Fixes: CVE-2026-16271
> Fixes: 3761abb16784 ("hw/display/qxl: fix signed to unsigned comparison")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3637
> Reported-by: huntr bubble
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> ---
>   hw/display/qxl-render.c | 11 +++++++++--
>   hw/display/qxl.c        | 29 +++++++++++++++++++++++++++++
>   2 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
> index 4799c9e8befd..231cad17ee3e 100644
> --- a/hw/display/qxl-render.c
> +++ b/hw/display/qxl-render.c
> @@ -27,6 +27,7 @@
>   static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
>   {
>       DisplaySurface *surface = qemu_console_surface(qxl->vga.con);
> +    int dst_stride = surface_stride(surface);
>       uint8_t *dst = surface_data(surface);
>       uint8_t *src;
>       int len, i;
> @@ -45,14 +46,14 @@ static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
>       } else {
>           src += rect->top * qxl->guest_primary.abs_stride;
>       }
> -    dst += rect->top  * qxl->guest_primary.abs_stride;
> +    dst += rect->top  * dst_stride;
>       src += rect->left * qxl->guest_primary.bytes_pp;
>       dst += rect->left * qxl->guest_primary.bytes_pp;
>       len  = (rect->right - rect->left) * qxl->guest_primary.bytes_pp;
>   
>       for (i = rect->top; i < rect->bottom; i++) {
>           memcpy(dst, src, len);
> -        dst += qxl->guest_primary.abs_stride;
> +        dst += dst_stride;
>           src += qxl->guest_primary.qxl_stride;
>       }
>   }
> @@ -103,6 +104,12 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
>       int height = qxl->guest_head0_height ?: qxl->guest_primary.surface.height;
>       int i;
>   
> +    if (qxl->guest_primary.bytes_pp > 0) {
> +        int max_width = qxl->guest_primary.abs_stride
> +                        / qxl->guest_primary.bytes_pp;
> +        width = MIN(width, max_width);
> +    }
> +
>       if (qxl->guest_primary.resized) {
>           qxl->guest_primary.resized = 0;
>           qxl->guest_primary.data = qxl_phys2virt(qxl,
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index b7d871b9ee33..41673c369bca 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -1496,6 +1496,7 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
>       QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
>       uint32_t requested_height = le32_to_cpu(sc->height);
>       int requested_stride = le32_to_cpu(sc->stride);
> +    uint32_t bytes_pp;
>   
>       if (requested_stride == INT32_MIN ||
>           abs(requested_stride) * (uint64_t)requested_height
> @@ -1532,6 +1533,34 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
>           return;
>       }
>   
> +    switch (surface.format) {
> +    case SPICE_SURFACE_FMT_16_555:
> +    case SPICE_SURFACE_FMT_16_565:
> +        bytes_pp = 2;
> +        break;
> +    case SPICE_SURFACE_FMT_32_xRGB:
> +    case SPICE_SURFACE_FMT_32_ARGB:
> +        bytes_pp = 4;
> +        break;
> +    default:
> +        qxl_set_guest_bug(qxl, "%s: unhandled format %d",
> +                          __func__, surface.format);
> +        return;
> +    }
> +
> +    if (surface.width == 0 || surface.height == 0) {
> +        qxl_set_guest_bug(qxl, "%s: zero dimension %ux%u",
> +                          __func__, surface.width, surface.height);
> +        return;
> +    }
> +
> +    if ((uint64_t)surface.width * bytes_pp > abs(surface.stride)) {
> +        qxl_set_guest_bug(qxl, "%s: stride too small for width:"
> +                          " stride %d width %u bpp %u",
> +                          __func__, surface.stride, surface.width, bytes_pp);
> +        return;
> +    }
> +
>       surface.mouse_mode = true;
>       surface.group_id   = MEMSLOT_GROUP_GUEST;
>       if (loadvm) {

[PATCH v2] hw/display/qxl: validate primary surface stride against width

[marcandre.lureau]

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

The existing validation in qxl_create_guest_primary() checks that
abs(stride) * height fits in vgamem_size and that stride is 4-byte
aligned, but never checks that abs(stride) is large enough to hold one
row of pixels for the declared width and format.

A malicious guest can create a primary surface with a stride much
smaller than width * bytes_per_pixel (e.g. stride=4 for a 64-wide 32bpp
surface). The spice server rejects this via red_validate_surface(), but
the return is void and QEMU unconditionally proceeds to set up the local
rendering state. On the next display refresh, VNC or SDL reads width *
bytes_pp per scanline from a region backed by only stride bytes per
row, causing a host-side out-of-bounds read.

Add three checks in qxl_create_guest_primary() before creating the
surface:
 - reject unknown surface formats
 - reject zero width or height
 - reject surfaces where abs(stride) < width * bytes_per_pixel

Also fix three related issues in qxl-render.c:
 - qxl_blit() used abs_stride to advance the dst pointer into the
   DisplaySurface, but when stride is negative the DisplaySurface is a
   packed buffer whose stride may be smaller. Use surface_stride()
   instead.
 - qxl_render_update_area_unlocked() uses guest_head0_width (set via
   QXL_IO_MONITORS_CONFIG_ASYNC) without validating it against
   abs_stride, bypassing the new validation. Clamp the effective width
   to abs_stride / bytes_pp to prevent out-of-bounds access while
   tolerating the normal transient where the monitor config arrives
   before the primary surface is resized to match.
 - Similarly, guest_head0_height bypasses qxl_create_guest_primary()
   validation. Without clamping, abs_stride * height can overrun
   vgamem_size, and the product can also overflow 32 bits (e.g.
   abs_stride=16 MiB, height=256 wraps to zero), defeating the
   qxl_phys2virt() bounds check. Clamp height to
   vgamem_size / abs_stride to prevent both.

Fixes: CVE-2026-16271
Fixes: 3761abb16784 ("hw/display/qxl: fix signed to unsigned comparison")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3637
Reported-by: huntr bubble
Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
---
v2:
 - also clamp height
 - factor out qxl_format_bpp() helper
---
 hw/display/qxl.h        |  2 ++
 hw/display/qxl-render.c | 46 ++++++++++++++++----------------
 hw/display/qxl.c        | 59 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 84 insertions(+), 23 deletions(-)

diff --git a/hw/display/qxl.h b/hw/display/qxl.h
index 48d664f77736..6f5b96fe86cc 100644
--- a/hw/display/qxl.h
+++ b/hw/display/qxl.h
@@ -181,6 +181,8 @@ void qxl_spice_oom(PCIQXLDevice *qxl);
 void qxl_spice_reset_memslots(PCIQXLDevice *qxl);
 void qxl_spice_reset_image_cache(PCIQXLDevice *qxl);
 void qxl_spice_reset_cursor(PCIQXLDevice *qxl);
+bool qxl_format_bpp(PCIQXLDevice *qxl, SpiceSurfaceFmt format,
+                    uint32_t *bytes_pp, uint32_t *bits_pp);
 
 /* qxl-logger.c */
 int qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id);
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index 4799c9e8befd..b0a71a95ad66 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -27,6 +27,7 @@
 static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
 {
     DisplaySurface *surface = qemu_console_surface(qxl->vga.con);
+    int dst_stride = surface_stride(surface);
     uint8_t *dst = surface_data(surface);
     uint8_t *src;
     int len, i;
@@ -45,14 +46,14 @@ static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
     } else {
         src += rect->top * qxl->guest_primary.abs_stride;
     }
-    dst += rect->top  * qxl->guest_primary.abs_stride;
+    dst += rect->top  * dst_stride;
     src += rect->left * qxl->guest_primary.bytes_pp;
     dst += rect->left * qxl->guest_primary.bytes_pp;
     len  = (rect->right - rect->left) * qxl->guest_primary.bytes_pp;
 
     for (i = rect->top; i < rect->bottom; i++) {
         memcpy(dst, src, len);
-        dst += qxl->guest_primary.abs_stride;
+        dst += dst_stride;
         src += qxl->guest_primary.qxl_stride;
     }
 }
@@ -64,27 +65,9 @@ void qxl_render_resize(PCIQXLDevice *qxl)
     qxl->guest_primary.qxl_stride = sc->stride;
     qxl->guest_primary.abs_stride = abs(sc->stride);
     qxl->guest_primary.resized++;
-    switch (sc->format) {
-    case SPICE_SURFACE_FMT_16_555:
-        qxl->guest_primary.bytes_pp = 2;
-        qxl->guest_primary.bits_pp = 15;
-        break;
-    case SPICE_SURFACE_FMT_16_565:
-        qxl->guest_primary.bytes_pp = 2;
-        qxl->guest_primary.bits_pp = 16;
-        break;
-    case SPICE_SURFACE_FMT_32_xRGB:
-    case SPICE_SURFACE_FMT_32_ARGB:
-        qxl->guest_primary.bytes_pp = 4;
-        qxl->guest_primary.bits_pp = 32;
-        break;
-    default:
-        fprintf(stderr, "%s: unhandled format: %x\n", __func__,
-                qxl->guest_primary.surface.format);
-        qxl->guest_primary.bytes_pp = 4;
-        qxl->guest_primary.bits_pp = 32;
-        break;
-    }
+    /* fallback to default bpp if format is unknown */
+    qxl_format_bpp(qxl, sc->format, &qxl->guest_primary.bytes_pp,
+                   &qxl->guest_primary.bits_pp);
 }
 
 static void qxl_set_rect_to_surface(PCIQXLDevice *qxl, QXLRect *area)
@@ -103,6 +86,23 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
     int height = qxl->guest_head0_height ?: qxl->guest_primary.surface.height;
     int i;
 
+    if (width <= 0 || height <= 0) {
+        qxl_set_guest_bug(qxl, "%s: invalid dimension %dx%d",
+                          __func__, width, height);
+        goto end;
+    }
+
+    if (qxl->guest_primary.bytes_pp > 0) {
+        int max_width = qxl->guest_primary.abs_stride
+                        / qxl->guest_primary.bytes_pp;
+        width = MIN(width, max_width);
+    }
+
+    if (qxl->guest_primary.abs_stride > 0) {
+        int max_height = qxl->vgamem_size / qxl->guest_primary.abs_stride;
+        height = MIN(height, max_height);
+    }
+
     if (qxl->guest_primary.resized) {
         qxl->guest_primary.resized = 0;
         qxl->guest_primary.data = qxl_phys2virt(qxl,
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index b7d871b9ee33..384b8767b8e6 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1489,6 +1489,47 @@ static void qxl_create_guest_primary_complete(PCIQXLDevice *qxl)
     qxl_render_resize(qxl);
 }
 
+/*
+ * Convert a SpiceSurfaceFormat to bytes per pixel and bits per pixel.
+ *
+ * Only valid for surface suitable for rendering.
+ */
+bool qxl_format_bpp(PCIQXLDevice *qxl, SpiceSurfaceFmt format,
+                    uint32_t *bytes_pp, uint32_t *bits_pp)
+{
+    uint32_t bypp = 4;
+    uint32_t bipp = 32;
+    bool ret = true;
+
+    switch (format) {
+    case SPICE_SURFACE_FMT_16_555:
+        bypp = 2;
+        bipp = 15;
+        break;
+    case SPICE_SURFACE_FMT_16_565:
+        bypp = 2;
+        bipp = 16;
+        break;
+    case SPICE_SURFACE_FMT_32_xRGB:
+    case SPICE_SURFACE_FMT_32_ARGB:
+        bypp = 4;
+        bipp = 32;
+        break;
+    default:
+        ret = false;
+        qxl_set_guest_bug(qxl, "%s: unhandled format: %x", __func__, format);
+    }
+
+    if (bytes_pp != NULL) {
+        *bytes_pp = bypp;
+    }
+    if (bits_pp != NULL) {
+        *bits_pp = bipp;
+    }
+
+    return ret;
+}
+
 static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
                                      qxl_async_io async)
 {
@@ -1496,6 +1537,7 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
     QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
     uint32_t requested_height = le32_to_cpu(sc->height);
     int requested_stride = le32_to_cpu(sc->stride);
+    uint32_t bytes_pp;
 
     if (requested_stride == INT32_MIN ||
         abs(requested_stride) * (uint64_t)requested_height
@@ -1532,6 +1574,23 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
         return;
     }
 
+    if (!qxl_format_bpp(qxl, surface.format, &bytes_pp, NULL)) {
+        return;
+    }
+
+    if (surface.width == 0 || surface.height == 0) {
+        qxl_set_guest_bug(qxl, "%s: zero dimension %ux%u",
+                          __func__, surface.width, surface.height);
+        return;
+    }
+
+    if ((uint64_t)surface.width * bytes_pp > abs(surface.stride)) {
+        qxl_set_guest_bug(qxl, "%s: stride too small for width:"
+                          " stride %d width %u bpp %u",
+                          __func__, surface.stride, surface.width, bytes_pp);
+        return;
+    }
+
     surface.mouse_mode = true;
     surface.group_id   = MEMSLOT_GROUP_GUEST;
     if (loadvm) {
-- 
2.55.0

Re: [PATCH v2] hw/display/qxl: validate primary surface stride against width

[Marc-André Lureau]

On Sat, Jul 25, 2026 at 4:24 PM <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The existing validation in qxl_create_guest_primary() checks that
> abs(stride) * height fits in vgamem_size and that stride is 4-byte
> aligned, but never checks that abs(stride) is large enough to hold one
> row of pixels for the declared width and format.
>
> A malicious guest can create a primary surface with a stride much
> smaller than width * bytes_per_pixel (e.g. stride=4 for a 64-wide 32bpp
> surface). The spice server rejects this via red_validate_surface(), but
> the return is void and QEMU unconditionally proceeds to set up the local
> rendering state. On the next display refresh, VNC or SDL reads width *
> bytes_pp per scanline from a region backed by only stride bytes per
> row, causing a host-side out-of-bounds read.
>
> Add three checks in qxl_create_guest_primary() before creating the
> surface:
>  - reject unknown surface formats
>  - reject zero width or height
>  - reject surfaces where abs(stride) < width * bytes_per_pixel
>
> Also fix three related issues in qxl-render.c:
>  - qxl_blit() used abs_stride to advance the dst pointer into the
>    DisplaySurface, but when stride is negative the DisplaySurface is a
>    packed buffer whose stride may be smaller. Use surface_stride()
>    instead.
>  - qxl_render_update_area_unlocked() uses guest_head0_width (set via
>    QXL_IO_MONITORS_CONFIG_ASYNC) without validating it against
>    abs_stride, bypassing the new validation. Clamp the effective width
>    to abs_stride / bytes_pp to prevent out-of-bounds access while
>    tolerating the normal transient where the monitor config arrives
>    before the primary surface is resized to match.
>  - Similarly, guest_head0_height bypasses qxl_create_guest_primary()
>    validation. Without clamping, abs_stride * height can overrun
>    vgamem_size, and the product can also overflow 32 bits (e.g.
>    abs_stride=16 MiB, height=256 wraps to zero), defeating the
>    qxl_phys2virt() bounds check. Clamp height to
>    vgamem_size / abs_stride to prevent both.
>
> Fixes: CVE-2026-16271
> Fixes: 3761abb16784 ("hw/display/qxl: fix signed to unsigned comparison")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3637
> Reported-by: huntr bubble
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> ---
> v2:

oops, should be v3. not resending

Re: [PATCH v2] hw/display/qxl: validate primary surface stride against width

[Marc-André Lureau]

On Sat, Jul 25, 2026 at 4:24 PM <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The existing validation in qxl_create_guest_primary() checks that
> abs(stride) * height fits in vgamem_size and that stride is 4-byte
> aligned, but never checks that abs(stride) is large enough to hold one
> row of pixels for the declared width and format.
>
> A malicious guest can create a primary surface with a stride much
> smaller than width * bytes_per_pixel (e.g. stride=4 for a 64-wide 32bpp
> surface). The spice server rejects this via red_validate_surface(), but
> the return is void and QEMU unconditionally proceeds to set up the local
> rendering state. On the next display refresh, VNC or SDL reads width *
> bytes_pp per scanline from a region backed by only stride bytes per
> row, causing a host-side out-of-bounds read.
>
> Add three checks in qxl_create_guest_primary() before creating the
> surface:
>  - reject unknown surface formats
>  - reject zero width or height
>  - reject surfaces where abs(stride) < width * bytes_per_pixel
>
> Also fix three related issues in qxl-render.c:
>  - qxl_blit() used abs_stride to advance the dst pointer into the
>    DisplaySurface, but when stride is negative the DisplaySurface is a
>    packed buffer whose stride may be smaller. Use surface_stride()
>    instead.
>  - qxl_render_update_area_unlocked() uses guest_head0_width (set via
>    QXL_IO_MONITORS_CONFIG_ASYNC) without validating it against
>    abs_stride, bypassing the new validation. Clamp the effective width
>    to abs_stride / bytes_pp to prevent out-of-bounds access while
>    tolerating the normal transient where the monitor config arrives
>    before the primary surface is resized to match.
>  - Similarly, guest_head0_height bypasses qxl_create_guest_primary()
>    validation. Without clamping, abs_stride * height can overrun
>    vgamem_size, and the product can also overflow 32 bits (e.g.
>    abs_stride=16 MiB, height=256 wraps to zero), defeating the
>    qxl_phys2virt() bounds check. Clamp height to
>    vgamem_size / abs_stride to prevent both.
>
> Fixes: CVE-2026-16271
> Fixes: 3761abb16784 ("hw/display/qxl: fix signed to unsigned comparison")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3637
> Reported-by: huntr bubble
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> ---
> v2:
>  - also clamp height
>  - factor out qxl_format_bpp() helper

ping

> ---
>  hw/display/qxl.h        |  2 ++
>  hw/display/qxl-render.c | 46 ++++++++++++++++----------------
>  hw/display/qxl.c        | 59 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 84 insertions(+), 23 deletions(-)
>
> diff --git a/hw/display/qxl.h b/hw/display/qxl.h
> index 48d664f77736..6f5b96fe86cc 100644
> --- a/hw/display/qxl.h
> +++ b/hw/display/qxl.h
> @@ -181,6 +181,8 @@ void qxl_spice_oom(PCIQXLDevice *qxl);
>  void qxl_spice_reset_memslots(PCIQXLDevice *qxl);
>  void qxl_spice_reset_image_cache(PCIQXLDevice *qxl);
>  void qxl_spice_reset_cursor(PCIQXLDevice *qxl);
> +bool qxl_format_bpp(PCIQXLDevice *qxl, SpiceSurfaceFmt format,
> +                    uint32_t *bytes_pp, uint32_t *bits_pp);
>
>  /* qxl-logger.c */
>  int qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id);
> diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
> index 4799c9e8befd..b0a71a95ad66 100644
> --- a/hw/display/qxl-render.c
> +++ b/hw/display/qxl-render.c
> @@ -27,6 +27,7 @@
>  static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
>  {
>      DisplaySurface *surface = qemu_console_surface(qxl->vga.con);
> +    int dst_stride = surface_stride(surface);
>      uint8_t *dst = surface_data(surface);
>      uint8_t *src;
>      int len, i;
> @@ -45,14 +46,14 @@ static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
>      } else {
>          src += rect->top * qxl->guest_primary.abs_stride;
>      }
> -    dst += rect->top  * qxl->guest_primary.abs_stride;
> +    dst += rect->top  * dst_stride;
>      src += rect->left * qxl->guest_primary.bytes_pp;
>      dst += rect->left * qxl->guest_primary.bytes_pp;
>      len  = (rect->right - rect->left) * qxl->guest_primary.bytes_pp;
>
>      for (i = rect->top; i < rect->bottom; i++) {
>          memcpy(dst, src, len);
> -        dst += qxl->guest_primary.abs_stride;
> +        dst += dst_stride;
>          src += qxl->guest_primary.qxl_stride;
>      }
>  }
> @@ -64,27 +65,9 @@ void qxl_render_resize(PCIQXLDevice *qxl)
>      qxl->guest_primary.qxl_stride = sc->stride;
>      qxl->guest_primary.abs_stride = abs(sc->stride);
>      qxl->guest_primary.resized++;
> -    switch (sc->format) {
> -    case SPICE_SURFACE_FMT_16_555:
> -        qxl->guest_primary.bytes_pp = 2;
> -        qxl->guest_primary.bits_pp = 15;
> -        break;
> -    case SPICE_SURFACE_FMT_16_565:
> -        qxl->guest_primary.bytes_pp = 2;
> -        qxl->guest_primary.bits_pp = 16;
> -        break;
> -    case SPICE_SURFACE_FMT_32_xRGB:
> -    case SPICE_SURFACE_FMT_32_ARGB:
> -        qxl->guest_primary.bytes_pp = 4;
> -        qxl->guest_primary.bits_pp = 32;
> -        break;
> -    default:
> -        fprintf(stderr, "%s: unhandled format: %x\n", __func__,
> -                qxl->guest_primary.surface.format);
> -        qxl->guest_primary.bytes_pp = 4;
> -        qxl->guest_primary.bits_pp = 32;
> -        break;
> -    }
> +    /* fallback to default bpp if format is unknown */
> +    qxl_format_bpp(qxl, sc->format, &qxl->guest_primary.bytes_pp,
> +                   &qxl->guest_primary.bits_pp);
>  }
>
>  static void qxl_set_rect_to_surface(PCIQXLDevice *qxl, QXLRect *area)
> @@ -103,6 +86,23 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
>      int height = qxl->guest_head0_height ?: qxl->guest_primary.surface.height;
>      int i;
>
> +    if (width <= 0 || height <= 0) {
> +        qxl_set_guest_bug(qxl, "%s: invalid dimension %dx%d",
> +                          __func__, width, height);
> +        goto end;
> +    }
> +
> +    if (qxl->guest_primary.bytes_pp > 0) {
> +        int max_width = qxl->guest_primary.abs_stride
> +                        / qxl->guest_primary.bytes_pp;
> +        width = MIN(width, max_width);
> +    }
> +
> +    if (qxl->guest_primary.abs_stride > 0) {
> +        int max_height = qxl->vgamem_size / qxl->guest_primary.abs_stride;
> +        height = MIN(height, max_height);
> +    }
> +
>      if (qxl->guest_primary.resized) {
>          qxl->guest_primary.resized = 0;
>          qxl->guest_primary.data = qxl_phys2virt(qxl,
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index b7d871b9ee33..384b8767b8e6 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -1489,6 +1489,47 @@ static void qxl_create_guest_primary_complete(PCIQXLDevice *qxl)
>      qxl_render_resize(qxl);
>  }
>
> +/*
> + * Convert a SpiceSurfaceFormat to bytes per pixel and bits per pixel.
> + *
> + * Only valid for surface suitable for rendering.
> + */
> +bool qxl_format_bpp(PCIQXLDevice *qxl, SpiceSurfaceFmt format,
> +                    uint32_t *bytes_pp, uint32_t *bits_pp)
> +{
> +    uint32_t bypp = 4;
> +    uint32_t bipp = 32;
> +    bool ret = true;
> +
> +    switch (format) {
> +    case SPICE_SURFACE_FMT_16_555:
> +        bypp = 2;
> +        bipp = 15;
> +        break;
> +    case SPICE_SURFACE_FMT_16_565:
> +        bypp = 2;
> +        bipp = 16;
> +        break;
> +    case SPICE_SURFACE_FMT_32_xRGB:
> +    case SPICE_SURFACE_FMT_32_ARGB:
> +        bypp = 4;
> +        bipp = 32;
> +        break;
> +    default:
> +        ret = false;
> +        qxl_set_guest_bug(qxl, "%s: unhandled format: %x", __func__, format);
> +    }
> +
> +    if (bytes_pp != NULL) {
> +        *bytes_pp = bypp;
> +    }
> +    if (bits_pp != NULL) {
> +        *bits_pp = bipp;
> +    }
> +
> +    return ret;
> +}
> +
>  static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
>                                       qxl_async_io async)
>  {
> @@ -1496,6 +1537,7 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
>      QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
>      uint32_t requested_height = le32_to_cpu(sc->height);
>      int requested_stride = le32_to_cpu(sc->stride);
> +    uint32_t bytes_pp;
>
>      if (requested_stride == INT32_MIN ||
>          abs(requested_stride) * (uint64_t)requested_height
> @@ -1532,6 +1574,23 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
>          return;
>      }
>
> +    if (!qxl_format_bpp(qxl, surface.format, &bytes_pp, NULL)) {
> +        return;
> +    }
> +
> +    if (surface.width == 0 || surface.height == 0) {
> +        qxl_set_guest_bug(qxl, "%s: zero dimension %ux%u",
> +                          __func__, surface.width, surface.height);
> +        return;
> +    }
> +
> +    if ((uint64_t)surface.width * bytes_pp > abs(surface.stride)) {
> +        qxl_set_guest_bug(qxl, "%s: stride too small for width:"
> +                          " stride %d width %u bpp %u",
> +                          __func__, surface.stride, surface.width, bytes_pp);
> +        return;
> +    }
> +
>      surface.mouse_mode = true;
>      surface.group_id   = MEMSLOT_GROUP_GUEST;
>      if (loadvm) {
> --
> 2.55.0
>

Re: [PATCH v2] hw/display/qxl: validate primary surface stride against width

[Akihiko Odaki]

Sorry for a late review; I missed this patch.

On 2026/07/25 21:24, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> The existing validation in qxl_create_guest_primary() checks that
> abs(stride) * height fits in vgamem_size and that stride is 4-byte
> aligned, but never checks that abs(stride) is large enough to hold one
> row of pixels for the declared width and format.
> 
> A malicious guest can create a primary surface with a stride much
> smaller than width * bytes_per_pixel (e.g. stride=4 for a 64-wide 32bpp
> surface). The spice server rejects this via red_validate_surface(), but
> the return is void and QEMU unconditionally proceeds to set up the local
> rendering state. On the next display refresh, VNC or SDL reads width *
> bytes_pp per scanline from a region backed by only stride bytes per
> row, causing a host-side out-of-bounds read.
> 
> Add three checks in qxl_create_guest_primary() before creating the
> surface:
>   - reject unknown surface formats
>   - reject zero width or height
>   - reject surfaces where abs(stride) < width * bytes_per_pixel
> 
> Also fix three related issues in qxl-render.c:
>   - qxl_blit() used abs_stride to advance the dst pointer into the
>     DisplaySurface, but when stride is negative the DisplaySurface is a
>     packed buffer whose stride may be smaller. Use surface_stride()
>     instead.
>   - qxl_render_update_area_unlocked() uses guest_head0_width (set via
>     QXL_IO_MONITORS_CONFIG_ASYNC) without validating it against
>     abs_stride, bypassing the new validation. Clamp the effective width
>     to abs_stride / bytes_pp to prevent out-of-bounds access while
>     tolerating the normal transient where the monitor config arrives
>     before the primary surface is resized to match.
>   - Similarly, guest_head0_height bypasses qxl_create_guest_primary()
>     validation. Without clamping, abs_stride * height can overrun
>     vgamem_size, and the product can also overflow 32 bits (e.g.
>     abs_stride=16 MiB, height=256 wraps to zero), defeating the
>     qxl_phys2virt() bounds check. Clamp height to
>     vgamem_size / abs_stride to prevent both.
> 
> Fixes: CVE-2026-16271
> Fixes: 3761abb16784 ("hw/display/qxl: fix signed to unsigned comparison")

The Fixes: 3761abb16784 tag is incorrect. That commit only repaired 
overflow in an existing total-size check; short-stride surfaces were 
already accepted. The vulnerable local renderer originated in 
a19cbfb34642, while the monitor-dimension bypass came from 979f7ef8966b 
and the overflowable qxl_phys2virt() size expression from 
8efec0ef8bbc/6dbbf055148c.

> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3637
> Reported-by: huntr bubble
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
> ---
> v2:
>   - also clamp height
>   - factor out qxl_format_bpp() helper
> ---
>   hw/display/qxl.h        |  2 ++
>   hw/display/qxl-render.c | 46 ++++++++++++++++----------------
>   hw/display/qxl.c        | 59 +++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 84 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/display/qxl.h b/hw/display/qxl.h
> index 48d664f77736..6f5b96fe86cc 100644
> --- a/hw/display/qxl.h
> +++ b/hw/display/qxl.h
> @@ -181,6 +181,8 @@ void qxl_spice_oom(PCIQXLDevice *qxl);
>   void qxl_spice_reset_memslots(PCIQXLDevice *qxl);
>   void qxl_spice_reset_image_cache(PCIQXLDevice *qxl);
>   void qxl_spice_reset_cursor(PCIQXLDevice *qxl);
> +bool qxl_format_bpp(PCIQXLDevice *qxl, SpiceSurfaceFmt format,
> +                    uint32_t *bytes_pp, uint32_t *bits_pp);
>   
>   /* qxl-logger.c */
>   int qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id);
> diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
> index 4799c9e8befd..b0a71a95ad66 100644
> --- a/hw/display/qxl-render.c
> +++ b/hw/display/qxl-render.c
> @@ -27,6 +27,7 @@
>   static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
>   {
>       DisplaySurface *surface = qemu_console_surface(qxl->vga.con);
> +    int dst_stride = surface_stride(surface);
>       uint8_t *dst = surface_data(surface);
>       uint8_t *src;
>       int len, i;
> @@ -45,14 +46,14 @@ static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
>       } else {
>           src += rect->top * qxl->guest_primary.abs_stride;
>       }
> -    dst += rect->top  * qxl->guest_primary.abs_stride;
> +    dst += rect->top  * dst_stride;
>       src += rect->left * qxl->guest_primary.bytes_pp;
>       dst += rect->left * qxl->guest_primary.bytes_pp;
>       len  = (rect->right - rect->left) * qxl->guest_primary.bytes_pp;
>   
>       for (i = rect->top; i < rect->bottom; i++) {
>           memcpy(dst, src, len);
> -        dst += qxl->guest_primary.abs_stride;
> +        dst += dst_stride;
>           src += qxl->guest_primary.qxl_stride;
>       }
>   }
> @@ -64,27 +65,9 @@ void qxl_render_resize(PCIQXLDevice *qxl)
>       qxl->guest_primary.qxl_stride = sc->stride;
>       qxl->guest_primary.abs_stride = abs(sc->stride);
>       qxl->guest_primary.resized++;
> -    switch (sc->format) {
> -    case SPICE_SURFACE_FMT_16_555:
> -        qxl->guest_primary.bytes_pp = 2;
> -        qxl->guest_primary.bits_pp = 15;
> -        break;
> -    case SPICE_SURFACE_FMT_16_565:
> -        qxl->guest_primary.bytes_pp = 2;
> -        qxl->guest_primary.bits_pp = 16;
> -        break;
> -    case SPICE_SURFACE_FMT_32_xRGB:
> -    case SPICE_SURFACE_FMT_32_ARGB:
> -        qxl->guest_primary.bytes_pp = 4;
> -        qxl->guest_primary.bits_pp = 32;
> -        break;
> -    default:
> -        fprintf(stderr, "%s: unhandled format: %x\n", __func__,
> -                qxl->guest_primary.surface.format);
> -        qxl->guest_primary.bytes_pp = 4;
> -        qxl->guest_primary.bits_pp = 32;
> -        break;
> -    }
> +    /* fallback to default bpp if format is unknown */
> +    qxl_format_bpp(qxl, sc->format, &qxl->guest_primary.bytes_pp,
> +                   &qxl->guest_primary.bits_pp);
>   }
>   
>   static void qxl_set_rect_to_surface(PCIQXLDevice *qxl, QXLRect *area)
> @@ -103,6 +86,23 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
>       int height = qxl->guest_head0_height ?: qxl->guest_primary.surface.height;
>       int i;
>   
> +    if (width <= 0 || height <= 0) {
> +        qxl_set_guest_bug(qxl, "%s: invalid dimension %dx%d",
> +                          __func__, width, height);
> +        goto end;
> +    }

The new dimension check runs before the existing !guest_primary.data 
exit. A qxl device starts in QXL_MODE_UNDEFINED with a zeroed primary 
surface, and qxl_render_update() explicitly calls this function in that 
state. Thus a host refresh—GTK, VNC, or screendump—sees 0×0 and calls 
qxl_set_guest_bug().

Below is an LLM-generated reproducer:

qemu-system-x86_64 -machine q35 -nodefaults -device qxl,id=qxl0 \
-display none -S -qmp stdio -trace enable=qxl_set_guest_bug <<'EOF'
{"execute":"qmp_capabilities"}
{"execute":"screendump","arguments":{"filename":"/dev/null","device":"qxl0","format":"ppm"}}
{"execute":"quit"}
EOF

Regards,
Akihiko Odaki

> +
> +    if (qxl->guest_primary.bytes_pp > 0) {
> +        int max_width = qxl->guest_primary.abs_stride
> +                        / qxl->guest_primary.bytes_pp;
> +        width = MIN(width, max_width);
> +    }
> +
> +    if (qxl->guest_primary.abs_stride > 0) {
> +        int max_height = qxl->vgamem_size / qxl->guest_primary.abs_stride;
> +        height = MIN(height, max_height);
> +    }
> +
>       if (qxl->guest_primary.resized) {
>           qxl->guest_primary.resized = 0;
>           qxl->guest_primary.data = qxl_phys2virt(qxl,
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index b7d871b9ee33..384b8767b8e6 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -1489,6 +1489,47 @@ static void qxl_create_guest_primary_complete(PCIQXLDevice *qxl)
>       qxl_render_resize(qxl);
>   }
>   
> +/*
> + * Convert a SpiceSurfaceFormat to bytes per pixel and bits per pixel.
> + *
> + * Only valid for surface suitable for rendering.
> + */
> +bool qxl_format_bpp(PCIQXLDevice *qxl, SpiceSurfaceFmt format,
> +                    uint32_t *bytes_pp, uint32_t *bits_pp)
> +{
> +    uint32_t bypp = 4;
> +    uint32_t bipp = 32;
> +    bool ret = true;
> +
> +    switch (format) {
> +    case SPICE_SURFACE_FMT_16_555:
> +        bypp = 2;
> +        bipp = 15;
> +        break;
> +    case SPICE_SURFACE_FMT_16_565:
> +        bypp = 2;
> +        bipp = 16;
> +        break;
> +    case SPICE_SURFACE_FMT_32_xRGB:
> +    case SPICE_SURFACE_FMT_32_ARGB:
> +        bypp = 4;
> +        bipp = 32;
> +        break;
> +    default:
> +        ret = false;
> +        qxl_set_guest_bug(qxl, "%s: unhandled format: %x", __func__, format);
> +    }
> +
> +    if (bytes_pp != NULL) {
> +        *bytes_pp = bypp;
> +    }
> +    if (bits_pp != NULL) {
> +        *bits_pp = bipp;
> +    }
> +
> +    return ret;
> +}
> +
>   static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
>                                        qxl_async_io async)
>   {
> @@ -1496,6 +1537,7 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
>       QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
>       uint32_t requested_height = le32_to_cpu(sc->height);
>       int requested_stride = le32_to_cpu(sc->stride);
> +    uint32_t bytes_pp;
>   
>       if (requested_stride == INT32_MIN ||
>           abs(requested_stride) * (uint64_t)requested_height
> @@ -1532,6 +1574,23 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
>           return;
>       }
>   
> +    if (!qxl_format_bpp(qxl, surface.format, &bytes_pp, NULL)) {
> +        return;
> +    }
> +
> +    if (surface.width == 0 || surface.height == 0) {
> +        qxl_set_guest_bug(qxl, "%s: zero dimension %ux%u",
> +                          __func__, surface.width, surface.height);
> +        return;
> +    }
> +
> +    if ((uint64_t)surface.width * bytes_pp > abs(surface.stride)) {
> +        qxl_set_guest_bug(qxl, "%s: stride too small for width:"
> +                          " stride %d width %u bpp %u",
> +                          __func__, surface.stride, surface.width, bytes_pp);
> +        return;
> +    }
> +
>       surface.mouse_mode = true;
>       surface.group_id   = MEMSLOT_GROUP_GUEST;
>       if (loadvm) {

Re: [PATCH v2] hw/display/qxl: validate primary surface stride against width

[Akihiko Odaki]

On 2026/08/04 13:54, Akihiko Odaki wrote:
> Sorry for a late review; I missed this patch.
> 
> On 2026/07/25 21:24, marcandre.lureau@redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> The existing validation in qxl_create_guest_primary() checks that
>> abs(stride) * height fits in vgamem_size and that stride is 4-byte
>> aligned, but never checks that abs(stride) is large enough to hold one
>> row of pixels for the declared width and format.
>>
>> A malicious guest can create a primary surface with a stride much
>> smaller than width * bytes_per_pixel (e.g. stride=4 for a 64-wide 32bpp
>> surface). The spice server rejects this via red_validate_surface(), but
>> the return is void and QEMU unconditionally proceeds to set up the local
>> rendering state. On the next display refresh, VNC or SDL reads width *
>> bytes_pp per scanline from a region backed by only stride bytes per
>> row, causing a host-side out-of-bounds read.
>>
>> Add three checks in qxl_create_guest_primary() before creating the
>> surface:
>>   - reject unknown surface formats
>>   - reject zero width or height
>>   - reject surfaces where abs(stride) < width * bytes_per_pixel
>>
>> Also fix three related issues in qxl-render.c:
>>   - qxl_blit() used abs_stride to advance the dst pointer into the
>>     DisplaySurface, but when stride is negative the DisplaySurface is a
>>     packed buffer whose stride may be smaller. Use surface_stride()
>>     instead.
>>   - qxl_render_update_area_unlocked() uses guest_head0_width (set via
>>     QXL_IO_MONITORS_CONFIG_ASYNC) without validating it against
>>     abs_stride, bypassing the new validation. Clamp the effective width
>>     to abs_stride / bytes_pp to prevent out-of-bounds access while
>>     tolerating the normal transient where the monitor config arrives
>>     before the primary surface is resized to match.
>>   - Similarly, guest_head0_height bypasses qxl_create_guest_primary()
>>     validation. Without clamping, abs_stride * height can overrun
>>     vgamem_size, and the product can also overflow 32 bits (e.g.
>>     abs_stride=16 MiB, height=256 wraps to zero), defeating the
>>     qxl_phys2virt() bounds check. Clamp height to
>>     vgamem_size / abs_stride to prevent both.

With negative strides, qxl_blit() walks src scanlines in reverse order. 
The starting point is determined with guest_primary.surface.height. So 
it will overrun if:

     guest_primary.surface.height
          < guest_head0_height
          <= vgamem_size / abs_stride
>>
>> Fixes: CVE-2026-16271
>> Fixes: 3761abb16784 ("hw/display/qxl: fix signed to unsigned comparison")
> 
> The Fixes: 3761abb16784 tag is incorrect. That commit only repaired 
> overflow in an existing total-size check; short-stride surfaces were 
> already accepted. The vulnerable local renderer originated in 
> a19cbfb34642, while the monitor-dimension bypass came from 979f7ef8966b 
> and the overflowable qxl_phys2virt() size expression from 
> 8efec0ef8bbc/6dbbf055148c.
> 
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3637
>> Reported-by: huntr bubble
>> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
>> ---
>> v2:
>>   - also clamp height
>>   - factor out qxl_format_bpp() helper
>> ---
>>   hw/display/qxl.h        |  2 ++
>>   hw/display/qxl-render.c | 46 ++++++++++++++++----------------
>>   hw/display/qxl.c        | 59 +++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 84 insertions(+), 23 deletions(-)
>>
>> diff --git a/hw/display/qxl.h b/hw/display/qxl.h
>> index 48d664f77736..6f5b96fe86cc 100644
>> --- a/hw/display/qxl.h
>> +++ b/hw/display/qxl.h
>> @@ -181,6 +181,8 @@ void qxl_spice_oom(PCIQXLDevice *qxl);
>>   void qxl_spice_reset_memslots(PCIQXLDevice *qxl);
>>   void qxl_spice_reset_image_cache(PCIQXLDevice *qxl);
>>   void qxl_spice_reset_cursor(PCIQXLDevice *qxl);
>> +bool qxl_format_bpp(PCIQXLDevice *qxl, SpiceSurfaceFmt format,
>> +                    uint32_t *bytes_pp, uint32_t *bits_pp);
>>   /* qxl-logger.c */
>>   int qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int 
>> group_id);
>> diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
>> index 4799c9e8befd..b0a71a95ad66 100644
>> --- a/hw/display/qxl-render.c
>> +++ b/hw/display/qxl-render.c
>> @@ -27,6 +27,7 @@
>>   static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
>>   {
>>       DisplaySurface *surface = qemu_console_surface(qxl->vga.con);
>> +    int dst_stride = surface_stride(surface);
>>       uint8_t *dst = surface_data(surface);
>>       uint8_t *src;
>>       int len, i;
>> @@ -45,14 +46,14 @@ static void qxl_blit(PCIQXLDevice *qxl, QXLRect 
>> *rect)
>>       } else {
>>           src += rect->top * qxl->guest_primary.abs_stride;
>>       }
>> -    dst += rect->top  * qxl->guest_primary.abs_stride;
>> +    dst += rect->top  * dst_stride;
>>       src += rect->left * qxl->guest_primary.bytes_pp;
>>       dst += rect->left * qxl->guest_primary.bytes_pp;
>>       len  = (rect->right - rect->left) * qxl->guest_primary.bytes_pp;
>>       for (i = rect->top; i < rect->bottom; i++) {
>>           memcpy(dst, src, len);
>> -        dst += qxl->guest_primary.abs_stride;
>> +        dst += dst_stride;
>>           src += qxl->guest_primary.qxl_stride;
>>       }
>>   }
>> @@ -64,27 +65,9 @@ void qxl_render_resize(PCIQXLDevice *qxl)
>>       qxl->guest_primary.qxl_stride = sc->stride;
>>       qxl->guest_primary.abs_stride = abs(sc->stride);
>>       qxl->guest_primary.resized++;
>> -    switch (sc->format) {
>> -    case SPICE_SURFACE_FMT_16_555:
>> -        qxl->guest_primary.bytes_pp = 2;
>> -        qxl->guest_primary.bits_pp = 15;
>> -        break;
>> -    case SPICE_SURFACE_FMT_16_565:
>> -        qxl->guest_primary.bytes_pp = 2;
>> -        qxl->guest_primary.bits_pp = 16;
>> -        break;
>> -    case SPICE_SURFACE_FMT_32_xRGB:
>> -    case SPICE_SURFACE_FMT_32_ARGB:
>> -        qxl->guest_primary.bytes_pp = 4;
>> -        qxl->guest_primary.bits_pp = 32;
>> -        break;
>> -    default:
>> -        fprintf(stderr, "%s: unhandled format: %x\n", __func__,
>> -                qxl->guest_primary.surface.format);
>> -        qxl->guest_primary.bytes_pp = 4;
>> -        qxl->guest_primary.bits_pp = 32;
>> -        break;
>> -    }
>> +    /* fallback to default bpp if format is unknown */
>> +    qxl_format_bpp(qxl, sc->format, &qxl->guest_primary.bytes_pp,
>> +                   &qxl->guest_primary.bits_pp);
>>   }
>>   static void qxl_set_rect_to_surface(PCIQXLDevice *qxl, QXLRect *area)
>> @@ -103,6 +86,23 @@ static void 
>> qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
>>       int height = qxl->guest_head0_height ?: qxl- 
>> >guest_primary.surface.height;
>>       int i;
>> +    if (width <= 0 || height <= 0) {
>> +        qxl_set_guest_bug(qxl, "%s: invalid dimension %dx%d",
>> +                          __func__, width, height);
>> +        goto end;
>> +    }
> 
> The new dimension check runs before the existing !guest_primary.data 
> exit. A qxl device starts in QXL_MODE_UNDEFINED with a zeroed primary 
> surface, and qxl_render_update() explicitly calls this function in that 
> state. Thus a host refresh—GTK, VNC, or screendump—sees 0×0 and calls 
> qxl_set_guest_bug().
> 
> Below is an LLM-generated reproducer:
> 
> qemu-system-x86_64 -machine q35 -nodefaults -device qxl,id=qxl0 \
> -display none -S -qmp stdio -trace enable=qxl_set_guest_bug <<'EOF'
> {"execute":"qmp_capabilities"}
> {"execute":"screendump","arguments":{"filename":"/dev/ 
> null","device":"qxl0","format":"ppm"}}
> {"execute":"quit"}
> EOF
> 
> Regards,
> Akihiko Odaki
> 
>> +
>> +    if (qxl->guest_primary.bytes_pp > 0) {
>> +        int max_width = qxl->guest_primary.abs_stride
>> +                        / qxl->guest_primary.bytes_pp;
>> +        width = MIN(width, max_width);
>> +    }
>> +
>> +    if (qxl->guest_primary.abs_stride > 0) {
>> +        int max_height = qxl->vgamem_size / qxl- 
>> >guest_primary.abs_stride;
>> +        height = MIN(height, max_height);
>> +    }
>> +
>>       if (qxl->guest_primary.resized) {
>>           qxl->guest_primary.resized = 0;
>>           qxl->guest_primary.data = qxl_phys2virt(qxl,
>> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
>> index b7d871b9ee33..384b8767b8e6 100644
>> --- a/hw/display/qxl.c
>> +++ b/hw/display/qxl.c
>> @@ -1489,6 +1489,47 @@ static void 
>> qxl_create_guest_primary_complete(PCIQXLDevice *qxl)
>>       qxl_render_resize(qxl);
>>   }
>> +/*
>> + * Convert a SpiceSurfaceFormat to bytes per pixel and bits per pixel.
>> + *
>> + * Only valid for surface suitable for rendering.
>> + */
>> +bool qxl_format_bpp(PCIQXLDevice *qxl, SpiceSurfaceFmt format,
>> +                    uint32_t *bytes_pp, uint32_t *bits_pp)
>> +{
>> +    uint32_t bypp = 4;
>> +    uint32_t bipp = 32;
>> +    bool ret = true;
>> +
>> +    switch (format) {
>> +    case SPICE_SURFACE_FMT_16_555:
>> +        bypp = 2;
>> +        bipp = 15;
>> +        break;
>> +    case SPICE_SURFACE_FMT_16_565:
>> +        bypp = 2;
>> +        bipp = 16;
>> +        break;
>> +    case SPICE_SURFACE_FMT_32_xRGB:
>> +    case SPICE_SURFACE_FMT_32_ARGB:
>> +        bypp = 4;
>> +        bipp = 32;
>> +        break;
>> +    default:
>> +        ret = false;
>> +        qxl_set_guest_bug(qxl, "%s: unhandled format: %x", __func__, 
>> format);
>> +    }
>> +
>> +    if (bytes_pp != NULL) {
>> +        *bytes_pp = bypp;
>> +    }
>> +    if (bits_pp != NULL) {
>> +        *bits_pp = bipp;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>>   static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
>>                                        qxl_async_io async)
>>   {
>> @@ -1496,6 +1537,7 @@ static void 
>> qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
>>       QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
>>       uint32_t requested_height = le32_to_cpu(sc->height);
>>       int requested_stride = le32_to_cpu(sc->stride);
>> +    uint32_t bytes_pp;
>>       if (requested_stride == INT32_MIN ||
>>           abs(requested_stride) * (uint64_t)requested_height
>> @@ -1532,6 +1574,23 @@ static void 
>> qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
>>           return;
>>       }
>> +    if (!qxl_format_bpp(qxl, surface.format, &bytes_pp, NULL)) {
>> +        return;
>> +    }
>> +
>> +    if (surface.width == 0 || surface.height == 0) {
>> +        qxl_set_guest_bug(qxl, "%s: zero dimension %ux%u",
>> +                          __func__, surface.width, surface.height);
>> +        return;
>> +    }
>> +
>> +    if ((uint64_t)surface.width * bytes_pp > abs(surface.stride)) {
>> +        qxl_set_guest_bug(qxl, "%s: stride too small for width:"
>> +                          " stride %d width %u bpp %u",
>> +                          __func__, surface.stride, surface.width, 
>> bytes_pp);
>> +        return;
>> +    }
>> +
>>       surface.mouse_mode = true;
>>       surface.group_id   = MEMSLOT_GROUP_GUEST;
>>       if (loadvm) {
> 
>