* [PATCH] ide/sii3112: Avoid leaking irqs array
@ 2020-03-23 14:32 BALATON Zoltan
2020-03-23 14:50 ` Philippe Mathieu-Daudé
2020-03-23 18:44 ` John Snow
0 siblings, 2 replies; 4+ messages in thread
From: BALATON Zoltan @ 2020-03-23 14:32 UTC (permalink / raw)
To: qemu-devel, qemu-block; +Cc: Peter Maydell, philmd, John Snow, Max Reitz
Coverity CID 1421984 reports a leak in allocated irqs, this patch
attempts to plug that.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/ide/sii3112.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
index 06605d7af2..c886916873 100644
--- a/hw/ide/sii3112.c
+++ b/hw/ide/sii3112.c
@@ -31,6 +31,7 @@ typedef struct SiI3112Regs {
typedef struct SiI3112PCIState {
PCIIDEState i;
MemoryRegion mmio;
+ qemu_irq *irqs;
SiI3112Regs regs[2];
} SiI3112PCIState;
@@ -252,7 +253,6 @@ static void sii3112_pci_realize(PCIDevice *dev, Error **errp)
SiI3112PCIState *d = SII3112_PCI(dev);
PCIIDEState *s = PCI_IDE(dev);
MemoryRegion *mr;
- qemu_irq *irq;
int i;
pci_config_set_interrupt_pin(dev->config, 1);
@@ -280,10 +280,10 @@ static void sii3112_pci_realize(PCIDevice *dev, Error **errp)
memory_region_init_alias(mr, OBJECT(d), "sii3112.bar4", &d->mmio, 0, 16);
pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, mr);
- irq = qemu_allocate_irqs(sii3112_set_irq, d, 2);
+ d->irqs = qemu_allocate_irqs(sii3112_set_irq, d, 2);
for (i = 0; i < 2; i++) {
ide_bus_new(&s->bus[i], sizeof(s->bus[i]), DEVICE(dev), i, 1);
- ide_init2(&s->bus[i], irq[i]);
+ ide_init2(&s->bus[i], d->irqs[i]);
bmdma_init(&s->bus[i], &s->bmdma[i], s);
s->bmdma[i].bus = &s->bus[i];
@@ -291,6 +291,13 @@ static void sii3112_pci_realize(PCIDevice *dev, Error **errp)
}
}
+static void sii3112_unrealize(DeviceState *dev, Error **errp)
+{
+ SiI3112PCIState *d = SII3112_PCI(dev);
+
+ qemu_free_irqs(d->irqs, 2);
+}
+
static void sii3112_pci_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -301,6 +308,7 @@ static void sii3112_pci_class_init(ObjectClass *klass, void *data)
pd->class_id = PCI_CLASS_STORAGE_RAID;
pd->revision = 1;
pd->realize = sii3112_pci_realize;
+ dc->unrealize = sii3112_unrealize;
dc->reset = sii3112_reset;
dc->desc = "SiI3112A SATA controller";
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
--
2.21.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] ide/sii3112: Avoid leaking irqs array
2020-03-23 14:32 [PATCH] ide/sii3112: Avoid leaking irqs array BALATON Zoltan
@ 2020-03-23 14:50 ` Philippe Mathieu-Daudé
2020-03-23 15:42 ` BALATON Zoltan
2020-03-23 18:44 ` John Snow
1 sibling, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-23 14:50 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-block
Cc: Peter Maydell, John Snow, Max Reitz
On 3/23/20 3:32 PM, BALATON Zoltan wrote:
> Coverity CID 1421984 reports a leak in allocated irqs, this patch
> attempts to plug that.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/ide/sii3112.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
> index 06605d7af2..c886916873 100644
> --- a/hw/ide/sii3112.c
> +++ b/hw/ide/sii3112.c
> @@ -31,6 +31,7 @@ typedef struct SiI3112Regs {
> typedef struct SiI3112PCIState {
> PCIIDEState i;
> MemoryRegion mmio;
> + qemu_irq *irqs;
> SiI3112Regs regs[2];
> } SiI3112PCIState;
>
> @@ -252,7 +253,6 @@ static void sii3112_pci_realize(PCIDevice *dev, Error **errp)
> SiI3112PCIState *d = SII3112_PCI(dev);
> PCIIDEState *s = PCI_IDE(dev);
> MemoryRegion *mr;
> - qemu_irq *irq;
> int i;
>
> pci_config_set_interrupt_pin(dev->config, 1);
> @@ -280,10 +280,10 @@ static void sii3112_pci_realize(PCIDevice *dev, Error **errp)
> memory_region_init_alias(mr, OBJECT(d), "sii3112.bar4", &d->mmio, 0, 16);
> pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, mr);
>
> - irq = qemu_allocate_irqs(sii3112_set_irq, d, 2);
> + d->irqs = qemu_allocate_irqs(sii3112_set_irq, d, 2);
> for (i = 0; i < 2; i++) {
> ide_bus_new(&s->bus[i], sizeof(s->bus[i]), DEVICE(dev), i, 1);
> - ide_init2(&s->bus[i], irq[i]);
> + ide_init2(&s->bus[i], d->irqs[i]);
>
> bmdma_init(&s->bus[i], &s->bmdma[i], s);
> s->bmdma[i].bus = &s->bus[i];
> @@ -291,6 +291,13 @@ static void sii3112_pci_realize(PCIDevice *dev, Error **errp)
> }
> }
>
> +static void sii3112_unrealize(DeviceState *dev, Error **errp)
> +{
> + SiI3112PCIState *d = SII3112_PCI(dev);
> +
> + qemu_free_irqs(d->irqs, 2);
You can't do that without calling unrealize() on all the devices in each
IDEBus. Apparently there is no code available to do that. Maybe easier
to not add any sii3112_unrealize(). Keeping a reference in the state
should be enough to silent Coverity.
> +}
> +
> static void sii3112_pci_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -301,6 +308,7 @@ static void sii3112_pci_class_init(ObjectClass *klass, void *data)
> pd->class_id = PCI_CLASS_STORAGE_RAID;
> pd->revision = 1;
> pd->realize = sii3112_pci_realize;
> + dc->unrealize = sii3112_unrealize;
> dc->reset = sii3112_reset;
> dc->desc = "SiI3112A SATA controller";
> set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] ide/sii3112: Avoid leaking irqs array
2020-03-23 14:50 ` Philippe Mathieu-Daudé
@ 2020-03-23 15:42 ` BALATON Zoltan
0 siblings, 0 replies; 4+ messages in thread
From: BALATON Zoltan @ 2020-03-23 15:42 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, John Snow, qemu-devel, qemu-block, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 2885 bytes --]
On Mon, 23 Mar 2020, Philippe Mathieu-Daudé wrote:
> On 3/23/20 3:32 PM, BALATON Zoltan wrote:
>> Coverity CID 1421984 reports a leak in allocated irqs, this patch
>> attempts to plug that.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> hw/ide/sii3112.c | 14 +++++++++++---
>> 1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
>> index 06605d7af2..c886916873 100644
>> --- a/hw/ide/sii3112.c
>> +++ b/hw/ide/sii3112.c
>> @@ -31,6 +31,7 @@ typedef struct SiI3112Regs {
>> typedef struct SiI3112PCIState {
>> PCIIDEState i;
>> MemoryRegion mmio;
>> + qemu_irq *irqs;
>> SiI3112Regs regs[2];
>> } SiI3112PCIState;
>> @@ -252,7 +253,6 @@ static void sii3112_pci_realize(PCIDevice *dev, Error
>> **errp)
>> SiI3112PCIState *d = SII3112_PCI(dev);
>> PCIIDEState *s = PCI_IDE(dev);
>> MemoryRegion *mr;
>> - qemu_irq *irq;
>> int i;
>> pci_config_set_interrupt_pin(dev->config, 1);
>> @@ -280,10 +280,10 @@ static void sii3112_pci_realize(PCIDevice *dev, Error
>> **errp)
>> memory_region_init_alias(mr, OBJECT(d), "sii3112.bar4", &d->mmio, 0,
>> 16);
>> pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, mr);
>> - irq = qemu_allocate_irqs(sii3112_set_irq, d, 2);
>> + d->irqs = qemu_allocate_irqs(sii3112_set_irq, d, 2);
>> for (i = 0; i < 2; i++) {
>> ide_bus_new(&s->bus[i], sizeof(s->bus[i]), DEVICE(dev), i, 1);
>> - ide_init2(&s->bus[i], irq[i]);
>> + ide_init2(&s->bus[i], d->irqs[i]);
>> bmdma_init(&s->bus[i], &s->bmdma[i], s);
>> s->bmdma[i].bus = &s->bus[i];
>> @@ -291,6 +291,13 @@ static void sii3112_pci_realize(PCIDevice *dev, Error
>> **errp)
>> }
>> }
>> +static void sii3112_unrealize(DeviceState *dev, Error **errp)
>> +{
>> + SiI3112PCIState *d = SII3112_PCI(dev);
>> +
>> + qemu_free_irqs(d->irqs, 2);
>
> You can't do that without calling unrealize() on all the devices in each
> IDEBus.
What? Those devices are not created in this object so whoever adds them
later is supposed to free them before this object is unrelaized. Or is
ownership of those devices silently passed to the the controller when
adding devices? Anyway, Peter's patch is simpler and should also fix the
issue so this does not matter any more (other than maybe showing we might
also leak the devices if their ownership is not clear).
> Apparently there is no code available to do that. Maybe easier to not
> add any sii3112_unrealize(). Keeping a reference in the state should be
> enough to silent Coverity.
The idea was to fix the problem not to hide it from Coverity so if it
can't be fixed it's probably better to be reminded about it than hiding
it.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ide/sii3112: Avoid leaking irqs array
2020-03-23 14:32 [PATCH] ide/sii3112: Avoid leaking irqs array BALATON Zoltan
2020-03-23 14:50 ` Philippe Mathieu-Daudé
@ 2020-03-23 18:44 ` John Snow
1 sibling, 0 replies; 4+ messages in thread
From: John Snow @ 2020-03-23 18:44 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-block; +Cc: Peter Maydell, philmd, Max Reitz
On 3/23/20 10:32 AM, BALATON Zoltan wrote:
> Coverity CID 1421984 reports a leak in allocated irqs, this patch
> attempts to plug that.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Dequeueing in favor of Peter Maydell's patch.
(Let me know if you feel that is a mistake.)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-23 18:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-23 14:32 [PATCH] ide/sii3112: Avoid leaking irqs array BALATON Zoltan
2020-03-23 14:50 ` Philippe Mathieu-Daudé
2020-03-23 15:42 ` BALATON Zoltan
2020-03-23 18:44 ` John Snow
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).