[QEMU HCI-8.0 PATCH 0/7] qxl cursor use-after-free plus stability backports #VSTOR-144000

[QEMU HCI-8.0 PATCH 0/7] qxl cursor use-after-free plus stability backports #VSTOR-144000

[Denis V. Lunev]

A guest with a qxl display can make QEMU drop more references to a
QEMUCursor than were taken. The cursor is freed while another owner
still points at it, and that owner's later cursor_unref() decrements
four bytes of a chunk the allocator has handed out again. Nothing
aborts and nothing is logged; QEMU dies later in an unrelated
allocation, in another thread.

Two defects get there, and neither fix is sufficient alone:

 - qxl_spice_reset_cursor() replaces qxl->ssd.cursor with no lock held,
   while every other writer of that field takes ssd.lock. It runs on a
   vCPU thread from QXL_IO_DESTROY_PRIMARY and, unlike qxl_hard_reset(),
   leaves the SPICE display worker running.

 - QEMUCursor.refcount is a plain int, taken and dropped from the main
   loop, the SPICE worker, ui/cocoa.m and ui/dbus-listener.c, with no
   lock common to all of them, so an increment can be lost.

A qxl device starts a spice-server instance for local rendering even
with no -spice, so this is not limited to SPICE console setups.

Patch 2 also asserts that the refcount was positive. Only qxl was
exercised here, so if another display backend drops a reference it
never took, that assert turns a silent leak into an abort.

Reproducer: a libdrm program in the guest queues cursor SET commands,
then disables the CRTC so the driver issues QXL_IO_DESTROY_PRIMARY.
Unpatched QEMU dies within seconds; with the series it does not.

Patches 1 and 2 were posted upstream and carry Marc-Andre's
Reviewed-by, but are not merged yet, so they have no cherry-pick line:

  https://lore.kernel.org/qemu-devel/20260903192647.2677279-1-den@openvz.org/

Patches 3 to 7 are definitive stability fixes, cherry-picked from
mainstream. All five are already reviewed and merged upstream, and none
of them was present on our branch. They are unrelated to the crash
above but sit in the same device, so they are worth taking in one go:

  3/7  mono cursor validation reading past a cursor chunk
  4/7  TOCTOU in cursor chunk data_size handling
  5/7  monitors_config heads[] validation in phys2virt
  6/7  vm_change_state handler and BHs left registered on unrealize
  7/7  primary surface stride not validated against width

Two of those carry CVE references in their upstream messages, 7/7
CVE-2026-16271 and 6/7 CVE-2026-63322.

None of these seven touches qxl_post_load(), so none of them addresses
the migration failure tracked separately in VSTOR-113533.

Denis V. Lunev (2):
  hw/display/qxl: hold ssd.lock while replacing ssd.cursor #VSTOR-144000
  ui/cursor: make the cursor refcount atomic #VSTOR-144000

Haotian Jiang (1):
  hw/display/qxl: unregister vm_change_state handler and BHs
    #VSTOR-144000

Marc-André Lureau (3):
  hw/display/qxl: fix TOCTOU in cursor chunk data_size handling
    #VSTOR-144000
  hw/display/qxl: validate monitors_config heads[] in phys2virt
    #VSTOR-144000
  hw/display/qxl: validate primary surface stride against width
    #VSTOR-144000

Thomas Huth (1):
  hw/display/qxl: Fix mono cursor validation that can read past a cursor
    chunk #VSTOR-144000

 hw/display/qxl-render.c | 98 +++++++++++++++++++++++++----------------
 hw/display/qxl.c        | 79 ++++++++++++++++++++++++++++++++-
 hw/display/qxl.h        |  3 ++
 include/ui/console.h    |  9 ++++
 ui/cursor.c             | 17 ++++---
 5 files changed, 160 insertions(+), 46 deletions(-)

-- 
2.53.0

[QEMU HCI-8.0 PATCH 1/7] hw/display/qxl: hold ssd.lock while replacing ssd.cursor #VSTOR-144000

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

qxl_spice_reset_cursor() unrefs qxl->ssd.cursor and installs the hidden
cursor without holding qxl->ssd.lock. Every other writer of that field
takes it: qxl_render_cursor(), display_mouse_define() and
qemu_spice_cursor_refresh_bh().

The unlocked path runs on a vCPU thread, reached from ioport_write() on
QXL_IO_DESTROY_PRIMARY and QXL_IO_DESTROY_PRIMARY_ASYNC, and holds only
the BQL, which the SPICE display worker never takes. Unlike
qxl_hard_reset(), it leaves that worker running.
spice_qxl_reset_cursor() does round trip through the dispatcher, but the
worker is free again as soon as it returns, so it can enter
qxl_render_cursor() and unref the same QEMUCursor a few instructions
later. Both threads then drop one reference for what is a single
reference, freeing a cursor that another user still holds. The store to
ssd.cursor races the same way, and a guest that keeps this up also ends
up waiting forever in qxl_fence_wait().

A guest reaches this by switching QXL mode while it also updates the
pointer shape.

Fixes: 958c2bceba06 ("qxl: fix cursor reset")
Cc: qemu-stable@nongnu.org
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/display/qxl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 5f53d54073b..7cccfa405d0 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -299,10 +299,12 @@ void qxl_spice_reset_cursor(PCIQXLDevice *qxl)
     qemu_mutex_lock(&qxl->track_lock);
     qxl->guest_cursor = 0;
     qemu_mutex_unlock(&qxl->track_lock);
+    qemu_mutex_lock(&qxl->ssd.lock);
     if (qxl->ssd.cursor) {
         cursor_unref(qxl->ssd.cursor);
     }
     qxl->ssd.cursor = cursor_builtin_hidden();
+    qemu_mutex_unlock(&qxl->ssd.lock);
 }
 
 static uint32_t qxl_crc32(const uint8_t *p, unsigned len)
-- 
2.53.0

[QEMU HCI-8.0 PATCH 2/7] ui/cursor: make the cursor refcount atomic #VSTOR-144000

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

A QEMUCursor outlives the call that publishes it and is shared between
threads, but its refcount was a plain int with no single lock covering
every user. qemu_console_set_cursor() takes and drops references from
the main loop under the BQL alone, hw/display/qxl-render.c does so from
the SPICE display worker thread, and ui/spice-display.c does so under
SimpleSpiceDisplay::lock. ui/cocoa.m and ui/dbus-listener.c add two more
threads.

The pair that collides is qemu_spice_cursor_refresh_bh(), which drops
ssd->lock before calling qemu_console_set_cursor(), and the worker
refcounting the same cursor under that lock. A lost increment frees the
cursor while the console still points at it, so the console's next unref
decrements memory the allocator has already handed out again. Locking
ssd.cursor is not enough on its own: with that done, this is the race
that remains.

Assert on the value the decrement observed while here. Dropping a
reference that was never taken used to be silent, because the decrement
lands in the allocator metadata of the freed chunk: nothing is logged,
the object is not freed twice, and the process runs on until some later
allocation walks the damaged free list and faults, arbitrarily far from
the code that caused it.

Fixes: 0b2824e5e48a ("spice: use bottom half instead of refresh timer for cursor updates")
Cc: qemu-stable@nongnu.org
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/console.h |  9 +++++++++
 ui/cursor.c          | 17 +++++++++++------
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 98feaa58bdd..a5db0b4e396 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -164,6 +164,15 @@ typedef struct QEMUCursor {
 } QEMUCursor;
 
 QEMUCursor *cursor_alloc(uint16_t width, uint16_t height);
+
+/*
+ * A cursor may be shared between the main loop, a vCPU thread and a
+ * display backend's own thread, so the refcount is atomic and these two
+ * may be called from any of them.  The object itself is not otherwise
+ * thread-safe: take a reference before publishing the pointer anywhere
+ * another thread can reach it, and never dereference a cursor you do
+ * not hold a reference to.
+ */
 QEMUCursor *cursor_ref(QEMUCursor *c);
 void cursor_unref(QEMUCursor *c);
 QEMUCursor *cursor_builtin_hidden(void);
diff --git a/ui/cursor.c b/ui/cursor.c
index 6e23244fbe6..69d27d49a13 100644
--- a/ui/cursor.c
+++ b/ui/cursor.c
@@ -1,4 +1,5 @@
 #include "qemu/osdep.h"
+#include "qemu/atomic.h"
 #include "ui/console.h"
 
 #include "cursor_hidden.xpm"
@@ -103,24 +104,28 @@ QEMUCursor *cursor_alloc(uint16_t width, uint16_t height)
     c = g_malloc0(sizeof(QEMUCursor) + datasize);
     c->width  = width;
     c->height = height;
-    c->refcount = 1;
+    qatomic_set(&c->refcount, 1);
     return c;
 }
 
 QEMUCursor *cursor_ref(QEMUCursor *c)
 {
-    c->refcount++;
+    qatomic_inc(&c->refcount);
     return c;
 }
 
 void cursor_unref(QEMUCursor *c)
 {
+    int refcount;
+
     if (c == NULL)
         return;
-    c->refcount--;
-    if (c->refcount)
-        return;
-    g_free(c);
+
+    refcount = qatomic_fetch_dec(&c->refcount);
+    assert(refcount > 0);
+    if (refcount == 1) {
+        g_free(c);
+    }
 }
 
 int cursor_get_mono_bpl(QEMUCursor *c)
-- 
2.53.0

[QEMU HCI-8.0 PATCH 3/7] hw/display/qxl: Fix mono cursor validation that can read past a cursor chunk #VSTOR-144000

[Denis V. Lunev]

From: Thomas Huth <thuth@redhat.com>

qxl_render_cursor() maps the guest-provided QXLCursor object using the
guest-controlled cursor->chunk.data_size.
For a mono cursor, qxl_cursor() then validates the expected bitmap size
against cursor->data_size, but it does not validate that the first chunk
actually contains that many bytes.
A guest could set cursor->data_size to the correct full mono cursor size
while setting cursor->chunk.data_size to zero. In that case, cursor_set_mono()
reads the AND/XOR masks starting at cursor->chunk.data. If the cursor object
is placed at the end of the QXL RAM BAR, those reads cross the mapped RAM
region and could crash the QEMU process (e.g. under ASan).

Fix it by double-checking cursor->chunk.data_size for the correct size.

This patch is based on the suggested changes by the reporter in the bug
ticket.

Reported-by: huntr bubble
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3646
Signed-off-by: Thomas Huth <thuth@redhat.com>
Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-ID: <20260630101022.379057-1-thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
(cherry picked from commit 0e51b71c7b7706923536c1f7923cace82877932b)
---
 hw/display/qxl-render.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index c6a9ac1da10..1fe63b6f5ca 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -272,9 +272,11 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor,
     case SPICE_CURSOR_TYPE_MONO:
         /* Assume that the full cursor is available in a single chunk. */
         size = 2 * cursor_get_mono_bpl(c) * c->height;
-        if (size != cursor->data_size) {
-            fprintf(stderr, "%s: bad monochrome cursor %ux%u with size %u\n",
-                    __func__, c->width, c->height, cursor->data_size);
+        if (size != cursor->data_size || cursor->chunk.data_size < size) {
+            qxl_set_guest_bug(qxl, "%s: bad monochrome cursor %ux%u"
+                              " data_size %u chunk_size %u",
+                              __func__, c->width, c->height,
+                              cursor->data_size, cursor->chunk.data_size);
             goto fail;
         }
         and_mask = cursor->chunk.data;
-- 
2.53.0

[QEMU HCI-8.0 PATCH 4/7] hw/display/qxl: fix TOCTOU in cursor chunk data_size handling #VSTOR-144000

[Denis V. Lunev]

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

Snapshot chunk.data_size into a host-local variable before passing it to
qxl_phys2virt() for validation, and pass it through qxl_cursor() and
qxl_unpack_chunks() so that no subsequent code re-reads the field.

Without this, a racing vCPU can inflate data_size between the
qxl_phys2virt() validation and the memcpy in qxl_unpack_chunks(),
causing a source read past the validated region. In practice the read
stays within the guest's own VRAM mmap, so the impact is limited.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3757
Reported-by: Feifan Qian <bea1e@proton.me>
Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Message-ID: <20260710134352.2313675-1-marcandre.lureau@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
(cherry picked from commit a3cc0069e151e5eb5db57bb4e86b00861d5b97ab)
---
 hw/display/qxl-render.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index 1fe63b6f5ca..cf0f6849cd0 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -217,7 +217,8 @@ void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie)
 }
 
 static void qxl_unpack_chunks(void *dest, size_t size, PCIQXLDevice *qxl,
-                              QXLDataChunk *chunk, uint32_t group_id)
+                              QXLDataChunk *chunk, uint32_t group_id,
+                              uint32_t chunk_data_size)
 {
     uint32_t max_chunks = 32;
     size_t offset = 0;
@@ -225,22 +226,21 @@ static void qxl_unpack_chunks(void *dest, size_t size, PCIQXLDevice *qxl,
     QXLPHYSICAL next_chunk_phys = 0;
 
     for (;;) {
-        bytes = MIN(size - offset, chunk->data_size);
+        bytes = MIN(size - offset, chunk_data_size);
         memcpy(dest + offset, chunk->data, bytes);
         offset += bytes;
         if (offset == size) {
             return;
         }
         next_chunk_phys = chunk->next_chunk;
-        /* fist time, only get the next chunk's data size */
         chunk = qxl_phys2virt(qxl, next_chunk_phys, group_id,
                               sizeof(QXLDataChunk));
         if (!chunk) {
             return;
         }
-        /* second time, check data size and get data */
+        chunk_data_size = chunk->data_size;
         chunk = qxl_phys2virt(qxl, next_chunk_phys, group_id,
-                              sizeof(QXLDataChunk) + chunk->data_size);
+                              sizeof(QXLDataChunk) + chunk_data_size);
         if (!chunk) {
             return;
         }
@@ -252,7 +252,7 @@ static void qxl_unpack_chunks(void *dest, size_t size, PCIQXLDevice *qxl,
 }
 
 static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor,
-                              uint32_t group_id)
+                              uint32_t group_id, uint32_t chunk_data_size)
 {
     QEMUCursor *c;
     uint8_t *and_mask, *xor_mask;
@@ -272,11 +272,11 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor,
     case SPICE_CURSOR_TYPE_MONO:
         /* Assume that the full cursor is available in a single chunk. */
         size = 2 * cursor_get_mono_bpl(c) * c->height;
-        if (size != cursor->data_size || cursor->chunk.data_size < size) {
+        if (size != cursor->data_size || chunk_data_size < size) {
             qxl_set_guest_bug(qxl, "%s: bad monochrome cursor %ux%u"
                               " data_size %u chunk_size %u",
                               __func__, c->width, c->height,
-                              cursor->data_size, cursor->chunk.data_size);
+                              cursor->data_size, chunk_data_size);
             goto fail;
         }
         and_mask = cursor->chunk.data;
@@ -288,7 +288,8 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor,
         break;
     case SPICE_CURSOR_TYPE_ALPHA:
         size = sizeof(uint32_t) * c->width * c->height;
-        qxl_unpack_chunks(c->data, size, qxl, &cursor->chunk, group_id);
+        qxl_unpack_chunks(c->data, size, qxl, &cursor->chunk, group_id,
+                          chunk_data_size);
         if (qxl->debug > 2) {
             cursor_print_ascii_art(c, "qxl/alpha");
         }
@@ -325,19 +326,23 @@ int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
     }
     switch (cmd->type) {
     case QXL_CURSOR_SET:
+    {
+        uint32_t chunk_data_size;
+
         /* First read the QXLCursor to get QXLDataChunk::data_size ... */
         cursor = qxl_phys2virt(qxl, cmd->u.set.shape, ext->group_id,
                                sizeof(QXLCursor));
         if (!cursor) {
             return 1;
         }
+        chunk_data_size = cursor->chunk.data_size;
         /* Then read including the chunked data following QXLCursor. */
         cursor = qxl_phys2virt(qxl, cmd->u.set.shape, ext->group_id,
-                               sizeof(QXLCursor) + cursor->chunk.data_size);
+                               sizeof(QXLCursor) + chunk_data_size);
         if (!cursor) {
             return 1;
         }
-        c = qxl_cursor(qxl, cursor, ext->group_id);
+        c = qxl_cursor(qxl, cursor, ext->group_id, chunk_data_size);
         if (c == NULL) {
             c = cursor_builtin_left_ptr();
         }
@@ -351,6 +356,7 @@ int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
         qemu_mutex_unlock(&qxl->ssd.lock);
         qemu_bh_schedule(qxl->ssd.cursor_bh);
         break;
+    }
     case QXL_CURSOR_MOVE:
         qemu_mutex_lock(&qxl->ssd.lock);
         qxl->ssd.mouse_x = cmd->u.position.x;
-- 
2.53.0

[QEMU HCI-8.0 PATCH 5/7] hw/display/qxl: validate monitors_config heads[] in phys2virt #VSTOR-144000

[Denis V. Lunev]

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

The qxl_phys2virt() call for guest_monitors_config only validates
sizeof(QXLMonitorsConfig), which covers the fixed header (count and
max_allowed) since commit 8efec0ef8bbc ("hw/display/qxl: Pass requested
buffer size to qxl_phys2virt()"), but not the flexible array member
heads[]. When count == 1, heads[0] is accessed without its memory being
validated, allowing a guest to cause an out-of-bounds read.

Include sizeof(QXLHead) in the size passed to qxl_phys2virt() so that
the first head entry is validated within the guest memory slot, preventing
guest-visible memory reading.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4027
Reported-by: Tristan @TristanInSec
Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Message-ID: <20260715072722.1643289-1-marcandre.lureau@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
(cherry picked from commit 9f436938c52125324e36cf9c13b877aa8aada096)
---
 hw/display/qxl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 7cccfa405d0..a6eea18730e 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -275,7 +275,7 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl, int replay)
     }
 
     cfg = qxl_phys2virt(qxl, qxl->guest_monitors_config, MEMSLOT_GROUP_GUEST,
-                        sizeof(QXLMonitorsConfig));
+                        sizeof(QXLMonitorsConfig) + sizeof(QXLHead));
     if (cfg != NULL && cfg->count == 1) {
         qxl->guest_primary.resized = 1;
         qxl->guest_head0_width  = cfg->heads[0].width;
-- 
2.53.0

[QEMU HCI-8.0 PATCH 6/7] hw/display/qxl: unregister vm_change_state handler and BHs #VSTOR-144000

[Denis V. Lunev]

From: Haotian Jiang <sundayjiang@tencent.com>

qxl_realize_common() registers a vm_change_state handler via
qemu_add_vm_change_state_handler() and creates three bottom halves
(update_irq, update_area_bh, cursor_bh), but none are ever cleaned up.
The return value of qemu_add_vm_change_state_handler() is discarded, so
the handler is never removed from the global list, and there is no
PCIDeviceClass.exit callback to delete the BHs.

When a secondary QXL device (hotpluggable by default) is hot-unplugged
via device_del, the PCIQXLDevice memory is freed but the vm_state
handler and BH entries remain with dangling opaque pointers. On the
next VM state change (stop/cont/migrate) or BH dispatch, the callback
dereferences freed memory, causing a use-after-free.

Fix this by storing the VMChangeStateEntry returned by
qemu_add_vm_change_state_handler() and adding a qxl_exit() callback
that deletes the vm_state handler, all three BHs, and the
guest_surfaces.cmds allocation before the device memory is freed.

Fixes: a19cbfb34642 ("spice: add qxl device")
Fixes: CVE-2026-63322
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3607
Signed-off-by: Haotian Jiang <jianghaotian.sunday@gmail.com>
Cc: qemu-stable@nongnu.org
[ Marc-André - tweak commit message, add TODO ]
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-ID: <20260720024855.3757499-1-jianghaotian.sunday@gmail.com>
(cherry picked from commit 4727cc883b7e81d2c30b9801af54482d65b84292)
---
 hw/display/qxl.c | 16 +++++++++++++++-
 hw/display/qxl.h |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index a6eea18730e..994bfcaa522 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -2225,7 +2225,8 @@ static void qxl_realize_common(PCIQXLDevice *qxl, Error **errp)
     }
 #endif
 
-    qemu_add_vm_change_state_handler(qxl_vm_change_state_handler, qxl);
+    qxl->vmstate_handler =
+        qemu_add_vm_change_state_handler(qxl_vm_change_state_handler, qxl);
 
     qxl->update_irq = qemu_bh_new_guarded(qxl_update_irq_bh, qxl,
                                           &DEVICE(qxl)->mem_reentrancy_guard);
@@ -2504,6 +2505,18 @@ static const Property qxl_properties[] = {
                             subsystem_vendor_id, PCI_VENDOR_ID_REDHAT_QUMRANET),
 };
 
+static void qxl_exit(PCIDevice *dev)
+{
+    PCIQXLDevice *qxl = PCI_QXL(dev);
+
+    /* TODO: complete cleanup, error paths etc */
+    g_clear_pointer(&qxl->vmstate_handler, qemu_del_vm_change_state_handler);
+    g_clear_pointer(&qxl->update_irq, qemu_bh_delete);
+    g_clear_pointer(&qxl->update_area_bh, qemu_bh_delete);
+    g_clear_pointer(&qxl->ssd.cursor_bh, qemu_bh_delete);
+    g_clear_pointer(&qxl->guest_surfaces.cmds, g_free);
+}
+
 static void qxl_pci_class_init(ObjectClass *klass, const void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -2511,6 +2524,7 @@ static void qxl_pci_class_init(ObjectClass *klass, const void *data)
 
     k->vendor_id = REDHAT_PCI_VENDOR_ID;
     k->device_id = QXL_DEVICE_ID_STABLE;
+    k->exit = qxl_exit;
     set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
     device_class_set_legacy_reset(dc, qxl_reset_handler);
     dc->vmsd = &qxl_vmstate;
diff --git a/hw/display/qxl.h b/hw/display/qxl.h
index 4afdbdd1ed8..a25d9865453 100644
--- a/hw/display/qxl.h
+++ b/hw/display/qxl.h
@@ -83,6 +83,7 @@ struct PCIQXLDevice {
 
     /* thread signaling */
     QEMUBH             *update_irq;
+    VMChangeStateEntry *vmstate_handler;
 
     /* ram pci bar */
     QXLRam             *ram;
-- 
2.53.0

[QEMU HCI-8.0 PATCH 7/7] hw/display/qxl: validate primary surface stride against width #VSTOR-144000

[Denis V. Lunev]

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.

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>
(cherry picked from commit ab7183ed4eecb4727532e3ffe5953d127e102c72)
---
 hw/display/qxl-render.c | 66 +++++++++++++++++++++++++----------------
 hw/display/qxl.c        | 59 ++++++++++++++++++++++++++++++++++++
 hw/display/qxl.h        |  2 ++
 3 files changed, 101 insertions(+), 26 deletions(-)

diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index cf0f6849cd0..f2bc25f03ad 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 994bfcaa522..f0cf346c430 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1509,6 +1509,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)
 {
@@ -1516,6 +1557,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
@@ -1552,6 +1594,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 a25d9865453..ed5f71c0a3a 100644
--- a/hw/display/qxl.h
+++ b/hw/display/qxl.h
@@ -182,6 +182,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);
-- 
2.53.0

Re: [QEMU HCI-8.0 PATCH 0/7] qxl cursor use-after-free plus stability backports #VSTOR-144000

[Andrey Drobyshev]

Ack
Applied to hci-8.0

On 9/3/26 11:25 PM, Denis V. Lunev wrote:
> A guest with a qxl display can make QEMU drop more references to a
> QEMUCursor than were taken. The cursor is freed while another owner
> still points at it, and that owner's later cursor_unref() decrements
> four bytes of a chunk the allocator has handed out again. Nothing
> aborts and nothing is logged; QEMU dies later in an unrelated
> allocation, in another thread.
> 
> Two defects get there, and neither fix is sufficient alone:
> 
>  - qxl_spice_reset_cursor() replaces qxl->ssd.cursor with no lock held,
>    while every other writer of that field takes ssd.lock. It runs on a
>    vCPU thread from QXL_IO_DESTROY_PRIMARY and, unlike qxl_hard_reset(),
>    leaves the SPICE display worker running.
> 
>  - QEMUCursor.refcount is a plain int, taken and dropped from the main
>    loop, the SPICE worker, ui/cocoa.m and ui/dbus-listener.c, with no
>    lock common to all of them, so an increment can be lost.
> 
> A qxl device starts a spice-server instance for local rendering even
> with no -spice, so this is not limited to SPICE console setups.
> 
> Patch 2 also asserts that the refcount was positive. Only qxl was
> exercised here, so if another display backend drops a reference it
> never took, that assert turns a silent leak into an abort.
> 
> Reproducer: a libdrm program in the guest queues cursor SET commands,
> then disables the CRTC so the driver issues QXL_IO_DESTROY_PRIMARY.
> Unpatched QEMU dies within seconds; with the series it does not.
> 
> Patches 1 and 2 were posted upstream and carry Marc-Andre's
> Reviewed-by, but are not merged yet, so they have no cherry-pick line:
> 
>   https://lore.kernel.org/qemu-devel/20260903192647.2677279-1-den@openvz.org/
> 
> Patches 3 to 7 are definitive stability fixes, cherry-picked from
> mainstream. All five are already reviewed and merged upstream, and none
> of them was present on our branch. They are unrelated to the crash
> above but sit in the same device, so they are worth taking in one go:
> 
>   3/7  mono cursor validation reading past a cursor chunk
>   4/7  TOCTOU in cursor chunk data_size handling
>   5/7  monitors_config heads[] validation in phys2virt
>   6/7  vm_change_state handler and BHs left registered on unrealize
>   7/7  primary surface stride not validated against width
> 
> Two of those carry CVE references in their upstream messages, 7/7
> CVE-2026-16271 and 6/7 CVE-2026-63322.
> 
> None of these seven touches qxl_post_load(), so none of them addresses
> the migration failure tracked separately in VSTOR-113533.
> 
> Denis V. Lunev (2):
>   hw/display/qxl: hold ssd.lock while replacing ssd.cursor #VSTOR-144000
>   ui/cursor: make the cursor refcount atomic #VSTOR-144000
> 
> Haotian Jiang (1):
>   hw/display/qxl: unregister vm_change_state handler and BHs
>     #VSTOR-144000
> 
> Marc-André Lureau (3):
>   hw/display/qxl: fix TOCTOU in cursor chunk data_size handling
>     #VSTOR-144000
>   hw/display/qxl: validate monitors_config heads[] in phys2virt
>     #VSTOR-144000
>   hw/display/qxl: validate primary surface stride against width
>     #VSTOR-144000
> 
> Thomas Huth (1):
>   hw/display/qxl: Fix mono cursor validation that can read past a cursor
>     chunk #VSTOR-144000
> 
>  hw/display/qxl-render.c | 98 +++++++++++++++++++++++++----------------
>  hw/display/qxl.c        | 79 ++++++++++++++++++++++++++++++++-
>  hw/display/qxl.h        |  3 ++
>  include/ui/console.h    |  9 ++++
>  ui/cursor.c             | 17 ++++---
>  5 files changed, 160 insertions(+), 46 deletions(-)
>