Re: [PULL v3 04/32] vfio: Add migration region initialization and finalize function

[Alex Williamson]

On Thu, 5 Nov 2020 23:55:32 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Sun, 1 Nov 2020 at 21:02, Alex Williamson <alex.williamson@redhat.com> 
> wrote:
> >
> > From: Kirti Wankhede <kwankhede@nvidia.com>
> >
> > Whether the VFIO device supports migration or not is decided based of
> > migration region query. If migration region query is successful and 
> > migration
> > region initialization is successful then migration is supported else
> > migration is blocked.
> >
> > Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> > Reviewed-by: Neo Jia <cjia@nvidia.com>
> > Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>  
> 
> Hi; Coverity points out (CID 1436126) that this code has a
> use-after-free:

Thanks, I already relayed this to Kirti and expect to see a patch.
Thanks,

Alex


> > +int vfio_migration_probe(VFIODevice *vbasedev, Error **errp)
> > +{
> > +    struct vfio_region_info *info = NULL;
> > +    Error *local_err = NULL;
> > +    int ret;
> > +
> > +    ret = vfio_get_dev_region_info(vbasedev, VFIO_REGION_TYPE_MIGRATION,
> > +                                   VFIO_REGION_SUBTYPE_MIGRATION, &info);
> > +    if (ret) {
> > +        goto add_blocker;
> > +    }
> > +
> > +    ret = vfio_migration_init(vbasedev, info);
> > +    if (ret) {
> > +        goto add_blocker;
> > +    }
> > +
> > +    g_free(info);
> > +    trace_vfio_migration_probe(vbasedev->name, info->index);  
> 
> We free info, and then access info->index. Switching the
> order of the g_free() and the tracepoint seems the obvious fix.
> 
> thanks
> -- PMM
>