[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-stable] [PATCH] nvme: Reset s->nr_queues upon open failure
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-stable] [PATCH] nvme: Reset s->nr_queues upon open failure |
Date: |
Wed, 13 Jun 2018 10:16:37 +0200 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
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