Re: [QEMU HCI-8.0 PATCH v2 1/5] vhost-blk: do not double close vhostfd

[Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>]

> vhost_dev_init() closes the backend fd on failure, so the cleanup
> path of vhost_blk_device_realize() closed vhostfd for the second
> time. The cleanup path is also reachable with vhostfd never opened,
> in which case we called close(-1).
> 
> Fixes: 0ff3cb5ff2 ("block: add vhost-blk backend")
> https://virtuozzo.atlassian.net/browse/VSTOR-143437
> Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
>
> diff --git a/hw/block/vhost-blk.c b/hw/block/vhost-blk.c
> index c52851fcf8b..9bd49fef2da 100644
> --- a/hw/block/vhost-blk.c
> +++ b/hw/block/vhost-blk.c
> @@ -318,6 +318,8 @@ static void vhost_blk_device_realize(DeviceState *dev, Error **errp)
>      if (ret < 0) {
>          error_setg(errp, "vhost-blk: vhost initialization failed: %s",
>                  strerror(-ret));
> +        /* vhost_dev_init() closes vhostfd on failure */
> +        s->vhostfd = -1;
>          goto cleanup;
>      }
>  
> @@ -328,7 +330,10 @@ cleanup:
>              qemu_del_vm_change_state_handler(s->mighand);
>      }
>      g_free(s->dev.vqs);

Not related, but while we're here:
  vhost_blk_device_realize()
    s->dev.vqs = g_new0()
    vhost_dev_init() fails ->
      vhost_dev_cleanup(s->dev)
        memset(s->dev, 0)
    g_free(s->dev.vqs)

Result: g_free(NULL) does nothing, array allocated with g_new0() leaks.
Worth another patch.  Or if it "Fixes:" the same commit - might fold
into the same patch.

Andrey

-- 
Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>