Re: [V3] drm/xe: Skip clearing purged page-table BOs

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

On Tue, Sep 08, 2026 at 01:26:04PM +0000, Upadhyay, Tejas wrote:
> 
> 
> > -----Original Message-----
> > From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> > Sent: 08 September 2026 17:35
> > To: Upadhyay, Tejas <tejas.upadhyay@intel.com>
> > Cc: intel-xe@lists.freedesktop.org
> > Subject: Re: [V3] drm/xe: Skip clearing purged page-table BOs
> > 
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
> > to consider:
> > 
> > New issues:
> > - [Critical] Early release of xe_svm_notifier_lock in xe_vm_close allows MMU
> > notifiers to unmap memory while GPU page tables are still valid, leading to a
> > physical memory use-after-free.
> > 
> > Pre-existing issues:
> > - [High] Missing NULL check for pagetable BO `vmap` in
> > `xe_pt_zap_ptes_entry` leads to a crash if the BO is purged.
> > --
> > 
> > commit 70efaa606c2633943ad2267dfc95c288525971af
> > Author: Tejas Upadhyay <tejas.upadhyay@intel.com>
> > 
> > drm/xe: Skip clearing purged page-table BOs
> > 
> > VRAM page offlining can clear a page-table BO's vmap before VM teardown
> > calls xe_pt_clear(). Check for a purged BO under its dma-resv lock before
> > writing to the mapping.
> > 
> > > diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> > > index 5d990c1c3740a..426e8a29c755a 100644
> > > --- a/drivers/gpu/drm/xe/xe_pt.c
> > > +++ b/drivers/gpu/drm/xe/xe_pt.c
> > > @@ -236,9 +236,11 @@ void xe_pt_destroy(struct xe_pt *pt, u32 flags,
> > struct llist_head *deferred)
> > >   */
> > >  void xe_pt_clear(struct xe_device *xe, struct xe_pt *pt)  {
> > > -	struct iosys_map *map = &pt->bo->vmap;
> > > +	struct xe_bo *bo = pt->bo;
> > >
> > > -	xe_map_memset(xe, map, 0, 0, SZ_4K);
> > > +	xe_bo_assert_held(bo);
> > > +	if (!iosys_map_is_null(&bo->vmap))
> > > +		xe_map_memset(xe, &bo->vmap, 0, 0, SZ_4K);
> > >  }
> > 
> > [Severity: High]
> > This isn't a bug introduced by this patch, but does xe_pt_zap_ptes_entry() in
> > drivers/gpu/drm/xe/xe_pt.c have a similar regression with missing NULL
> > checks for the pagetable BO vmap?
> > 
> > Looking at xe_pt_zap_ptes_entry(), it accesses &xe_child->bo->vmap without
> > validating it:
> 
> Found couple of more places like, add_compact_pt_prl() and xe_migrate_update_pgtables_cpu(). Will hand there as well.
> 

See my comment here [1]. Properly handling offline PT BOs and making
this safe with respect to zaps, asynchronous VM binds, and asynchronous
TLB invalidations will likely require a larger follow-up change. Let's
focus on getting this fix merged first before expanding the scope, as
this change is both necessary and correct on its own.

[1] https://patchwork.freedesktop.org/patch/750921/?series=173375&rev=1#comment_1384263

> > 
> > 	xe_map_memset(tile_to_xe(xe_walk->tile), &xe_child->bo->vmap,
> > 		      offset * sizeof(u64), 0,
> > 		      (end_offset - offset) * sizeof(u64));
> > 
> > This is reachable via xe_vm_invalidate_vma(), which only holds the user BO's
> > dma_resv lock or the SVM notifier_lock as seen in xe_pt_zap_ptes():
> > 
> > 	else if (xe_vma_is_userptr(vma))
> > 		lockdep_assert_held(&xe_vma_vm(vma)-
> > >svm.gpusvm.notifier_lock);
> > 
> > Since the page-table BO's dma_resv lock (the VM dma_resv lock) is not held,
> > could a concurrent VRAM page offlining event purge the page-table BO and
> > clear its vmap, leading to a NULL pointer dereference when
> > xe_map_memset() executes?
> > 
> > [ ... ]
> > 
> > > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > > index 9e71567b27cc7..e77ff3cf3ca38 100644
> > > --- a/drivers/gpu/drm/xe/xe_vm.c
> > > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > > @@ -1876,10 +1876,13 @@ static void xe_vm_close(struct xe_vm *vm)
> > >  	bound = drm_dev_enter(&xe->drm, &idx);
> > >
> > >  	down_write(&vm->lock);
> > > +	xe_vm_lock(vm, false);
> > > +
> > >  	if (xe_vm_in_fault_mode(vm))
> > >  		xe_svm_notifier_lock(vm);
> > > -
> > >  	vm->size = 0;
> > > +	if (xe_vm_in_fault_mode(vm))
> > > +		xe_svm_notifier_unlock(vm);
> > 
> > [Severity: Critical]
> > Does moving xe_svm_notifier_unlock() here create a race condition with
> > MMU notifiers?
> 
> Will fix this.
> 

Yea, don't move how long this lock is held in this patch.

Matt

> Tejas
> > 
> > If xe_svm_notifier_unlock() is called here before the GPU page tables are
> > cleared and the TLB is invalidated, a concurrent MMU notifier like
> > xe_svm_invalidate() could execute.
> > 
> > Since vm->size is 0, xe_svm_invalidate() in drivers/gpu/drm/xe/xe_svm.c will
> > check:
> > 
> > 	if (xe_vm_is_closed(vm))
> > 		goto range_notifier_event_end;
> > 
> > This check evaluates to true, skipping the zapping of GPU page tables and
> > allowing the physical memory to be freed prematurely. Could the GPU then
> > still read and write to these freed physical pages (which might be reallocated
> > to other processes) until xe_vm_close() finally clears the page tables?
> > 
> > --
> > Sashiko AI review ·
> > https://sashiko.dev/#/patchset/20260908114545.915049-2-
> > tejas.upadhyay@intel.com?part=1