hw: Fix qemu_init_irq() leaks

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

Posted by Peter Maydell 2 months, 1 week ago

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

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

Posted by Philippe Mathieu-Daudé 1 month, 4 weeks ago

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 2/3] hw/char/serial-pci-multi: Use qemu_init_irq_child() to avoid leak

Posted by Peter Maydell 1 month, 4 weeks ago

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