Re: [PATCH V19 02/15] drm/xe: Link LRC BO and its execution Queue

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Flawed synchronization design between LRC BO queue back-pointer reader and queue teardown, potentially leading to use-after-free of queue internals and queue memory.
--

commit 533bbdfdba916103ef324691d9fb513031965dd4
Author: Tejas Upadhyay <tejas.upadhyay@intel.com>

drm/xe: Link LRC BO and its execution Queue

To establish a link between an LRC BO and its corresponding execution
Queue, a back-pointer is stored within the BO's private data structure.
This allows the driver to identify and take corrective action on the
specific queue if the LRC BO encounters an error.

[ ... ]
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> index c4213bb9c137f..91ed6c0fac84e 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> @@ -387,6 +387,12 @@ static int __xe_exec_queue_init(struct xe_exec_queue *q, u32 exec_queue_flags)
>  				goto err_lrc;
>  			}
>  
> +			/*
> +			 * The queue ref counts the LRCs, thus it safe for the LRC BO to hold a
> +			 * pointer to queue without reference.  The reader holds dma_resv (
> +			 * xe_bo_lock) which serializes with xe_lrc_finish().
> +			 */
> +			WRITE_ONCE(lrc->bo->q, xe_exec_queue_multi_queue_primary(q));

[Severity: High]
Does this code introduce a flawed synchronization design between the LRC BO
queue back-pointer reader and queue teardown, potentially leading to a
use-after-free of queue internals and queue memory?

If a multi-LRC queue creation succeeds for the first LRC (i=0) but fails for a
subsequent one, the error path calls __xe_exec_queue_fini(q) while the queue's
refcount is still 1. 

__xe_exec_queue_fini(q) destroys backend structures before dropping LRC
references (which would clear bo->q via xe_lrc_finish()):

drivers/gpu/drm/xe/xe_exec_queue.c:__xe_exec_queue_fini() {
	...
	q->ops->fini(q);

	for (i = 0; i < q->width; ++i)
		xe_lrc_put(q->lrc[i]);
	...
}

Because the backend structures are destroyed before the back-pointer is cleared,
there is a window where bo->q points to a partially destroyed queue.

Concurrently, if a VRAM page offline or memory shrinker (in xe_ttm_vram_mgr.c)
locks the first LRC's BO, reads bo->q, and successfully takes a reference
(xe_exec_queue_get_unless_zero(q) succeeds because refcount=1), the reader
could then call xe_exec_queue_kill(q). This would access already-freed
backend structures (e.g., q->guc->sched).

Additionally, looking at the error path for xe_exec_queue_create(), the code
forcefully frees the queue memory without checking if a concurrent reader via
bo->q has elevated the refcount:

drivers/gpu/drm/xe/xe_exec_queue.c:xe_exec_queue_create() {
	...
err_post_init:
	__xe_exec_queue_fini(q);
err_post_alloc:
	__xe_exec_queue_free(q);
	return ERR_PTR(err);
}

Can this lead to a use-after-free of the queue memory itself when the
concurrent reader later uses or puts the queue?

>  			xe_exec_queue_set_lrc(q, lrc, i);
>  
>  			if (__lrc)
[ ... ]

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