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

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

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

scx_bpf_dsq_reenq() queues a deferred reenq (dru) that runs from
run_deferred(), not ops.dispatch(). If the DSQ is destroyed before the dru
runs, process_deferred_reenq_users() sees dsq->id == SCX_DSQ_INVALID and
hits the BUG_ON. destroy_dsq() doesn't flush pending drus, so just skip.

Fixes: 84b1a0ea0b7c ("sched_ext: Implement scx_bpf_dsq_reenq() for user DSQs")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
Change in v2:
Only skip SCX_DSQ_INVALID and keep the BUG_ON for other builtin ids
instead of skipping every builtin-flagged id, per Tejun.
---
 kernel/sched/ext/ext.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 26dbbbfe5a87..7d097209e778 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -4637,6 +4637,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 */
+		if (unlikely(dsq->id == SCX_DSQ_INVALID))
+			continue;
+
 		BUG_ON(dsq->id & SCX_DSQ_FLAG_BUILTIN);
 		reenq_user(rq, dsq, reenq_flags);
 	}
-- 
2.43.0

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

[sashiko-bot]

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 <cuitao@kylinos.cn>

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?

> +		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?

>  		reenq_user(rq, dsq, reenq_flags);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815022017.3305427-1-cui.tao@linux.dev?part=1

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

[Tao Cui]

在 2026/8/15 10:34, sashiko-bot@kernel.org 写道:
> 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 <cuitao@kylinos.cn>
> 
> 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);
>>  	}
>