Re: [Qemu-block] [PATCH] nvme: Reset s->nr_queues upon open failure

[Kevin Wolf]

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: address@hidden
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  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