Re: [PATCH v2] drm/imagination: acquire vm_ctx->lock before mapping memory to GPU VM

[Alessio Belle]

On Tue, 2026-07-14 at 15:36 +0800, Icenowy Zheng wrote:
> The drm gpuvm code doesn't protect find operation against map operation,
> and the driver needs to ensure a map operation shouldn't happen when a
> find operation is in progress.
> 
> In some cases a find operation will be in progress when doing map/unmap
> operations, and the find operation will do a NULL pointer deference.
> 
> An example of the stack trace of such NULL deference is shown below:
> 
> ```
> Unable to handle kernel access to user memory without uaccess routines at
> virtual address 0000000000000010
> 
> [<ffffffff01e989d4>] drm_gpuva_find+0x28/0x6c [drm_gpuvm]
> [<ffffffff01ed3a40>] pvr_vm_unmap+0x34/0x68 [powervr]
> [<ffffffff01ec69da>] pvr_ioctl_vm_unmap+0x2e/0x50 [powervr]
> [<ffffffff8080ce0a>] drm_ioctl_kernel+0x8e/0xdc
> [<ffffffff8080d016>] drm_ioctl+0x1be/0x3e0
> [<ffffffff802bec3e>] __riscv_sys_ioctl+0xba/0xc4
> [<ffffffff80d858b2>] do_trap_ecall_u+0x23e/0x3f4
> [<ffffffff80d92288>] handle_exception+0x168/0x174
> ```
> 
> As all occurences of drm_gpuva_find*() are already guarded by
> vm_ctx->lock, make pvr_vm_map() to acquire this lock to prevent
> disturbing any find operation. This fixes the NULL deference problem in
> drm_gpuva_find*().
> 
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
> Fixes: 4bc736f890ce ("drm/imagination: vm: make use of GPUVM's drm_exec helper")
> Signed-off-by: Icenowy Zheng <zhengxingda@xxxxxxxxxxx>

Reviewed-by: Alessio Belle <alessio.belle@xxxxxxxxxx>

p.s. there's a double typo deference -> dereference that I can fix before
applying this early next week.

Thanks,
Alessio

> ---
> Changes in v2:
> - Dropped wrongly duplicated mutex_unlock() call and reordered it to
>   before pvr_vm_bind_op_fini() (the lock is acquired after
>   pvr_vm_bind_op_map_init() call). (Thanks to Brajesh)
> - Added a extra Fixes pointing to the original broken code that was
>   refactored by the original Fixes (but still broken). (Thanks to
>   Alessio)
> - Fixed some typos in commit message. (Thanks to Alessio)
> - Added a stacktrace of the oops that occured on my board. (As suggested
>   by Alessio)
> 
>  drivers/gpu/drm/imagination/pvr_vm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
> index 396d349fb6ce4..ceb78694cd987 100644
> --- a/drivers/gpu/drm/imagination/pvr_vm.c
> +++ b/drivers/gpu/drm/imagination/pvr_vm.c
> @@ -747,6 +747,7 @@ pvr_vm_map(struct pvr_vm_context *vm_ctx, struct pvr_gem_object *pvr_obj,
>  
>  	pvr_gem_object_get(pvr_obj);
>  
> +	mutex_lock(&vm_ctx->lock);
>  	err = drm_gpuvm_exec_lock(&vm_exec);
>  	if (err)
>  		goto err_cleanup;
> @@ -756,6 +757,7 @@ pvr_vm_map(struct pvr_vm_context *vm_ctx, struct pvr_gem_object *pvr_obj,
>  	drm_gpuvm_exec_unlock(&vm_exec);
>  
>  err_cleanup:
> +	mutex_unlock(&vm_ctx->lock);
>  	pvr_vm_bind_op_fini(&bind_op);
>  
>  	return err;