Re: [PATCH 2/2] drm/imagination: Stop locking reservations around map and unmap

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/imagination: missing cleanup of deferred GPUVM BOs in pvr_vm_map()
--

commit 7b2c8feb3b00c98681f3c0f485d9d8ac9c58c5eb
Author: Gyeyoung Baek <gye976@gmail.com>

drm/imagination: Stop locking reservations around map and unmap

The GPUVA lists are now protected by gpuva.lock instead of the
reservations, so map and unmap no longer need to lock the reservations.
Nothing else on that path relies on them.

> diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
> index 3921b765688e1..2f1bbcfdb60e3 100644
> --- a/drivers/gpu/drm/imagination/pvr_vm.c
> +++ b/drivers/gpu/drm/imagination/pvr_vm.c

[ ... ]

> @@ -739,15 +729,6 @@ pvr_vm_map(struct pvr_vm_context *vm_ctx, struct pvr_gem_object *pvr_obj,
>  	   u64 pvr_obj_offset, u64 device_addr, u64 size)
>  {
>  	struct pvr_vm_bind_op bind_op = {0};
> -	struct drm_gpuvm_exec vm_exec = {
> -		.vm = &vm_ctx->gpuvm_mgr,
> -		.flags = DRM_EXEC_INTERRUPTIBLE_WAIT |
> -			 DRM_EXEC_IGNORE_DUPLICATES,
> -		.extra = {
> -			.fn = pvr_vm_lock_extra,
> -			.priv = &bind_op,
> -		},
> -	};
>  
>  	int err = pvr_vm_bind_op_map_init(&bind_op, vm_ctx, pvr_obj,
>  					  pvr_obj_offset, device_addr,

[Severity: Medium]
If pvr_vm_bind_op_map_init() fails after creating a gpuvm_bo, does this code
leak the deferred BO?

Looking at pvr_vm_bind_op_map_init(), if it fails (e.g., due to ENOMEM or an
import mapping failure in pvr_gem_object_get_pages_sgt()), the error path
calls pvr_vm_bind_op_fini() which defers the BO cleanup:

pvr_vm_bind_op_fini() {
	drm_gpuvm_bo_put_deferred(bind_op->gpuvm_bo);
	...
}

However, the early return path in pvr_vm_map() (right after the call to
pvr_vm_bind_op_map_init()) skips calling drm_gpuvm_bo_deferred_cleanup():

	int err = pvr_vm_bind_op_map_init(&bind_op, vm_ctx, pvr_obj,
					  pvr_obj_offset, device_addr,
					  size);

	if (err)
		return err;

Since pvr_vm_map() is reachable via the PVR_IOCTL_VM_MAP ioctl, could a faulty
or malicious userspace trigger this error path repeatedly and cause the deferred
list to grow unbounded, leaking kernel memory until the VM context is destroyed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011-pvr-gpuvm-immediate-v1-0-5b298dfce4f8@gmail.com?part=2