On 10/9/26 03:56, Jesse Zhang wrote:
> amdgpu_vm_pt_alloc() drops the eviction lock while it creates the new
> page table. When the VM started to get evicted in the meantime,
> amdgpu_vm_begin_critical() returns -EBUSY and we jump to error_free_pt
> before pt_bo is assigned. That unreferences an uninitialized pointer,
> leaks the new page table (still pinned for NPA VMs) and returns 0 since
> r is the result of the successful page table creation.
>
> With vm->evicting set right after amdgpu_vm_pt_create() (what
> amdgpu_vm_evictable() does when TTM evicts one of the VM page tables in
> that window), the first GEM_VA map that needs a new page table oopses:
>
> BUG: kernel NULL pointer dereference, address: 0000000000000460
> RIP: 0010:amdgpu_bo_unref+0x27/0x70 [amdgpu]
> Call Trace:
> amdgpu_vm_ptes_update+0xa4e/0xa60 [amdgpu]
> amdgpu_vm_update_range+0x2eb/0x970 [amdgpu]
> amdgpu_vm_bo_update+0x325/0x6d0 [amdgpu]
> amdgpu_gem_va_update_vm+0xdb/0x1c0 [amdgpu]
> amdgpu_gem_va_ioctl+0x86c/0x940 [amdgpu]
>
> Assign pt_bo first, unpin and free the new page table and return the
> error from amdgpu_vm_begin_critical().
>
> Fixes: 1e0041e2ac56 ("drm/amdgpu: rework eviction lock handling into critical
> section v3")
> Signed-off-by: Jesse Zhang <[email protected]>
Reviewed-by: Christian König <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> index c03327f1242d..599c0085cdd9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> @@ -531,13 +531,16 @@ static int amdgpu_vm_pt_alloc(struct
> amdgpu_vm_update_params *p,
> r2 = amdgpu_vm_begin_critical(p);
> if (r)
> return r;
> - if (r2)
> - goto error_free_pt;
> +
> + pt_bo = &pt->bo;
> + if (r2) {
> + r = r2;
> + goto error_unpin;
> + }
>
> /* Keep a reference to the root directory to avoid
> * freeing them up in the wrong order.
> */
> - pt_bo = &pt->bo;
> pt_bo->parent = amdgpu_bo_ref(cursor->parent->bo);
> amdgpu_vm_bo_base_init(entry, p->vm, pt_bo);
> r = amdgpu_vm_pt_clear(p->adev, p->vm, pt, p->immediate);
> @@ -549,7 +552,6 @@ static int amdgpu_vm_pt_alloc(struct
> amdgpu_vm_update_params *p,
> error_unpin:
> if (p->vm->is_npa)
> amdgpu_bo_unpin(pt_bo);
> -error_free_pt:
> amdgpu_bo_unref(&pt_bo);
> return r;
> }
