[Top][All Lists]

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

Re: [PATCH] ide/sii3112: Avoid leaking irqs array

From: BALATON Zoltan
Subject: Re: [PATCH] ide/sii3112: Avoid leaking irqs array
Date: Mon, 23 Mar 2020 16:42:01 +0100 (CET)
User-agent: Alpine 2.22 (BSF 395 2020-01-19)

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 <address@hidden>
  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.


reply via email to

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