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

[sashiko-bot@kernel.org]

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:

	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?

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