[PATCH v2 14/33] drm/xe: Destroy page tables after unlinking all VMAs on VM close

[Matthew Brost <matthew.brost@intel.com>]

xe_vm_close_and_put() freed the VM's page-table roots via
xe_vm_pt_destroy() before destroying the "contested" VMAs - those backed
by external (shared) BOs, which are deferred to xe_vma_destroy_unlocked()
because unlinking them requires taking the external BO's dma-resv lock
(via drm_exec), not vm->lock.

Until such a VMA is unlinked it remains reachable from its BO through
drm_gem_for_each_gpuvm_bo(). A concurrent BO move that only holds the
BO's dma-resv lock - TTM eviction, the shrinker, or the defrag worker -
calls xe_bo_move_notify() -> xe_bo_trigger_rebind() ->
xe_vm_invalidate_vma() -> xe_pt_zap_ptes(), which dereferences
vm->pt_root[tile->id]. With the page tables already freed this is a
use-after-free / NULL dereference, e.g.:

  Oops: general protection fault [...] kernel NULL pointer dereference
  RIP: xe_pt_zap_ptes+0x7c [xe]
  Call Trace:
   xe_vm_invalidate_vma_submit+0x64/0x130 [xe]
   xe_vm_invalidate_vma+0x27/0x60 [xe]
   xe_bo_move_notify+0x1e7/0x3d0 [xe]
   xe_bo_move+0x62d/0x1620 [xe]
   ttm_bo_handle_move_mem+0x243/0x330 [ttm]
   ttm_bo_validate+0xb4/0x1a0 [ttm]

The race is not closable with a flag or a NULL pt_root check in the
invalidate path: the freeing side holds only vm->lock while the walker
holds only the BO's resv, so there is no common lock to serialise on.

Fix the ordering instead: destroy the page tables only after every VMA -
including the contested external-BO VMAs - has been unlinked. Once the
contested loop completes no BO can reach a VMA of this VM, so freeing the
page tables (under vm->lock, to satisfy xe_vm_assert_held()) is safe.
xe_vm_free_scratch() still runs before the free as it reads
pt_root[]->level.

Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
Assisted-by: GitHub_Copilot:claude-opus-4.8
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/xe/xe_vm.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 4557a8a4d270..9f55d22cb985 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -1934,15 +1934,7 @@ void xe_vm_close_and_put(struct xe_vm *vm)
 		vma->gpuva.flags |= XE_VMA_DESTROYED;
 	}
 
-	/*
-	 * All vm operations will add shared fences to resv.
-	 * The only exception is eviction for a shared object,
-	 * but even so, the unbind when evicted would still
-	 * install a fence to resv. Hence it's safe to
-	 * destroy the pagetables immediately.
-	 */
 	xe_vm_free_scratch(vm);
-	xe_vm_pt_destroy(vm);
 	xe_vm_unlock(vm);
 
 	/*
@@ -1956,6 +1948,22 @@ void xe_vm_close_and_put(struct xe_vm *vm)
 		xe_vma_destroy_unlocked(vma);
 	}
 
+	/*
+	 * Destroy the page tables only after every VMA has been unlinked. The
+	 * contested (external-BO) VMAs above are unlinked from their BO under
+	 * the BO's dma-resv lock, not vm->lock, so they can still be reached
+	 * via drm_gem_for_each_gpuvm_bo() by a concurrent BO move (eviction,
+	 * shrinker or the defrag worker) that only holds the BO's resv. Such a
+	 * move calls xe_vm_invalidate_vma() -> xe_pt_zap_ptes(), which
+	 * dereferences vm->pt_root[]; freeing the page tables while those VMAs
+	 * are still linked is a use-after-free. All vm operations add shared
+	 * fences to the resv (eviction of a shared object still installs an
+	 * unbind fence), so no GPU work outlives this point.
+	 */
+	xe_vm_lock(vm, false);
+	xe_vm_pt_destroy(vm);
+	xe_vm_unlock(vm);
+
 	xe_svm_fini(vm);
 
 	up_write(&vm->lock);
-- 
2.34.1