Re: [PATCH V19 09/15] drm/xe/vram: Add VRAM page offline fault handler

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] TOCTOU race allows eviction of the BO and destruction of innocent data
- [High] Deadlock via shrinker when allocating with GFP_KERNEL under vram_mgr->lock
- [High] Unconditional BO unpin and lack of state rollback causes reference count corruption
- [High] Use-After-Free via lockless access to exec_queue pointer
- [Medium] Failed page reservations are permanently silenced, allowing continued hardware faults
- [Medium] Faulty physical pages escape offlining when their owning BO is being freed
--

commit e00d08042f6e028303a11bd3305d790b12e07011
Author: Tejas Upadhyay <tejas.upadhyay@intel.com>
Subject: drm/xe/vram: Add VRAM page offline fault handler

This commit adds core VRAM page offlining logic to handle hardware-reported
faulty physical addresses. It introduces functions to purge BOs, verify
previously processed pages, reserve faulty buddy blocks, and map addresses to
VRAM regions.

> diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> index 1253989a8d06a..e5994c14487d3 100644
> --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c

[ ... ]

> @@ -572,3 +578,263 @@ u64 xe_ttm_vram_get_avail(struct ttm_resource_manager *man)

[ ... ]

> +static int xe_ttm_vram_purge_page(struct xe_device *xe, struct xe_bo *bo)
> +{

[ ... ]

> +	xe_bo_lock(bo, false);
> +	q = READ_ONCE(bo->q);
> +	/*  Ban exec queue if BO is lrc */
> +	if (q && xe_exec_queue_get_unless_zero(q)) {

[Severity: High]
Is this lockless access to the exec queue susceptible to a use-after-free?

When xe_ttm_vram_purge_page() reads the queue pointer locklessly, it appears
possible for a concurrent destruction path like xe_lrc_finish() to clear the
pointer and synchronously free the queue via xe_exec_queue_destroy().

If this thread resumes after the free but before the RCU grace period, it
might try to read the refcount of a freed object in
xe_exec_queue_get_unless_zero().

> +		/* ban queue */
> +		q_to_put = q;
> +	}
> +
> +	if (bo->purgeable.state == XE_MADV_PURGEABLE_PURGED) {
> +		/* Already purged by shrinker during unlocked window — nothing to do */
> +		xe_bo_unlock(bo);
> +		goto out;
> +	}
> +
> +	xe_bo_set_purgeable_state(bo, XE_MADV_PURGEABLE_DONTNEED);
> +	ttm_bo_unmap_virtual(&bo->ttm);   /* nuke CPU mmap + VRAM IO mappings */
> +	if (xe_bo_is_pinned(bo))
> +		xe_bo_unpin(bo);
> +	ret = xe_ttm_bo_purge(&bo->ttm, &ctx);
> +	xe_bo_unlock(bo);

[Severity: High]
Could this unconditional unpin corrupt the reference count if the purge
operation fails?

In xe_ttm_vram_purge_page(), if the BO was concurrently pinned multiple times
(for instance, by the display subsystem), dropping a single pin leaves the
count above zero.

Consequently, xe_ttm_bo_purge() will fail during validation and return an
error. Since the state and pin count are not rolled back on this failure
path, the legitimate owner might later unpin the BO, causing an underflow
panic.

[ ... ]

> +static int xe_ttm_vram_reserve_page_at_addr(struct xe_device *xe, u64 addr,
> +					    struct xe_ttm_vram_mgr *vram_mgr, struct gpu_buddy *mm)
> +{
> +	struct xe_ttm_vram_offline_resource *pos, *n;
> +	struct xe_ttm_vram_offline_resource *nentry;
> +	struct ttm_buffer_object *tbo = NULL;
> +	struct xe_bo *pbo_to_put = NULL;
> +	struct gpu_buddy_block *block;
> +	u64 size = SZ_4K;
> +	int ret = 0;
> +
> +	scoped_guard(mutex, &vram_mgr->lock) {
> +		if (xe_ttm_vram_page_already_processed(vram_mgr, addr))
> +			return -EEXIST;
> +		block = gpu_buddy_allocated_addr_to_block(mm, addr);
> +		if (WARN_ON(IS_ERR(block)))
> +			return PTR_ERR(block);
> +
> +		nentry = kzalloc_obj(*nentry);

[Severity: High]
Will this allocation risk a direct reclaim deadlock under memory pressure?

The xe_ttm_vram_reserve_page_at_addr() function acquires vram_mgr->lock and
then performs an implicit GFP_KERNEL allocation via kzalloc_obj().

If this allocation blocks and triggers direct reclaim, the GEM shrinker could
attempt to evict a VRAM BO. That eviction path will call
xe_ttm_vram_mgr_del(), which tries to acquire the same vram_mgr->lock,
resulting in an AB-BA deadlock.

> +		if (!nentry)
> +			return -ENOMEM;
> +		INIT_LIST_HEAD(&nentry->blocks);
> +		nentry->status = XE_PAGE_RESERVE_PENDING;
> +		nentry->addr = addr;
> +
> +		if (block) {
> +			struct xe_bo *pbo;
> +
> +			if (!block->private) {
> +				/* Race: another thread just reserved this block */
> +				kfree(nentry);
> +				return -EEXIST;
> +			}
> +			tbo = block->private;
> +			pbo = ttm_to_xe_bo(tbo);
> +
> +			/* Get reference safely - BO may have zero refcount */
> +			if (!xe_bo_get_unless_zero(pbo)) {
> +				kfree(nentry);
> +				return -ENOENT;
> +			}

[Severity: Medium]
Does returning early here allow the corrupted physical page to escape back to
the global free pool?

If xe_bo_get_unless_zero() fails in xe_ttm_vram_reserve_page_at_addr(), the
owning BO is already being destroyed.

By freeing the tracking entry and returning without recording the faulty
address in the offlined or queued lists, the memory manager might
subsequently reallocate these corrupted buddy blocks to a new, innocent BO.

[ ... ]

> +	if (block) {
> +		struct xe_bo *pbo = ttm_to_xe_bo(tbo);
> +
> +		/*
> +		 * Purge BO containing address - reference held from above.
> +		 * Note: brief window between purge (freeing blocks) and re-reserve
> +		 * below. If another allocation claims the block, buddy_alloc fails
> +		 * and the status will be shown as failed reservation.
> +		 */
> +		ret = xe_ttm_vram_purge_page(xe, pbo);
> +		xe_bo_put(pbo);

[Severity: Critical]
Is there a race condition during the unlocked window that could lead to data
destruction?

Since vram_mgr->lock is dropped before xe_ttm_vram_purge_page() is called in
xe_ttm_vram_reserve_page_at_addr(), the memory manager might concurrently
evict this BO to system memory and reuse its VRAM blocks for a completely
different BO.

When execution resumes, xe_ttm_vram_purge_page() would blindly destroy the
original BO that now safely resides in system memory. Furthermore, the new BO
occupying the faulty VRAM wouldn't be purged, and the subsequent buddy
allocation would fail.

> +		if (ret)
> +			drm_warn(&xe->drm, "Purge failed at addr:0x%llx, ret:%d\n", addr, ret);
> +	}
> +
> +	scoped_guard(mutex, &vram_mgr->lock) {
> +		ret = xe_ttm_vram_buddy_alloc(vram_mgr, addr, addr + size,
> +					      size, size, &nentry->blocks,
> +					      GPU_BUDDY_RANGE_ALLOCATION,
> +					      NULL, &nentry->used_visible_size);
> +		if (ret) {
> +			drm_warn(&xe->drm,
> +				 "Could not reserve page at addr:0x%llx, ret:%d\n",
> +				 addr, ret);
> +			nentry->status = XE_PAGE_RESERVE_FAIL;
> +			return ret;
> +		}

[Severity: Medium]
What happens to the faulty address if xe_ttm_vram_buddy_alloc() fails?

In this error path in xe_ttm_vram_reserve_page_at_addr(), the code returns
early without removing the tracking entry from the queued_pages list.

If the hardware later faults on this exact same physical address,
xe_ttm_vram_page_already_processed() will incorrectly report it as already
handled. This permanently blacklists the faulty page from future offlining
attempts.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831064942.315720-17-tejas.upadhyay@intel.com?part=9