qemu-devel
[Top][All Lists]
Advanced

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

Re: Coverity CID 1421984


From: BALATON Zoltan
Subject: Re: Coverity CID 1421984
Date: Mon, 23 Mar 2020 14:11:38 +0100 (CET)
User-agent: Alpine 2.22 (BSF 395 2020-01-19)

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

reply via email to

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