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

[Alessio Belle <Alessio.Belle@imgtec.com>]

Hi Icenowy,

On Wed, 2026-04-22 at 01:57 +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.
> 
> As all occurences of drm_gpuva_find*() is already guarded by

nit: is -> are

> vm_ctx->lock, make pvr_vm_map() to acquire this lock to prevent
> disturbing any find operation.
> 
> This fixes occasional NULL deference in drm_gpuva_find*().

nit: NULL deference -> NULL [pointer] dereference

Do you still have kernel logs following this bug?

> 
> Cc: stable@vger.kernel.org
> Fixes: 4bc736f890ce ("drm/imagination: vm: make use of GPUVM's drm_exec helper")

As far as I can see, that commit swapped one way of locking resources with
another, but the problem of VA find/map/unmap operations not being protected by
the same lock already existed in commit ff5f643de0bf ("drm/imagination: Add GEM
and VM related code").

Thanks,
Alessio

> Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
> ---
> Changes in v2:
> - Fixed wrong commit prefix.
> 
>  drivers/gpu/drm/imagination/pvr_vm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
> index e1ec60f34b6e6..eea88e7ad03c1 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;
> @@ -754,9 +755,11 @@ pvr_vm_map(struct pvr_vm_context *vm_ctx, struct pvr_gem_object *pvr_obj,
>  	err = pvr_vm_bind_op_exec(&bind_op);
>  
>  	drm_gpuvm_exec_unlock(&vm_exec);
> +	mutex_unlock(&vm_ctx->lock);
>  
>  err_cleanup:
>  	pvr_vm_bind_op_fini(&bind_op);
> +	mutex_unlock(&vm_ctx->lock);
>  
>  	return err;
>  }