Re: [RFC PATCH v10 4/4] drm/virtio: wire blob ioctl creation to userptr objects

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/virtio: unnotified ctrlq strands blob on handle creation
  failure
--

commit f8b823e6be2332eb525d4e0d0576cd0869cd03e1
Author: Honglei Huang <honghuan@amd.com>

drm/virtio: wire blob ioctl creation to userptr objects

This commit integrates userptr creation and read-only blob support into
the blob resource creation and getparam ioctls, and switches the error
paths in virtio_gpu_resource_create_blob_ioctl() from
drm_gem_object_release() to drm_gem_object_put().

> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 3d8e4ccdb7c1f..e57750588df01 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
[ ... ]
> @@ -557,14 +586,14 @@ static int virtio_gpu_resource_create_blob_ioctl(struct drm_device *dev,
>  	if (params.blob_flags & VIRTGPU_BLOB_FLAG_USE_CROSS_DEVICE) {
>  		ret = virtio_gpu_resource_assign_uuid(vgdev, bo);
>  		if (ret) {
> -			drm_gem_object_release(obj);
> +			drm_gem_object_put(obj);
>  			return ret;
>  		}
>  	}
>  
>  	ret = drm_gem_handle_create(file, obj, &handle);
>  	if (ret) {
> -		drm_gem_object_release(obj);
> +		drm_gem_object_put(obj);
>  		return ret;
>  	}

[Severity: Medium]
Can this leave obj stranded in vgdev->ctrlq when drm_gem_handle_create()
fails after virtio_gpu_resource_assign_uuid() succeeds?

Neither the blob creation helpers nor virtio_gpu_resource_assign_uuid()
call virtio_gpu_notify(vgdev); notification normally happens at the end
of virtio_gpu_gem_object_open() inside drm_gem_handle_create().  However,
virtio_gpu_resource_assign_uuid() takes an extra reference on obj via
virtio_gpu_array_add_obj() for the queued command buffer:

drivers/gpu/drm/virtio/virtgpu_prime.c:virtio_gpu_resource_assign_uuid() {
	objs = virtio_gpu_array_alloc(1);
	if (!objs)
		return -ENOMEM;

	virtio_gpu_array_add_obj(objs, &bo->base.base);

	return virtio_gpu_cmd_resource_assign_uuid(vgdev, objs);
}

If drm_gem_handle_create() fails before virtio_gpu_gem_object_open()
reaches virtio_gpu_notify(vgdev), drm_gem_object_put(obj) only drops
obj->refcount from 2 to 1.  Because the refcount does not reach 0,
obj->funcs->free() is not called, leaving the queued commands un-kicked
and keeping userptr pages pinned and charged to mm->pinned_vm until an
unrelated command notifies ctrlq.  Calling virtio_gpu_notify(vgdev) on
the drm_gem_handle_create() failure path would ensure the queued command
completes and drops its reference.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106082625.812860-1-honghuan@amd.com?part=4