Re: [PATCH] sched_ext: Hold DSQ refs for deferred reenqueues

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] sched_ext: invalid wait context calling kfree() under raw_spinlock_t
- [High] sched_ext: UAF due to missing memory barrier in deferred reenqueue
--

commit 32dd5c50544bf44b37d3cb4933b2658023766e35
Author: Hui Su <sh_def@163.com>

sched_ext: Hold DSQ refs for deferred reenqueues

This patch adds a reference count to DSQ to protect deferred reenqueues.
It takes a reference before detaching the node and drops it after the final 
DSQ access, preventing the DSQ from being freed prematurely by the RCU
callback.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 6b037d27c99d3..88f229bd0c4d2 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
[ ... ]
> @@ -5075,6 +5075,7 @@ static void process_deferred_reenq_users(struct rq *rq)
>  			dsq_pcpu = container_of(dru, struct scx_dsq_pcpu,
>  						deferred_reenq_user);
>  			dsq = dsq_pcpu->dsq;
> +			refcount_inc(&dsq->deferred_reenq_refs);
>  			reenq_flags = dru->flags;
>  			WRITE_ONCE(dru->flags, 0);
>  			list_del_init(&dru->node);

[Severity: High]
Is a memory barrier needed here to ensure the refcount increment is
visible before the list deletion?

Because refcount_inc() uses relaxed atomics, it provides no memory
ordering guarantees. On weakly-ordered architectures, the CPU can commit
the list_del_init() to memory before the refcount_inc().

Concurrently, exit_dsq() performs a lockless read check:

kernel/sched/ext/ext.c:exit_dsq() {
    ...
    if (WARN_ON_ONCE(!list_empty(&dru->node))) {
        ...
    }
}

If exit_dsq() observes the reordered list deletion, it skips acquiring
the spinlock and proceeds to drop the base refcount. Since the refcount
increment hasn't reached global visibility yet, the base refcount drops
from 1 to 0 and dsq is prematurely freed, which can cause a
use-after-free when the deferred path continues.

Additionally, list_del_init() is not atomic and momentarily leaves
node->next pointing to the old element before pointing to itself. The
lockless check in exit_dsq() could evaluate to true during this window,
triggering a spurious warning.

[ ... ]
> @@ -5086,10 +5087,14 @@ static void process_deferred_reenq_users(struct rq *rq)
>  		/* destroy_dsq() may have raced and invalidated @dsq, nothing to reenq */
>  		dsq_id = READ_ONCE(dsq->id);
>  		if (unlikely(dsq_id == SCX_DSQ_INVALID))
> -			continue;
> +			goto put_dsq;
>  
>  		BUG_ON(dsq_id & SCX_DSQ_FLAG_BUILTIN);
>  		reenq_user(rq, dsq, reenq_flags);
> +
> +put_dsq:
> +		if (refcount_dec_and_test(&dsq->deferred_reenq_refs))
> +			kfree(dsq);

[Severity: High]
Does calling kfree() here violate lock nesting rules?

The process_deferred_reenq_users() function is executed with the
runqueue lock held, which is a raw_spinlock_t (never sleeps).

The slab allocator's kfree() slow path acquires internal spinlock_t or
local locks, which can sleep on PREEMPT_RT kernels. Acquiring a
sleepable lock while holding a true spinning lock triggers an "Invalid
wait context" splat and could potentially lead to deadlocks.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930101722.2468434-1-sh_def@163.com?part=1