Re: [PATCH] vdpa: Fix file descriptor leak on get features error

[Peter Maydell]

On Mon, 1 Aug 2022 at 19:31, Laurent Vivier <lvivier@redhat.com> wrote:
>
> On 01/08/2022 16:47, Eugenio Pérez wrote:
> > File descriptor vdpa_device_fd is not free in the case of returning
> > error from vhost_vdpa_get_features. Fixing it by making all errors go to
> > the same error path.
> >
> > Resolves: Coverity CID 1490785
> > Fixes: 8170ab3f43 ("vdpa: Extract get features part from 
> > vhost_vdpa_get_max_queue_pairs")
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >   net/vhost-vdpa.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > index 6abad276a6..303447a68e 100644
> > --- a/net/vhost-vdpa.c
> > +++ b/net/vhost-vdpa.c
> > @@ -566,7 +566,7 @@ int net_init_vhost_vdpa(const Netdev *netdev, const 
> > char *name,
> >       g_autofree NetClientState **ncs = NULL;
> >       g_autoptr(VhostIOVATree) iova_tree = NULL;
> >       NetClientState *nc;
> > -    int queue_pairs, r, i, has_cvq = 0;
> > +    int queue_pairs, r, i = 0, has_cvq = 0;
> >
> >       assert(netdev->type == NET_CLIENT_DRIVER_VHOST_VDPA);
> >       opts = &netdev->u.vhost_vdpa;
> > @@ -582,7 +582,7 @@ int net_init_vhost_vdpa(const Netdev *netdev, const 
> > char *name,
> >
> >       r = vhost_vdpa_get_features(vdpa_device_fd, &features, errp);
> >       if (unlikely(r < 0)) {
> > -        return r;
> > +        goto err;
>
> Why don't you use "goto err_svq"?

This patch is kind of half of the idea discussed in the thread where
this problem was reported, which is that by setting i = 0 you can
then consistently have all the error handling be 'goto err' and that
frees everything that needs to be freed regardless of whether it's
called after or before the initialization of the ncs[] entries. But it
doesn't do the other half of the job, which is making all the other
error handling code in the function also use 'goto err', so it looks
a bit odd as it stands.

-- PMM