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 <[email protected]>
---
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
