[PATCH 1/2] drm/amdgpu: fix the error path in amdgpu_vm_pt_alloc()

[Jesse Zhang]

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 <jesse.zh...@amd.com>
---
 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;
 }
-- 
2.50.1