Re: [PATCH] sched_ext: Reset cpuperf_target when a sub-scheduler loses SCX_CAP_PERF or dies

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] sched_ext: UAF on scx_cid_to_cpu_tbl in scx_sub_reset_cpuperf() on teardown
- [Medium] sched_ext: TOCTOU race in scx_cpuperf_queue_reset() triggers offline CPU WARN
- [Medium] sched_ext: offline CPUs retain stale cpuperf_target causing frequency pinning
--

commit 8df3a4e837553ee14c5a70ecea2b4bec546c3881
Author: Tao Cui <cuitao@kylinos.cn>

    sched_ext: Reset cpuperf_target when a sub-scheduler loses SCX_CAP_PERF or dies

This commit introduces a mechanism to reset rq->scx.cpuperf_target to the
neutral SCX_CPUPERF_ONE base when a sub-scheduler that wrote targets loses
its capabilities or dies. It queues per-CPU irq_works to perform the reset
under the target CPU's rq lock, and skips offline CPUs.

> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index 0554448835bd0..38e48e4648da5 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c

[ ... ]

> @@ -28,6 +28,57 @@
>   */
>  DEFINE_STATIC_KEY_FALSE(__scx_has_subs);
>  
> +/*
> + * Reset rq->scx.cpuperf_target back to the neutral SCX_CPUPERF_ONE base
> + * when a sub-scheduler that wrote targets loses SCX_CAP_PERF or dies. The

[ ... ]

> +/*
> + * Queue a reset of @cpu's cpuperf target. Idempotent: the work is always a
> + * plain restore of the neutral base, so concurrent queues collapse into one
> + * run; CPUs that never had a sub-scheduler target no-op.
> + */
> +static void scx_cpuperf_queue_reset(s32 cpu)
> +{
> +	if (cpu < 0 || cpu >= nr_cpu_ids || !cpu_online(cpu))
> +		return;

[Severity: Medium]
If the CPU is already offline when this function is called, the early return
skips resetting the target. Does this leave the offline CPU with a stale
cpuperf_target in its rq, permanently pinning the CPU to a low frequency
when it comes back online?

> +	irq_work_queue_on(&per_cpu(scx_cpuperf_reset_iw, cpu), cpu);

[Severity: Medium]
Since cpu_online() is checked without preemption disabled or any hotplug
locks held, is there a TOCTOU race window here? If a concurrent CPU offline
operation clears cpu_online_mask immediately after this check, wouldn't
calling irq_work_queue_on() trigger a WARN_ON_ONCE(cpu_is_offline(cpu))?

> +}

[ ... ]

> +static void scx_sub_reset_cpuperf(struct scx_sched *sch)
> +{
> +	s32 si, cid;
> +
> +	if (!READ_ONCE(sch->pshard))
> +		return;
> +
> +	for (si = 0; si < sch->nr_pshards; si++) {
> +		struct scx_cmask *cm = &sch->pshard[si]->caps[__SCX_CAP_PERF].cmask;
> +
> +		scx_cmask_for_each_cid(cid, cm)
> +			scx_cpuperf_queue_reset(__scx_cid_to_cpu(cid));

[Severity: High]
This relies on __scx_cid_to_cpu(), which locklessly dereferences
scx_cid_to_cpu_tbl. Since scx_sub_reset_cpuperf() is called after dropping
scx_enable_mutex, if scx_root_disable() concurrently nulls and frees the
tables, wouldn't this cause a UAF or NULL pointer dereference?

> +	}
> +}

[ ... ]

> @@ -1631,6 +1715,13 @@ void scx_sub_disable(struct scx_sched *sch)
>  
>  	mutex_unlock(&scx_enable_mutex);
>  
> +	/*
> +	 * @sch can no longer receive grants or writes: queue a cpuperf reset
> +	 * for every CPU it held SCX_CAP_PERF on so schedutil doesn't keep
> +	 * consuming the targets it left behind.
> +	 */
> +	scx_sub_reset_cpuperf(sch);

[Severity: Medium]
This calls scx_sub_reset_cpuperf() with preemption enabled and without
any hotplug protection, exposing the cpu_online() TOCTOU race mentioned
above.

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