On 7/6/26 14:58, Shahyan Soltani wrote:
> The vm pointer returned from amdgpu_vm_get_vm_from_pasid() is only
> valid while the lock is still being held. Once xa_unlock_irqrestore is
> called and returned, the pointer is no longer under lock and is subject
> to modification. Since, the caller still dereferences vm->task_info in
> amdgpu_vm_get_task_info_vm() after the lock is removed, this causes a
> use after unlock problem.
> 
> Remove the lifetime issue present in amdgpu_vm_get_task_info_pasid()
> through removing the amdgpu_vm_get_vm_from_pasid() function from
> amdgpu_vm.c and making the relevant code inline to hold the lock while
> it is still in use.
> 
> Signed-off-by: Shahyan Soltani <[email protected]>

Reviewed-by: Christian König <[email protected]>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 25 ++++++++++---------------
>  1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 32719f31b6c9..180d694323e8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2460,19 +2460,6 @@ static void amdgpu_vm_destroy_task_info(struct kref 
> *kref)
>       kfree(ti);
>  }
>  
> -static inline struct amdgpu_vm *
> -amdgpu_vm_get_vm_from_pasid(struct amdgpu_device *adev, u32 pasid)
> -{
> -     struct amdgpu_vm *vm;
> -     unsigned long flags;
> -
> -     xa_lock_irqsave(&adev->vm_manager.pasids, flags);
> -     vm = xa_load(&adev->vm_manager.pasids, pasid);
> -     xa_unlock_irqrestore(&adev->vm_manager.pasids, flags);
> -
> -     return vm;
> -}
> -
>  /**
>   * amdgpu_vm_put_task_info - reference down the vm task_info ptr
>   *
> @@ -2519,8 +2506,16 @@ amdgpu_vm_get_task_info_vm(struct amdgpu_vm *vm)
>  struct amdgpu_task_info *
>  amdgpu_vm_get_task_info_pasid(struct amdgpu_device *adev, u32 pasid)
>  {
> -     return amdgpu_vm_get_task_info_vm(
> -                     amdgpu_vm_get_vm_from_pasid(adev, pasid));
> +     struct amdgpu_task_info *ti;
> +     struct amdgpu_vm *vm;
> +     unsigned long flags;
> +
> +     xa_lock_irqsave(&adev->vm_manager.pasids, flags);
> +     vm = xa_load(&adev->vm_manager.pasids, pasid);
> +     ti = amdgpu_vm_get_task_info_vm(vm);
> +     xa_unlock_irqrestore(&adev->vm_manager.pasids, flags);
> +
> +     return ti;
>  }
>  
>  static int amdgpu_vm_create_task_info(struct amdgpu_vm *vm)

Reply via email to