@samitouri / QOSamiQemu / commits / 4727cc883b

hw/display/qxl: unregister vm_change_state handler and BHs

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>

Haotian Jiang committed Jul 20, 2026 at 10:48 UTC 4727cc883b7e81d2c30b9801af54482d65b84292
2 files changed +16 -1
hw/display/qxl.c
+15 -1
@@ -2203,7 +2203,8 @@ static void qxl_realize_common(PCIQXLDevice *qxl, Error **errp)
2203 error_report_err(err);
2204 }
2205
2206 - qemu_add_vm_change_state_handler(qxl_vm_change_state_handler, qxl);
2206 + qxl->vmstate_handler =
2207 + qemu_add_vm_change_state_handler(qxl_vm_change_state_handler, qxl);
2208
2209 qxl->update_irq = qemu_bh_new_guarded(qxl_update_irq_bh, qxl,
2210 &DEVICE(qxl)->mem_reentrancy_guard);
@@ -2475,6 +2476,18 @@ static const Property qxl_properties[] = {
2476 DEFINE_PROP_UINT32("yres", PCIQXLDevice, yres, 0),
2477 };
2478
2479 +static void qxl_exit(PCIDevice *dev)
2480 +{
2481 + PCIQXLDevice *qxl = PCI_QXL(dev);
2482 +
2483 + /* TODO: complete cleanup, error paths etc */
2484 + g_clear_pointer(&qxl->vmstate_handler, qemu_del_vm_change_state_handler);
2485 + g_clear_pointer(&qxl->update_irq, qemu_bh_delete);
2486 + g_clear_pointer(&qxl->update_area_bh, qemu_bh_delete);
2487 + g_clear_pointer(&qxl->ssd.cursor_bh, qemu_bh_delete);
2488 + g_clear_pointer(&qxl->guest_surfaces.cmds, g_free);
2489 +}
2490 +
2491 static void qxl_pci_class_init(ObjectClass *klass, const void *data)
2492 {
2493 DeviceClass *dc = DEVICE_CLASS(klass);
@@ -2482,6 +2495,7 @@ static void qxl_pci_class_init(ObjectClass *klass, const void *data)
2495
2496 k->vendor_id = REDHAT_PCI_VENDOR_ID;
2497 k->device_id = QXL_DEVICE_ID_STABLE;
2498 + k->exit = qxl_exit;
2499 set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
2500 device_class_set_legacy_reset(dc, qxl_reset_handler);
2501 dc->vmsd = &qxl_vmstate;
hw/display/qxl.h
+1
@@ -83,6 +83,7 @@ struct PCIQXLDevice {
83
84 /* thread signaling */
85 QEMUBH *update_irq;
86 + VMChangeStateEntry *vmstate_handler;
87
88 /* ram pci bar */
89 QXLRam *ram;