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

marcandre.lureau@redhat.com posted 1 patch 14 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260723135909.429714-1-marcandre.lureau@redhat.com
hw/display/qxl-render.c | 11 +++++++++--
hw/display/qxl.c        | 29 +++++++++++++++++++++++++++++
2 files changed, 38 insertions(+), 2 deletions(-)
[PATCH v2] hw/display/qxl: validate primary surface stride against width
Posted by marcandre.lureau@redhat.com 14 hours ago
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