On 5/7/25 05:51, Kasireddy, Vivek wrote:
> 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.osipe...@collabora.com>
>>> Cc: Thomas Zimmermann <tzimmerm...@suse.de>
>>> Signed-off-by: Vivek Kasireddy <vivek.kasire...@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;
> }
Indeed, it was changed a month ago by [1].
Then the patch looks okay, thanks. Will merge soon.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8260731ccad0451207b45844bb66eb161a209218
--
Best regards,
Dmitry
