Re: [PATCH v2] sched_ext: Don't BUG_ON a destroyed DSQ in process_deferred_reenq_users

在 2026/8/15 10:34, [email protected] 写道:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] Lockless double-read of `dsq->id` creates a Time-Of-Check to Time-Of-Use (TOCTOU) race that can still falsely trigger the `BUG_ON`, crashing the kernel.
> 
> Pre-existing issues:
> - [Critical] Dereferencing `dsq->id` in `process_deferred_reenq_users()` causes a Use-After-Free because an RCU grace period can complete while the target CPU is idle, freeing `dsq` before it is accessed.
> --
> 
> --- Patch [1]: [PATCH v2] sched_ext: Don't BUG_ON a destroyed DSQ in process_deferred_reenq_users ---
> commit f1934b9161ead6ceeb2cb6d00c64a01e1a233104
> Author: Tao Cui <[email protected]>
> 
> sched_ext: Don't BUG_ON a destroyed DSQ in process_deferred_reenq_users
> 
> This commit attempts to fix a BUG_ON triggered when process_deferred_reenq_users()
> encounters a destroyed DSQ. It adds a check to skip processing if the DSQ ID 
> has been set to SCX_DSQ_INVALID.
> 
>> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
>> index 18183062f751c..1a059e5f09e80 100644
>> --- a/kernel/sched/ext/ext.c
>> +++ b/kernel/sched/ext/ext.c
>> @@ -4332,6 +4332,10 @@ static void process_deferred_reenq_users(struct rq *rq)
>>  		/* see schedule_dsq_reenq() */
>>  		smp_mb();
>>  
>> +		/* destroy_dsq() may have raced and invalidated @dsq, nothing to reenq */
> 
> [Severity: Critical]
> This is a pre-existing issue, but does dereferencing dsq->id here outside of
> the rq->scx.deferred_reenq_lock critical section expose the pointer to a
> concurrent kfree() in the RCU teardown path?
> 
> If CPU B is idle (in an RCU Extended Quiescent State), and CPU A queues a
> deferred reenq for a DSQ to CPU B and calls destroy_dsq(), the RCU grace
> period can end immediately without waiting for CPU B. The RCU callback
> free_dsq_rcufn then runs on CPU C and calls exit_dsq().
> 
> Concurrently, if CPU B wakes up, enters process_deferred_reenq_users(),
> removes dru from the list, and drops the lock:
> 
> CPU B:
> scoped_guard (raw_spinlock, &rq->scx.deferred_reenq_lock) {
> 	...
> 	list_del_init(&dru->node);
> }
> // CPU B is now at the dsq->id check
> 
> CPU C's exit_dsq() then evaluates the list without locks:
> 
> CPU C:
> exit_dsq() {
> 	...
> 	if (WARN_ON_ONCE(!list_empty(&dru->node))) {
> 		// Skips lock because list is empty
> 	}
> 	...
> 	kfree(dsq);
> }
> 
> Would CPU B then read from the freed dsq memory when checking dsq->id?
> 
I think this one doesn't exist. schedule_dsq_reenq() queues the dru on the inserting CPU's own rq (rq = this_rq()), not on a remote CPU, and process_deferred_reenq_users() runs with rq->lock held, so the CPU cannot pass through a quiescent state between dequeuing the dru and reading dsq->id. A grace period cannot complete underneath it. The residual case where the dru is never processed is what the WARN and forced delete in exit_dsq() cover.
>> +		if (unlikely(dsq->id == SCX_DSQ_INVALID))
>> +			continue;
>> +
>>  		BUG_ON(dsq->id & SCX_DSQ_FLAG_BUILTIN);
> 
> [Severity: High]
> Because dsq->id is accessed twice consecutively without READ_ONCE(), can
> the compiler emit two separate memory loads?
> 
> If a concurrent destroy_dsq() sets dsq->id = SCX_DSQ_INVALID exactly
> between the two loads, the first load would see a valid user DSQ ID and
> bypass the continue. The second load would then observe SCX_DSQ_INVALID.
> 
> Since SCX_DSQ_INVALID evaluates to true for SCX_DSQ_FLAG_BUILTIN,
> wouldn't this still trigger the BUG_ON?
> 
Fair in theory. The two loads are adjacent and will be merged by the compiler in practice, but folding them into a single READ_ONCE() would close it for good. Happy to do that in a follow-up.
>>  		reenq_user(rq, dsq, reenq_flags);
>>  	}
>