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: [email protected] > Signed-off-by: Fam Zheng <[email protected]> > --- > 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
