[PATCH] drm/virtio: Fix NULL pointer deref in virtgpu_dma_buf_free_obj()

[Kasireddy, Vivek]

Hi Dmitry,

> Subject: Re: [PATCH] drm/virtio: Fix NULL pointer deref in
> virtgpu_dma_buf_free_obj()
> 
> On 5/2/25 02:24, Vivek Kasireddy wrote:
> > There is a chance that obj->dma_buf would be NULL by the time
> > virtgpu_dma_buf_free_obj() is called. This can happen for imported
> > prime objects, when drm_gem_object_exported_dma_buf_free() gets
> called
> > on them before drm_gem_object_free(). This is because
> > drm_gem_object_exported_dma_buf_free() explicitly sets
> > obj->dma_buf to NULL.
> >
> > Therefore, fix this issue by storing the dma_buf pointer in the
> > virtio_gpu_object instance and using it in virtgpu_dma_buf_free_obj.
> > This stored pointer is guaranteed to be valid until the object is
> > freed as we took a reference on it in virtgpu_gem_prime_import().
> >
> > Fixes: 415cb45895f4 ("drm/virtio: Use dma_buf from GEM object
> > instance")
> > Cc: Dmitry Osipenko <dmitry.osipenko at collabora.com>
> > Cc: Thomas Zimmermann <tzimmermann at suse.de>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
> > ---
> >  drivers/gpu/drm/virtio/virtgpu_drv.h   | 1 +
> >  drivers/gpu/drm/virtio/virtgpu_prime.c | 3 ++-
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
> > b/drivers/gpu/drm/virtio/virtgpu_drv.h
> > index f17660a71a3e..f7def8b42068 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> > @@ -88,6 +88,7 @@ struct virtio_gpu_object_params {
> >
> >  struct virtio_gpu_object {
> >  	struct drm_gem_shmem_object base;
> > +	struct dma_buf *dma_buf;
> >  	struct sg_table *sgt;
> >  	uint32_t hw_res_handle;
> >  	bool dumb;
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c
> > b/drivers/gpu/drm/virtio/virtgpu_prime.c
> > index 1118a0250279..722cde5e2d86 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_prime.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
> > @@ -206,7 +206,7 @@ static void virtgpu_dma_buf_free_obj(struct
> drm_gem_object *obj)
> >  	struct virtio_gpu_device *vgdev = obj->dev->dev_private;
> >
> >  	if (drm_gem_is_imported(obj)) {
> > -		struct dma_buf *dmabuf = obj->dma_buf;
> > +		struct dma_buf *dmabuf = bo->dma_buf;
> 
> drm_gem_is_imported() checks whether obj->dma_buf is NULL, hence
> drm_gem_is_imported() can't be used here too?
Unless I am missing something, it looks like drm_gem_is_imported() does
not seem to check obj->dma_buf:

static inline bool drm_gem_is_imported(const struct drm_gem_object *obj)
{
        return !!obj->import_attach;
}

Thanks,
Vivek

> 
> --
> Best regards,
> Dmitry