| Message ID | 20180613074552.15656-1-famz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 13.06.2018 um 09:45 hat Fam Zheng geschrieben: > It is wrong to leave this field as 1, as nvme_close() called in the > error handling code in nvme_file_open() will use it and try to free > s->queues again. > > Clear the fields to avoid double-free. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > block/nvme.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/block/nvme.c b/block/nvme.c > index 6f71122bf5..7bdeb0ffce 100644 > --- a/block/nvme.c > +++ b/block/nvme.c > @@ -666,6 +666,8 @@ fail_queue: > nvme_free_queue_pair(bs, s->queues[0]); > fail: > g_free(s->queues); > + s->queues = NULL; > + s->nr_queues = 0; > if (s->regs) { > qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)s->regs, 0, NVME_BAR_SIZE); > } Hm... Basically all the cleanup is duplicated. It's not only nvme_free_queue_pair(), but also qemu_vfio_pci_unmap_bar() and qemu_vfio_close(). Are we sure it's intended to call them twice? Maybe nvme_init() shouldn't clean up any of this and rely on the later nvme_close() call to do that? I also notice that the error handling code in nvme_init() has a g_free(s->queues) and event_notifier_cleanup(&s->irq_notifier), which nvme_close() doesn't. Are these leaks in nvme_close()? Kevin
diff --git a/block/nvme.c b/block/nvme.c index 6f71122bf5..7bdeb0ffce 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -666,6 +666,8 @@ fail_queue: nvme_free_queue_pair(bs, s->queues[0]); fail: g_free(s->queues); + s->queues = NULL; + s->nr_queues = 0; if (s->regs) { qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)s->regs, 0, NVME_BAR_SIZE); }
It is wrong to leave this field as 1, as nvme_close() called in the error handling code in nvme_file_open() will use it and try to free s->queues again. Clear the fields to avoid double-free. Cc: qemu-stable@nongnu.org Signed-off-by: Fam Zheng <famz@redhat.com> --- block/nvme.c | 2 ++ 1 file changed, 2 insertions(+)