Re: [PATCH v2 sched_ext/for-7.3 22/36] sched_ext: Add sub_ecaps_updated() effective-cap change notifier

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Use-After-Free of `pcpu` in `scx_process_sync_ecaps` due to `rq_lock` being dropped
--

commit 93fedaa49ec9781e4eea1aa5bfcf150af020301b
Author: Tejun Heo <tj@kernel.org>

sched_ext: Add sub_ecaps_updated() effective-cap change notifier

This commit adds a sub_ecaps_updated() callback to notify sub-schedulers
when effective caps change on a CPU. It runs in dispatch context so the
scheduler can directly insert or preempt tasks, and is hooked up with the
CPU hotplug machinery.

> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index 3d488c1b0d599..deb4c802af83a 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c

[ ... ]

> @@ -355,33 +357,140 @@ void scx_process_sync_ecaps(struct rq *rq)
>  	if (likely(llist_empty(&rq->scx.ecaps_to_sync)))
>  		return;
>  
[ ... ]
> +		ecaps = calc_effective_caps(ps, cid);
> +		WRITE_ONCE(pcpu->ecaps, ecaps);
> +
> +		/* tell the sched its effective caps on this cid changed */
> +		if (ecaps != pcpu->reported_ecaps &&
> +		    SCX_HAS_OP(pcpu->sch, sub_ecaps_updated) &&
> +		    !scx_bypassing(pcpu->sch, cpu)) {
> +			struct scx_dsp_ctx *dspc = &pcpu->dsp_ctx;
> +
> +			dspc->rq = rq;
> +			/* stash @prev so nested dispatches can access it */
> +			rq->scx.sub_dispatch_prev = prev;
> +			SCX_CALL_OP(pcpu->sch, sub_ecaps_updated, rq, scx_cpu_arg(cpu),
> +				    pcpu->reported_ecaps, ecaps);
> +			rq->scx.sub_dispatch_prev = NULL;
> +			scx_flush_dispatch_buf(pcpu->sch, rq);
> +			pcpu->reported_ecaps = ecaps;

[Severity: Critical]
Can this write cause a use-after-free of pcpu if scx_flush_dispatch_buf()
temporarily drops the rq lock?

If the sub_ecaps_updated() callback dispatches a task from a remote
runqueue, scx_flush_dispatch_buf() -> dispatch_to_local_dsq() can
temporarily drop the local rq lock:

kernel/sched/ext/ext.c:dispatch_to_local_dsq() {
    ...
    if (locked_rq != src_rq) {
        raw_spin_rq_unlock(locked_rq);
        locked_rq = src_rq;
        raw_spin_rq_lock(src_rq);
    }
    ...
}

If a concurrent detachment is waiting in scx_discard_ecaps_to_sync(), it
could observe the node off-list while the rq lock is dropped here:

> @@ -371,11 +480,29 @@ void scx_discard_ecaps_to_sync(s32 cpu, struct scx_sched_pcpu *pcpu)
>  {
> -	scoped_guard (rq_lock_irqsave, cpu_rq(cpu))
> -		scx_process_sync_ecaps(cpu_rq(cpu));
> +	struct rq *rq = cpu_rq(cpu);
>  
> -	WARN_ON_ONCE(llist_on_list(&pcpu->ecaps_to_sync_node));
> +	while (true) {
> +		scoped_guard (rq_lock_irqsave, rq) {
> +			/*
> +			 * scx_process_sync_ecaps() takes the node off the list
> +			 * before it is done accessing @pcpu but does all of it
> +			 * under the rq lock. Off-list observed under the rq
> +			 * lock guarantees that the sync is complete.
> +			 */
> +			if (!llist_on_list(&pcpu->ecaps_to_sync_node))
> +				return;

If scx_discard_ecaps_to_sync() returns and frees pcpu while the rq lock
is dropped, the assignment to pcpu->reported_ecaps upon returning from
scx_flush_dispatch_buf() will write to freed memory.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706014058.439853-1-tj@kernel.org?part=22