Re: [PATCH v3 sched_ext/for-7.3 09/36] sched_ext: Make the kick machinery per-sched

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Early return in `kick_cpus_irq_workfn()` skips removing `pcpu->to_kick_node` from `sched_pcpus_to_kick` if `ksyncs_pcpu` is NULL, leading to a Use-After-Free when the `pcpu` structure is later freed.
--

--- Patch [9]: [PATCH v3 sched_ext/for-7.3 09/36] sched_ext: Make the kick machinery per-sched ---
commit f524c2556374db5c875f61c03449ff025ad14859
Author: Tejun Heo <tj@kernel.org>
sched_ext: Make the kick machinery per-sched

This commit moves the kick masks into the per-sched per-cpu scx_sched_pcpu
and has scx_kick_cpu() link the sched onto a per-cpu list, allowing preempt
kicks to be scoped to the requesting scheduler.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 187754d0c3c84..150e74129266b 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -4624,6 +4624,17 @@ static void scx_sched_free_rcu_work(struct work_struct *work)
>  		 */
>  		WARN_ON_ONCE(!list_empty(&pcpu->deferred_reenq_local.node));
>  
> +		/*
> +		 * Bypass blocks new kicks. Flush the kick irq_work so this
> +		 * pcpu's to_kick_node is off the list before it is freed.
> +		 */
> +		irq_work_sync(&cpu_rq(cpu)->scx.kick_cpus_irq_work);
> +		WARN_ON_ONCE(!list_empty(&pcpu->to_kick_node));
> +		free_cpumask_var(pcpu->cpus_to_kick);

[ ... ]

> @@ -7535,15 +7566,24 @@ static void kick_cpus_irq_workfn(struct irq_work *irq_work)
>  
>  	ksyncs = rcu_dereference_bh(ksyncs_pcpu)->syncs;
>  
> -	for_each_cpu(cpu, this_scx->cpus_to_kick) {
> -		should_wait |= kick_one_cpu(cpu, this_rq, ksyncs);
> -		cpumask_clear_cpu(cpu, this_scx->cpus_to_kick);
> -		cpumask_clear_cpu(cpu, this_scx->cpus_to_kick_if_idle);
> -	}
> +	/*
> +	 * Walk scheds with pending kicks on this cpu. scx_kick_cpu() adds to
> +	 * the list under local_irq_save() and only this irq_work consumes it.
> +	 * A plain list without locking is sufficient.
> +	 */
> +	list_for_each_entry_safe(pcpu, tmp, &this_scx->sched_pcpus_to_kick, to_kick_node) {
> +		list_del_init(&pcpu->to_kick_node);

[Severity: High]
Does this code risk a use-after-free on the scheduler disable path?

If a kick races with scx_root_disable(), free_kick_syncs() can set the
per-CPU scx_kick_syncs pointers to NULL. If kick_cpus_irq_workfn() executes
after this, it hits an early return just before the code above:

kernel/sched/ext/ext.c:kick_cpus_irq_workfn() {
    ...
	if (unlikely(!ksyncs_pcpu))
		return;
    ...
}

Because this early return bypasses the list_for_each_entry_safe() loop,
pcpu->to_kick_node remains on the rq->scx.sched_pcpus_to_kick list.

When scx_sched_free_rcu_work() later runs, it syncs the irq_work, triggers
the WARN_ON_ONCE(!list_empty(&pcpu->to_kick_node)) check added in this patch,
and frees the pcpu structure via free_percpu() while it is still linked.

When a new scheduler is loaded and calls scx_kick_cpu(), will it corrupt the
list by calling list_add_tail() on this freed memory?

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