Re: [PATCH] drm/xe: fix UAF race between exec-queue teardown and GuC CT pause-abort

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

On Fri, Nov 13, 2026 at 01:46:25PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [High] drm/xe: stalled CLEANUP and queue leak in
>   xe_guc_submit_pause_abort()
> --
> 
> commit 904ab0c1661eb0e9b87cd4d66b549b254ac2e0f0
> Author: Tejas Upadhyay <tejas.upadhyay@intel.com>
> 
> drm/xe: fix UAF race between exec-queue teardown and GuC CT pause-abort
> 
> This commit uses kref_get_unless_zero() in xe_guc_submit_pause_abort() to
> skip exec queues whose refcount has already dropped to zero during
> concurrent teardown.
> 
> > diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> > index 9e7c2778029ca..92290b8be24aa 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> > @@ -3297,12 +3297,18 @@ void xe_guc_submit_pause_abort(struct xe_guc *guc)
> >  	xa_for_each(&guc->submission_state.exec_queue_lookup, index, q) {
> >  		struct xe_gpu_scheduler *sched = &q->guc->sched;
> >  
> > -		/* Prevent redundant attempts to stop parallel queues */
> > -		if (q->guc->id != index)
> > +		/*
> > +		 * Prevent redundant attempts to stop parallel queues, and skip
> > +		 * queues already dropped to a zero refcount via their own
> > +		 * independent teardown (guc_exec_queue_fini(), xarray entry
> > +		 * not yet erased)
> > +		 */
> > +		if (q->guc->id != index || !kref_get_unless_zero(&q->refcount))
> >  			continue;
> >  
> >  		xe_sched_submission_start(sched);
> >  		guc_exec_queue_kill(q);
> > +		xe_exec_queue_put(q);
> >  	}
> >  	mutex_unlock(&guc->submission_state.lock);
> >  }
> 
> [Severity: High]
> Does skipping xe_sched_submission_start(sched) when
> kref_get_unless_zero(&q->refcount) fails leave sched permanently stopped
> with an unprocessed CLEANUP message, leaking q?
> 

This is right - if the kref == 0, the queue still needs to be started again.

Matt

> When the final reference to q is dropped via xe_exec_queue_put(),
> q->refcount is already zero when xe_exec_queue_destroy() ->
> guc_exec_queue_destroy() queues STATIC_MSG_CLEANUP onto sched->msgs to run
> teardown asynchronously on sched->work_process_msg:
> 
> drivers/gpu/drm/xe/xe_guc_submit.c:guc_exec_queue_destroy() {
> 	struct xe_sched_msg *msg = q->guc->static_msgs + STATIC_MSG_CLEANUP;
> 
> 	guc_exec_queue_add_msg(q, msg, CLEANUP);
> }
> 
> Callers of xe_guc_submit_pause_abort() first stop every scheduler in
> exec_queue_lookup via xe_guc_submit_stop() -> xe_sched_submission_stop(),
> which stops the workqueue and cancels sched->work_process_msg.  During
> driver unbind or SR-IOV VF removal, guc_submit_fini() drops references on
> wedged queues (and earlier devm release actions such as xe_migrate_fini()
> drop kernel queues) right before calling xe_guc_submit_stop().  If
> xe_sched_submission_stop() runs before work_process_msg pops
> STATIC_MSG_CLEANUP, exec_queue_destroyed(q) is still false, so
> guc_exec_queue_stop() does not destroy the queue either.
> 
> When xe_guc_submit_pause_abort() then skips xe_sched_submission_start(sched)
> because q->refcount is zero, STATIC_MSG_CLEANUP remains stranded on the
> stopped scheduler.  guc_exec_queue_destroy_async() and guc_exec_queue_fini()
> never run, leaking q along with its runtime PM and drm_device references,
> leaving its entry in exec_queue_lookup (which trips the xa_empty() assert in
> guc_submit_sw_fini()), and hanging xe_file_close() on
> xef->exec_queue.pending_removal for user queues.
> 
> Should xe_sched_submission_start(sched) be called before checking
> kref_get_unless_zero(&q->refcount) so pending CLEANUP messages can still
> be processed?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261113132149.2989667-2-tejas.upadhyay@intel.com?part=1