qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Memory leak in via_isa_realize()


From: BALATON Zoltan
Subject: Re: Memory leak in via_isa_realize()
Date: Mon, 21 Mar 2022 19:34:31 +0100 (CET)

On Mon, 21 Mar 2022, Peter Maydell wrote:
On Mon, 21 Mar 2022 at 10:31, Thomas Huth <thuth@redhat.com> wrote:
FYI, I'm seeing a memory leak in via_isa_realize() when building
QEMU with sanitizers enabled or when running QEMU through valgrind:
Same problem happens with qemu-system-ppc64 and the pegasos2 machine.

No clue how to properly fix this... is it safe to free the pointer
at the end of the function?

This is because the code is still using the old function
qemu_allocate_irqs(), which is almost always going to involve
it leaking memory. The fix is usually to rewrite the code to not use
that function at all, i.e. to manage its irq/gpio lines differently.
Probably the i8259 code should have a named GPIO output line
rather than wanting to be passed a qemu_irq in an init function,
and the via code should have an input GPIO line which it connects
up to the i8259. It looks from a quick glance like the i8259 and
its callers have perhaps not been completely QOMified.

In this specific case, though, it seems like the only thing that
the via_isa_request_i8259_irq() function does is pass the interrupt
signal through to its own s->cpu_intr, so I think a relatively
self-contained way to deal with the leak is to pass s->cpu_intr
into i8259_init() and drop the isa_irq allocated irq and its
associated helper function entirely. (There might be some subtlety
I'm missing that means that wouldn't work, of course.)

So the recent piix4 cleanups did not change this and it seems to have the same qemu_allocate_irqs and forwarding as well as i82378. Does similar leak exist for those too? And probably all other callers of i8259_init(), so this would probably be better fixed in that device model instead but I don't know anything about that so I don't want to touch it. Within vt82c686 we could probably store the allocated irq in the device state and free it in a finalize function or simliar if that's possible but that would only fix one case of simliar problems and it's unnecessary complication if this could be properly fixed elsewhere. So I give up on this for now and wait if somebody could do something about i8259_init instead. That seems like a legacy init function that should be resolved,

Regards,
BALATON Zoltan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]