Hi
On Fri, Jul 17, 2026 at 8:54 AM <[email protected]> wrote:
>
> From: Haotian Jiang <[email protected]>
>
> 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
> Reported-by: Haotian Jiang of Tencent Security (Yunding Lab)
> <[email protected]>
> Signed-off-by: Haotian Jiang <[email protected]>
> Cc: [email protected]
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3607
> ---
> hw/display/qxl.c | 29 ++++++++++++++++++++++++++++-
> hw/display/qxl.h | 1 +
> 2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index 74258afa58..23608f988a 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -2203,7 +2203,8 @@ static void qxl_realize_common(PCIQXLDevice *qxl, Error
> **errp)
> error_report_err(err);
> }
>
> - 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);
> @@ -2475,6 +2476,31 @@ static const Property qxl_properties[] = {
> DEFINE_PROP_UINT32("yres", PCIQXLDevice, yres, 0),
> };
>
> +static void qxl_exit(PCIDevice *dev)
> +{
> + PCIQXLDevice *qxl = PCI_QXL(dev);
> +
> + if (qxl->vmstate_handler) {
> + qemu_del_vm_change_state_handler(qxl->vmstate_handler);
> + qxl->vmstate_handler = NULL;
> + }
> + if (qxl->update_irq) {
> + qemu_bh_delete(qxl->update_irq);
> + qxl->update_irq = NULL;
> + }
> + if (qxl->update_area_bh) {
> + qemu_bh_delete(qxl->update_area_bh);
> + qxl->update_area_bh = NULL;
> + }
> + if (qxl->ssd.cursor_bh) {
> + qemu_bh_delete(qxl->ssd.cursor_bh);
> + qxl->ssd.cursor_bh = NULL;
> + }
Please use g_clear_pointer(&bh, qemu_bh_delete).
> +
> + g_free(qxl->guest_surfaces.cmds);
> + qxl->guest_surfaces.cmds = NULL;
And g_clear_pointer(&..cmds, g_free)
Hard to say without careful examination if other fields should be
unref/free here...
> +}
> +
> static void qxl_pci_class_init(ObjectClass *klass, const void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -2482,6 +2508,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 ad8a912878..48d664f777 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.34.1
>
>
