[PATCH 0/3] hw: Fix qemu_init_irq() leaks

[PATCH 0/3] hw: Fix qemu_init_irq() leaks

[Peter Maydell]

The qemu_init_irq() method initializes an IRQ object, but
the caller is responsible for eventually freeing it by calling
qemu_free_irq(). Generally we don't remember to do this.

Implement a harder-to-misuse API, qemu_init_irq_child().  This is to
qemu_init_irq() what object_initialize_child() is to
object_initialize(): it both initializes the object and makes it a
child of the parent QOM object.  If you use this in a device's
realize or instance_init method then the IRQ will be automatically
freed when the device is destroyed.

Patch 1 is the new function; patches 2 and 3 are bugfixes for
leaks that show up with ASAN in device-introspect-test (which
does an instance_init -> deinit on every device).

The other callers of qemu_init_irq() could also be changed over
to use this new function, but they don't cause in-practice
leaks because they call the function in realize, and they
are devices which are never unrealized.

thanks
-- PMM

Peter Maydell (3):
  hw/irq: New qemu_init_irq_child() function
  hw/char/serial-pci-multi: Use qemu_init_irq_child() to avoid leak
  hw/ide/ich.c: Use qemu_init_irq_child() to avoid memory leak

 include/hw/irq.h           | 23 ++++++++++++++++++++++-
 hw/char/serial-pci-multi.c |  4 +++-
 hw/core/irq.c              |  8 ++++++++
 hw/ide/ich.c               |  3 ++-
 4 files changed, 35 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH 1/3] hw/irq: New qemu_init_irq_child() function

[Peter Maydell]

The qemu_init_irq() function initializes a TYPE_IRQ QOM object.  The
caller is therefore responsible for eventually calling
qemu_free_irq() to unref (and thus free) it.

In many places where we want to initialize an IRQ we are in
the init/realize of some other QOM object; if we have a variant
of this function that calls object_initialize_child() then the
IRQ will be automatically cleaned up when its parent object is
destroyed, and we don't need to remember to manually free it.

Implement qemu_init_irq_child(), which is to qemu_init_irq()
what object_initialize_child() is to object_initialize().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/irq.h | 23 ++++++++++++++++++++++-
 hw/core/irq.c    |  8 ++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/include/hw/irq.h b/include/hw/irq.h
index b3012237acd..291fdd67df4 100644
--- a/include/hw/irq.h
+++ b/include/hw/irq.h
@@ -36,11 +36,32 @@ static inline void qemu_irq_pulse(qemu_irq irq)
 
 /*
  * Init a single IRQ. The irq is assigned with a handler, an opaque data
- * and the interrupt number.
+ * and the interrupt number. The caller must free this with qemu_free_irq().
+ * If you are using this inside a device's init or realize method, then
+ * qemu_init_irq_child() is probably a better choice to avoid the need
+ * to manually clean up the IRQ.
  */
 void qemu_init_irq(IRQState *irq, qemu_irq_handler handler, void *opaque,
                    int n);
 
+/**
+ * qemu_init_irq_child: Initialize IRQ and make it a QOM child
+ * @parent: QOM object which owns this IRQ
+ * @propname: child property name
+ * @irq: pointer to IRQState to initialize
+ * @handler: handler function for incoming interrupts
+ * @opaque: opaque data to pass to @handler
+ * @n: interrupt number to pass to @handler
+ *
+ * Init a single IRQ and make the IRQ object a child of @parent with
+ * the child-property name @propname. The IRQ object will thus be
+ * automatically freed when @parent is destroyed.
+ */
+void qemu_init_irq_child(Object *parent, const char *propname,
+                         IRQState *irq, qemu_irq_handler handler,
+                         void *opaque, int n);
+
+
 /**
  * qemu_init_irqs: Initialize an array of IRQs.
  *
diff --git a/hw/core/irq.c b/hw/core/irq.c
index 6dd8d47bd6e..0c768f7704e 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -49,6 +49,14 @@ void qemu_init_irq(IRQState *irq, qemu_irq_handler handler, void *opaque,
     init_irq_fields(irq, handler, opaque, n);
 }
 
+void qemu_init_irq_child(Object *parent, const char *propname,
+                         IRQState *irq, qemu_irq_handler handler,
+                         void *opaque, int n)
+{
+    object_initialize_child(parent, propname, irq, TYPE_IRQ);
+    init_irq_fields(irq, handler, opaque, n);
+}
+
 void qemu_init_irqs(IRQState irq[], size_t count,
                     qemu_irq_handler handler, void *opaque)
 {
-- 
2.43.0

[PATCH 2/3] hw/char/serial-pci-multi: Use qemu_init_irq_child() to avoid leak

[Peter Maydell]

The serial-pci-multi device initializes an IRQ with qemu_init_irq()
in its instance_init function; however it never calls qemu_free_irq(),
so the init/deinit cycle has a memory leak, which ASAN catches
in the device-introspect-test:

Direct leak of 576 byte(s) in 6 object(s) allocated from:
    #0 0x626306ddade3 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qem
u-system-arm+0x21f1de3) (BuildId: 52ece17287eba2d68e5be980e1856cd1f6be932f)
    #1 0x7756ade79b09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1
eb6131419edb83b2178b682829a6913cf682d75)
    #2 0x7756ade5b45a in g_hash_table_new_full (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4445a
) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #3 0x62630965da37 in object_initialize_with_type /mnt/nvmedisk/linaro/qemu-from-laptop/qem
u/build/arm-asan/../../qom/object.c:568:23
    #4 0x62630965d440 in object_initialize /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/ar
m-asan/../../qom/object.c:578:5
    #5 0x626309653eeb in qemu_init_irq /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-as
an/../../hw/core/irq.c:48:5
    #6 0x6263072370bb in multi_serial_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/char/serial-pci-multi.c:183:9

Use the new qemu_init_irq_child() function instead, so that the
IRQ object is automatically unreffed when the serial-pci
device is deinited.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/char/serial-pci-multi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/char/serial-pci-multi.c b/hw/char/serial-pci-multi.c
index 13df272691a..9410428ba90 100644
--- a/hw/char/serial-pci-multi.c
+++ b/hw/char/serial-pci-multi.c
@@ -180,7 +180,9 @@ static void multi_serial_init(Object *o)
     size_t i, nports = multi_serial_get_port_count(PCI_DEVICE_GET_CLASS(dev));
 
     for (i = 0; i < nports; i++) {
-        qemu_init_irq(&pms->irqs[i], multi_serial_irq_mux, pms, i);
+        g_autofree char *irqpropname = g_strdup_printf("irq[%zu]", i);
+        qemu_init_irq_child(o, irqpropname, &pms->irqs[i],
+                            multi_serial_irq_mux, pms, i);
         object_initialize_child(o, "serial[*]", &pms->state[i], TYPE_SERIAL);
     }
 }
-- 
2.43.0

[PATCH 3/3] hw/ide/ich.c: Use qemu_init_irq_child() to avoid memory leak

[Peter Maydell]

The ICH9 PCI device uses qemu_init_irq() in its instance_init method,
but fails to clean it up in its uninit. This results in a leak,
detected by ASAN when running the device-introspect-test:

Direct leak of 96 byte(s) in 1 object(s) allocated from:
    #0 0x58f3b53ecde3 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qem
u-system-arm+0x21f1de3) (BuildId: 8dcd38b1d76bd7bd44f905c38200f4cceafd7ca4)
    #1 0x72e446dd5b09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1
eb6131419edb83b2178b682829a6913cf682d75)
    #2 0x72e446db745a in g_hash_table_new_full (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4445a
) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
    #3 0x58f3b7c6fc67 in object_initialize_with_type /mnt/nvmedisk/linaro/qemu-from-laptop/qem
u/build/arm-asan/../../qom/object.c:568:23
    #4 0x58f3b7c6f670 in object_initialize /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/ar
m-asan/../../qom/object.c:578:5
    #5 0x58f3b7c6611b in qemu_init_irq /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/core/irq.c:48:5
    #6 0x58f3b5c6e931 in pci_ich9_ahci_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/ide/ich.c:117:5

We could call qemu_free_irq() in pci_ich9_uninit(), but
since we have a method of initializing the IRQ that doesn't
need manual freeing, use that instead: qemu_init_irq_child().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/ide/ich.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index 4cade0d1219..b00987f08d4 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -114,7 +114,8 @@ static void pci_ich9_ahci_init(Object *obj)
 {
     AHCIPCIState *d = ICH9_AHCI(obj);
 
-    qemu_init_irq(&d->irq, pci_ich9_ahci_update_irq, d, 0);
+    qemu_init_irq_child(obj, "update-irq", &d->irq,
+                        pci_ich9_ahci_update_irq, d, 0);
     ahci_init(&d->ahci, DEVICE(obj));
     d->ahci.irq = &d->irq;
 }
-- 
2.43.0

Re: [PATCH 1/3] hw/irq: New qemu_init_irq_child() function

[Philippe Mathieu-Daudé]

On 21/8/25 17:40, Peter Maydell wrote:
> The qemu_init_irq() function initializes a TYPE_IRQ QOM object.  The
> caller is therefore responsible for eventually calling
> qemu_free_irq() to unref (and thus free) it.
> 
> In many places where we want to initialize an IRQ we are in
> the init/realize of some other QOM object; if we have a variant
> of this function that calls object_initialize_child() then the
> IRQ will be automatically cleaned up when its parent object is
> destroyed, and we don't need to remember to manually free it.
> 
> Implement qemu_init_irq_child(), which is to qemu_init_irq()
> what object_initialize_child() is to object_initialize().
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   include/hw/irq.h | 23 ++++++++++++++++++++++-
>   hw/core/irq.c    |  8 ++++++++
>   2 files changed, 30 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 2/3] hw/char/serial-pci-multi: Use qemu_init_irq_child() to avoid leak

[Philippe Mathieu-Daudé]

On 21/8/25 17:40, Peter Maydell wrote:
> The serial-pci-multi device initializes an IRQ with qemu_init_irq()
> in its instance_init function; however it never calls qemu_free_irq(),
> so the init/deinit cycle has a memory leak, which ASAN catches
> in the device-introspect-test:
> 
> Direct leak of 576 byte(s) in 6 object(s) allocated from:
>      #0 0x626306ddade3 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qem
> u-system-arm+0x21f1de3) (BuildId: 52ece17287eba2d68e5be980e1856cd1f6be932f)
>      #1 0x7756ade79b09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1
> eb6131419edb83b2178b682829a6913cf682d75)
>      #2 0x7756ade5b45a in g_hash_table_new_full (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4445a
> ) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>      #3 0x62630965da37 in object_initialize_with_type /mnt/nvmedisk/linaro/qemu-from-laptop/qem
> u/build/arm-asan/../../qom/object.c:568:23
>      #4 0x62630965d440 in object_initialize /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/ar
> m-asan/../../qom/object.c:578:5
>      #5 0x626309653eeb in qemu_init_irq /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-as
> an/../../hw/core/irq.c:48:5
>      #6 0x6263072370bb in multi_serial_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/char/serial-pci-multi.c:183:9
> 
> Use the new qemu_init_irq_child() function instead, so that the
> IRQ object is automatically unreffed when the serial-pci
> device is deinited.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/char/serial-pci-multi.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/char/serial-pci-multi.c b/hw/char/serial-pci-multi.c
> index 13df272691a..9410428ba90 100644
> --- a/hw/char/serial-pci-multi.c
> +++ b/hw/char/serial-pci-multi.c
> @@ -180,7 +180,9 @@ static void multi_serial_init(Object *o)
>       size_t i, nports = multi_serial_get_port_count(PCI_DEVICE_GET_CLASS(dev));
>   
>       for (i = 0; i < nports; i++) {
> -        qemu_init_irq(&pms->irqs[i], multi_serial_irq_mux, pms, i);
> +        g_autofree char *irqpropname = g_strdup_printf("irq[%zu]", i);
> +        qemu_init_irq_child(o, irqpropname, &pms->irqs[i],
> +                            multi_serial_irq_mux, pms, i);

We could also pass "irq[*]".

>           object_initialize_child(o, "serial[*]", &pms->state[i], TYPE_SERIAL);
>       }
>   }

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 3/3] hw/ide/ich.c: Use qemu_init_irq_child() to avoid memory leak

[Philippe Mathieu-Daudé]

On 21/8/25 17:40, Peter Maydell wrote:
> The ICH9 PCI device uses qemu_init_irq() in its instance_init method,
> but fails to clean it up in its uninit. This results in a leak,
> detected by ASAN when running the device-introspect-test:
> 
> Direct leak of 96 byte(s) in 1 object(s) allocated from:
>      #0 0x58f3b53ecde3 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qem
> u-system-arm+0x21f1de3) (BuildId: 8dcd38b1d76bd7bd44f905c38200f4cceafd7ca4)
>      #1 0x72e446dd5b09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1
> eb6131419edb83b2178b682829a6913cf682d75)
>      #2 0x72e446db745a in g_hash_table_new_full (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4445a
> ) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
>      #3 0x58f3b7c6fc67 in object_initialize_with_type /mnt/nvmedisk/linaro/qemu-from-laptop/qem
> u/build/arm-asan/../../qom/object.c:568:23
>      #4 0x58f3b7c6f670 in object_initialize /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/ar
> m-asan/../../qom/object.c:578:5
>      #5 0x58f3b7c6611b in qemu_init_irq /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/core/irq.c:48:5
>      #6 0x58f3b5c6e931 in pci_ich9_ahci_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/ide/ich.c:117:5
> 
> We could call qemu_free_irq() in pci_ich9_uninit(), but
> since we have a method of initializing the IRQ that doesn't
> need manual freeing, use that instead: qemu_init_irq_child().
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/ide/ich.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 0/3] hw: Fix qemu_init_irq() leaks

[Philippe Mathieu-Daudé]

On 21/8/25 17:40, Peter Maydell wrote:
> The qemu_init_irq() method initializes an IRQ object, but
> the caller is responsible for eventually freeing it by calling
> qemu_free_irq(). Generally we don't remember to do this.
> 
> Implement a harder-to-misuse API, qemu_init_irq_child().  This is to
> qemu_init_irq() what object_initialize_child() is to
> object_initialize(): it both initializes the object and makes it a
> child of the parent QOM object.  If you use this in a device's
> realize or instance_init method then the IRQ will be automatically
> freed when the device is destroyed.
> 
> Patch 1 is the new function; patches 2 and 3 are bugfixes for
> leaks that show up with ASAN in device-introspect-test (which
> does an instance_init -> deinit on every device).
> 
> The other callers of qemu_init_irq() could also be changed over
> to use this new function, but they don't cause in-practice
> leaks because they call the function in realize, and they
> are devices which are never unrealized.

There are only 4 uses left. I don't see any good reason to keep
qemu_init_irq() over qemu_init_irq_child(). Maybe better fully
remove the former by the latter?

> Peter Maydell (3):
>    hw/irq: New qemu_init_irq_child() function
>    hw/char/serial-pci-multi: Use qemu_init_irq_child() to avoid leak
>    hw/ide/ich.c: Use qemu_init_irq_child() to avoid memory leak

Series queued, thanks!

Re: [PATCH 2/3] hw/char/serial-pci-multi: Use qemu_init_irq_child() to avoid leak

[Peter Maydell]

On Tue, 2 Sept 2025 at 11:23, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 21/8/25 17:40, Peter Maydell wrote:
> > The serial-pci-multi device initializes an IRQ with qemu_init_irq()
> > in its instance_init function; however it never calls qemu_free_irq(),
> > so the init/deinit cycle has a memory leak, which ASAN catches
> > in the device-introspect-test:
> >
> > Direct leak of 576 byte(s) in 6 object(s) allocated from:
> >      #0 0x626306ddade3 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qem
> > u-system-arm+0x21f1de3) (BuildId: 52ece17287eba2d68e5be980e1856cd1f6be932f)
> >      #1 0x7756ade79b09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1
> > eb6131419edb83b2178b682829a6913cf682d75)
> >      #2 0x7756ade5b45a in g_hash_table_new_full (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4445a
> > ) (BuildId: 1eb6131419edb83b2178b682829a6913cf682d75)
> >      #3 0x62630965da37 in object_initialize_with_type /mnt/nvmedisk/linaro/qemu-from-laptop/qem
> > u/build/arm-asan/../../qom/object.c:568:23
> >      #4 0x62630965d440 in object_initialize /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/ar
> > m-asan/../../qom/object.c:578:5
> >      #5 0x626309653eeb in qemu_init_irq /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-as
> > an/../../hw/core/irq.c:48:5
> >      #6 0x6263072370bb in multi_serial_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/char/serial-pci-multi.c:183:9
> >
> > Use the new qemu_init_irq_child() function instead, so that the
> > IRQ object is automatically unreffed when the serial-pci
> > device is deinited.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   hw/char/serial-pci-multi.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/char/serial-pci-multi.c b/hw/char/serial-pci-multi.c
> > index 13df272691a..9410428ba90 100644
> > --- a/hw/char/serial-pci-multi.c
> > +++ b/hw/char/serial-pci-multi.c
> > @@ -180,7 +180,9 @@ static void multi_serial_init(Object *o)
> >       size_t i, nports = multi_serial_get_port_count(PCI_DEVICE_GET_CLASS(dev));
> >
> >       for (i = 0; i < nports; i++) {
> > -        qemu_init_irq(&pms->irqs[i], multi_serial_irq_mux, pms, i);
> > +        g_autofree char *irqpropname = g_strdup_printf("irq[%zu]", i);
> > +        qemu_init_irq_child(o, irqpropname, &pms->irqs[i],
> > +                            multi_serial_irq_mux, pms, i);
>
> We could also pass "irq[*]".

Oh yes, this was something that confused me: I'd forgotten
that we special case [*] in object_property_try_add().
Using "[*]" seems better here, then.

Incidentally, the code in object_property_try_add() is
quadratic in the number of properties with that name:
if you have irq[*] for N irqs then it will recursively
call object_property_try_add() once for irq 0, twice for
irq 1, 3 times for irq 2, and so up to N times for irq N-1,
for a total of N(N+1)/2 calls...

-- PMM