Re: [PATCH] drm/amdgpu: fix NULL pointer dereference in amdgpu_vm_init error path

[Christian König]

On 8/25/26 11:29, Yang Zi wrote:
> In amdgpu_vm_init(), the error_free_root cleanup label runs
> amdgpu_vm_pt_free_root() before amdgpu_bo_unreserve(). However,
> amdgpu_vm_pt_free_root() walks the page table tree and frees each
> entry via amdgpu_vm_pt_free(), which calls amdgpu_bo_unref() on the
> root BO and clears vm->root.bo to NULL. The subsequent
> amdgpu_bo_unreserve(vm->root.bo) then dereferences a NULL pointer.
> 
> Swap the two statements so that the root BO is unreserved before it is
> freed. This mirrors the success path at the end of the function, which
> unreserves before releasing its reference.
> 
> Found by static analysis of the error handling path; the failure cases
> (dma_resv_reserve_fences() or amdgpu_vm_pt_clear() returning an error)
> trigger this path.
> 
> Signed-off-by: Yang Zi <2959243019@xxxxxx>
> ---
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index d2ad5b0e8759..dc72181ea0fa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2672,8 +2672,8 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>      return 0;
> 
>  error_free_root:
> -    amdgpu_vm_pt_free_root(adev, vm);
>      amdgpu_bo_unreserve(vm->root.bo);
> +    amdgpu_vm_pt_free_root(adev, vm);

Clear NAK, that is just nonsense and will potentially trigger tons of warnings.

The amdgpu_bo_unreserve(() call just needs to use the local variable root_bo instead.

Regards,
Christian.

>      amdgpu_bo_unref(&root_bo);
> 
>  error_free_delayed:
>