On Tue, Nov 03, 2026 at 05:44:10PM -0800, Niranjana Vishwanathapura wrote:
> On Fri, Jul 10, 2026 at 07:56:00PM -0700, Matthew Brost wrote:
> > 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]>
>
> Looks like this change is already merged with below commit
> "drm/xe/vm: nuke PTs only after unlinking contested VMAs"
>
> So, needs to be dropped in this series.
>
Yes, Auld got to this one so it can be droppped.
Matt
> Niranjana
>
> > ---
> > 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
> >
