Re: [PATCH 07/18] vhost-vdpa: tweak the error label in vhost_vdpa_add()

[Stefano Garzarella]

On Tue, Jul 06, 2021 at 04:10:22PM +0800, Jason Wang wrote:
> 在 2021/7/6 下午4:03, Jason Wang 写道:
> > 在 2021/6/23 下午11:03, Stefano Garzarella 写道:
> > > On Mon, Jun 21, 2021 at 12:16:39PM +0800, Jason Wang wrote:
> > > > Introduce new error label to avoid the unnecessary checking of net
> > > > pointer.
> > > >
> > > > Fixes: 1e0a84ea49b68 ("vhost-vdpa: introduce vhost-vdpa net client")
> > > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > > ---
> > > > net/vhost-vdpa.c | 13 ++++++-------
> > > > 1 file changed, 6 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > > > index 21f09c546f..0da7bc347a 100644
> > > > --- a/net/vhost-vdpa.c
> > > > +++ b/net/vhost-vdpa.c
> > > > @@ -100,19 +100,18 @@ static int vhost_vdpa_add(NetClientState 
> > > > *ncs, void *be)
> > > >     net = vhost_net_init(&options);
> > > >     if (!net) {
> > > >         error_report("failed to init vhost_net for queue");
> > > > -        goto err;
> > > > +        goto err_init;
> > > >     }
> > > >     s->vhost_net = net;
> > > >     ret = vhost_vdpa_net_check_device_id(net);
> > > >     if (ret) {
> > > > -        goto err;
> > > > +        goto err_check;
> > > >     }
> > > >     return 0;
> > > > -err:
> > > > -    if (net) {
> > > > -        vhost_net_cleanup(net);
> > > > -        g_free(net);
> > > > -    }
> > > > +err_check:
> > > > +    vhost_net_cleanup(net);
> > > > +    g_free(net);
> > >
> > > Should we set s->vhost_net to NULL to avoid use after free?
> > >
> > > Then we should also remove the `assert(s->vhost_net)` in 
> > > net_vhost_vdpa_init() since we can fail.
> >
> >
> > Right, will do this in a separate patch.
>
>
> I just forget the job has been done in the next patch :)

I saw it later too ;-)

> So we are fine here.

Yep for the assert(), but what about setting s->vhost_net to NULL?
Or just move the s->vhost_net assignment just before the `return 0`.

Thanks,
Stefano