Commit ab7183ed4e for qemu.org

commit ab7183ed4eecb4727532e3ffe5953d127e102c72
Author: Marc-André Lureau <marcandre.lureau@redhat.com>
Date:   Thu Aug 6 13:40:27 2026 +0400

    hw/display/qxl: validate primary surface stride against width

    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.

    While touch it, fix some endianness issues.

    Fixes: CVE-2026-16271
    Fixes: a19cbfb34642 ("spice: add qxl device")
    Fixes: 979f7ef8966b ("qxl: use guest_monitor_config for local renderer.")
    Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3637
    Reported-by: huntr bubble
    Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
    Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
    Message-ID: <20260806094028.640676-1-marcandre.lureau@redhat.com>

diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index 4799c9e8be..348ddba768 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;
     }
 }
@@ -61,30 +62,13 @@ void qxl_render_resize(PCIQXLDevice *qxl)
 {
     QXLSurfaceCreate *sc = &qxl->guest_primary.surface;

-    qxl->guest_primary.qxl_stride = sc->stride;
-    qxl->guest_primary.abs_stride = abs(sc->stride);
+    qxl->guest_primary.qxl_stride = le32_to_cpu(sc->stride);
+    qxl->guest_primary.abs_stride = abs(qxl->guest_primary.qxl_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, le32_to_cpu(sc->format),
+                   &qxl->guest_primary.bytes_pp,
+                   &qxl->guest_primary.bits_pp);
 }

 static void qxl_set_rect_to_surface(PCIQXLDevice *qxl, QXLRect *area)
@@ -101,15 +85,45 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
     DisplaySurface *surface;
     int width = qxl->guest_head0_width ?: qxl->guest_primary.surface.width;
     int height = qxl->guest_head0_height ?: qxl->guest_primary.surface.height;
+    uint64_t map_height;
     int i;

+    if (width <= 0 || height <= 0) {
+        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.qxl_stride < 0) {
+        /* qxl_blit() uses the primary height to find the first scanline. */
+        height = MIN(height, (int)qxl->guest_primary.surface.height);
+    }
+
+    if (qxl->guest_primary.abs_stride > 0) {
+        int max_height = qxl->vgamem_size / qxl->guest_primary.abs_stride;
+        height = MIN(height, max_height);
+    }
+
+    /*
+     * height limits the visible update, while map_height is the guest memory
+     * span validated by qxl_phys2virt().  With a negative stride qxl_blit()
+     * addresses scanlines from the declared primary height, so a shorter
+     * monitor still requires validating the full primary surface.
+     */
+    map_height = qxl->guest_primary.qxl_stride < 0 ?
+                 qxl->guest_primary.surface.height : height;
+
     if (qxl->guest_primary.resized) {
         qxl->guest_primary.resized = 0;
         qxl->guest_primary.data = qxl_phys2virt(qxl,
                                                 qxl->guest_primary.surface.mem,
                                                 MEMSLOT_GROUP_GUEST,
                                                 qxl->guest_primary.abs_stride
-                                                * height);
+                                                * map_height);
         if (!qxl->guest_primary.data) {
             goto end;
         }
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index b7d871b9ee..384b8767b8 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) {
diff --git a/hw/display/qxl.h b/hw/display/qxl.h
index 48d664f777..6f5b96fe86 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);