On Mon, 23 Mar 2020, Philippe Mathieu-Daudé wrote:
Cc'ing qemu-ppc since this is restricted to the aCube Sam460ex board.
On 3/23/20 12:46 PM, Max Reitz wrote:
Hi,
I was triaging new Coverity block layer reports today, and one that
seemed like a real bug was CID 1421984:
It complains about a memleak in sii3112_pci_realize() in
hw/ide/sii3112.c, specifically about @irq being leaked (it’s allocated
by qemu_allocate_irqs(), but never put anywhere or freed).
I’m not really well-versed in anything under hw/ide, so I was wondering
whether you agree it’s a bug and whether you know the correct way to fix
it. (I assume it’s just a g_free(irqs), but maybe there’s a more
specific way that’s applicable here.)
What does other devices is hold a reference in the DeviceState
(SiI3112PCIState) to make static analyzers happy.
Other IDE devices such as ahci and cmd646 seem to free it at the end of
the init function after calling ide_init2 with it. However it's not clear
to me how all this is supposed to work. Ahci does for example:
qemu_irq *irqs = qemu_allocate_irqs(ahci_irq_set, s, s->ports);
for (i = 0; i < s->ports; i++) {
ide_init2(&ad->port, irqs[i]);
}
g_free(irqs);
So it allocates an array of s->ports irqs then only frees a single
element? Also aren't these needed after ide_init2 to actually raise the
irq or are these end up copied to the irq field of the BMDMAState sonehow?
Where will that be freed?
Ideally we should free such memory in the DeviceUnrealize handler, but we in
the reality we only care for hotunpluggable devices.
PCI devices usually are. There is a trick however, you can mark overwrite the
DeviceClass::hotpluggable field in sii3112_pci_class_init:
dc->hotpluggable = false;
If the above is correct then simply adding g_free(irq) after the loop at
end of sii3112_pci_realize should be enough but I can't tell if that's
correct. Setting hotpluggable to false does not seem to be a good fix.
Regards,
BALATON Zoltan
