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

[Icenowy Zheng]

在 2026-07-17五的 11:14 +0000，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.

Oops I made the same mistake again when rephrasing the message...

Maybe I need some reinforced learning.

Thanks,
Icenowy

> 
> 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;