Re: [PATCH] block/nvme: Remove memory leak

[Philippe Mathieu-Daudé]

On 4/15/20 9:46 AM, Stefano Garzarella wrote:
> On Tue, Apr 14, 2020 at 05:17:26PM +0200, Philippe Mathieu-Daudé wrote:
> > Fixes: bdd6a90a9 ("Add VFIO based NVMe driver")
> > Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> > ---
> >   block/nvme.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/block/nvme.c b/block/nvme.c
> > index 7b7c0cc5d6..9f3c7ab819 100644
> > --- a/block/nvme.c
> > +++ b/block/nvme.c
> > @@ -163,6 +163,7 @@ static void nvme_init_queue(BlockDriverState *bs, NVMeQueue 
> > *q,
> >       }
> >       r = qemu_vfio_dma_map(s->vfio, q->queue, bytes, false, &q->iova);
> >       if (r) {
> > +        qemu_vfree(q->queue);
> >           error_setg(errp, "Cannot map queue");
> >       }
> >   }
> > --
> > 2.21.1
>
> Are we adding a double free with this change?
>
> IIUC when nvme_init_queue() fails in the nvme_create_queue_pair() we
> already call the nvme_free_queue_pair() that frees sq.queue and
> cq.queue.

Argh you are right, I missed that...

> I think your patch make the code cleaner, freeing the buffer near the
> allocation during an error, maybe we can also set q->queue to NULL
> after the qemu_vfree().
>
> Thanks,
> Stefano