1. Patchew
  2. QEMU
  3. View series

[PATCH] hw/display/qxl: unregister vm_change_state handler and BHs on device exit (CVE-2026-63322)

jianghaotian.sunday@gmail.com posted 1 patch 5 days, 8 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260720024855.3757499-1-jianghaotian.sunday@gmail.com
hw/display/qxl.c | 15 ++++++++++++++-
hw/display/qxl.h |  1 +
2 files changed, 15 insertions(+), 1 deletion(-)
[PATCH] hw/display/qxl: unregister vm_change_state handler and BHs on device exit (CVE-2026-63322)
Posted by jianghaotian.sunday@gmail.com 5 days, 8 hours ago 
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
Reported-by: Haotian Jiang of Tencent Security (Yunding Lab) <jianghaotian.sunday@gmail.com>
Signed-off-by: Haotian Jiang <jianghaotian.sunday@gmail.com>
Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3607
---
 hw/display/qxl.c | 15 ++++++++++++++-
 hw/display/qxl.h |  1 +
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 74258afa58..c9323672a6 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,17 @@ static const Property qxl_properties[] = {
         DEFINE_PROP_UINT32("yres", PCIQXLDevice, yres, 0),
 };
 
+static void qxl_exit(PCIDevice *dev)
+{
+    PCIQXLDevice *qxl = PCI_QXL(dev);
+
+    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);
@@ -2482,6 +2494,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;
-- 
Changes v1 -> v2:
  - Use g_clear_pointer() for cleanup per Marc-Andre Lureau's suggestion
  - Simplifies the code by removing redundant NULL checks and assignments
2.34.1
Re: [PATCH] hw/display/qxl: unregister vm_change_state handler and BHs on device exit (CVE-2026-63322)
Posted by Thomas Huth 5 days, 5 hours ago 
Thanks for the patch! ... I've got a question below...

On 20/07/2026 04.48, jianghaotian.sunday@gmail.com wrote:
> 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
> Reported-by: Haotian Jiang of Tencent Security (Yunding Lab) <jianghaotian.sunday@gmail.com>
> Signed-off-by: Haotian Jiang <jianghaotian.sunday@gmail.com>
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3607
> ---
>   hw/display/qxl.c | 15 ++++++++++++++-
>   hw/display/qxl.h |  1 +
>   2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index 74258afa58..c9323672a6 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,17 @@ static const Property qxl_properties[] = {
>           DEFINE_PROP_UINT32("yres", PCIQXLDevice, yres, 0),
>   };
>   
> +static void qxl_exit(PCIDevice *dev)
> +{
> +    PCIQXLDevice *qxl = PCI_QXL(dev);
> +
> +    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);
> @@ -2482,6 +2494,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;
The resources are allocated during the realize() function of the qxl device, 
so not sure, but using the PCI->exit() function might be the wrong spot? 
Could you put the clean-up into a unrealize() function instead, i.e. add 
this via "k->unrealize = ..." in qxl_primary_class_init() and 
qxl_secondary_class_init() ?

Also, there are some more resources allocated in the realize() function, e.g. :

     qemu_mutex_init(&qxl->track_lock);
     qemu_mutex_init(&qxl->async_lock);
...
     qxl->guest_surfaces.cmds = g_new0(QXLPHYSICAL, qxl->ssd.num_surfaces);

Should these get deleted, too, to avoid leaking memory?

  Thomas
Re: [PATCH] hw/display/qxl: unregister vm_change_state handler and BHs on device exit (CVE-2026-63322)
Posted by Marc-André Lureau 3 days, 19 hours ago 
Hi

On Mon, Jul 20, 2026 at 10:16 AM Thomas Huth <thuth@redhat.com> wrote:
>
>
> Thanks for the patch! ... I've got a question below...
>
> On 20/07/2026 04.48, jianghaotian.sunday@gmail.com wrote:
> > 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
> > Reported-by: Haotian Jiang of Tencent Security (Yunding Lab) <jianghaotian.sunday@gmail.com>
> > Signed-off-by: Haotian Jiang <jianghaotian.sunday@gmail.com>
> > Cc: qemu-stable@nongnu.org
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3607
> > ---
> >   hw/display/qxl.c | 15 ++++++++++++++-
> >   hw/display/qxl.h |  1 +
> >   2 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> > index 74258afa58..c9323672a6 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,17 @@ static const Property qxl_properties[] = {
> >           DEFINE_PROP_UINT32("yres", PCIQXLDevice, yres, 0),
> >   };
> >
> > +static void qxl_exit(PCIDevice *dev)
> > +{
> > +    PCIQXLDevice *qxl = PCI_QXL(dev);
> > +
> > +    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);
> > @@ -2482,6 +2494,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;
> The resources are allocated during the realize() function of the qxl device,
> so not sure, but using the PCI->exit() function might be the wrong spot?
> Could you put the clean-up into a unrealize() function instead, i.e. add
> this via "k->unrealize = ..." in qxl_primary_class_init() and
> qxl_secondary_class_init() ?
>
> Also, there are some more resources allocated in the realize() function, e.g. :
>
>      qemu_mutex_init(&qxl->track_lock);
>      qemu_mutex_init(&qxl->async_lock);
> ...
>      qxl->guest_surfaces.cmds = g_new0(QXLPHYSICAL, qxl->ssd.num_surfaces);
>
> Should these get deleted, too, to avoid leaking memory?
>

I am afraid trying to fix the leaks is going into a rabbit hole (at
least I did). Probably it is best to queue this fix and handle the
leaks post 11.1 imho

@jianghaotian.sunday@gmail.com Are you working on a new version?
Re: [PATCH] hw/display/qxl: unregister vm_change_state handler and BHs on device exit (CVE-2026-63322)
Posted by Marc-André Lureau 5 days, 5 hours ago 
On Mon, Jul 20, 2026 at 10:15 AM Thomas Huth <thuth@redhat.com> wrote:
>
>
> Thanks for the patch! ... I've got a question below...
>
> On 20/07/2026 04.48, jianghaotian.sunday@gmail.com wrote:
> > 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
> > Reported-by: Haotian Jiang of Tencent Security (Yunding Lab) <jianghaotian.sunday@gmail.com>
> > Signed-off-by: Haotian Jiang <jianghaotian.sunday@gmail.com>
> > Cc: qemu-stable@nongnu.org
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3607

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

> > ---
> >   hw/display/qxl.c | 15 ++++++++++++++-
> >   hw/display/qxl.h |  1 +
> >   2 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> > index 74258afa58..c9323672a6 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,17 @@ static const Property qxl_properties[] = {
> >           DEFINE_PROP_UINT32("yres", PCIQXLDevice, yres, 0),
> >   };
> >
> > +static void qxl_exit(PCIDevice *dev)
> > +{
> > +    PCIQXLDevice *qxl = PCI_QXL(dev);
> > +
> > +    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);
> > @@ -2482,6 +2494,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;
> The resources are allocated during the realize() function of the qxl device,
> so not sure, but using the PCI->exit() function might be the wrong spot?
> Could you put the clean-up into a unrealize() function instead, i.e. add
> this via "k->unrealize = ..." in qxl_primary_class_init() and
> qxl_secondary_class_init() ?
>
> Also, there are some more resources allocated in the realize() function, e.g. :
>
>      qemu_mutex_init(&qxl->track_lock);
>      qemu_mutex_init(&qxl->async_lock);
> ...
>      qxl->guest_surfaces.cmds = g_new0(QXLPHYSICAL, qxl->ssd.num_surfaces);
>
> Should these get deleted, too, to avoid leaking memory?
>
>   Thomas
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.