Re: [PATCH sched_ext/for-7.3 25/32] sched_ext: Tie cpu occupancy to SCX_CAP_BASE through the task slice

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scx_bpf_task_set_slice() assumes holding task_rq(p) implies exclusive access, but blocked tasks can migrate locklessly, allowing capability bypass and data races.
--

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

sched_ext: Tie cpu occupancy to SCX_CAP_BASE through the task slice

This commit ties CPU occupancy to the SCX_CAP_BASE capability, ensuring
that a task's slice can only be extended if its scheduler holds baseline
CPU access. It updates scx_bpf_task_set_slice() to apply the slice
synchronously when the rq lock is held, or out-of-band otherwise.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c

[ ... ]

> @@ -8626,20 +8761,45 @@ __bpf_kfunc_start_defs();
>  __bpf_kfunc bool scx_bpf_task_set_slice(struct task_struct *p, u64 slice,
>  					const struct bpf_prog_aux *aux)
>  {
>  	struct scx_sched *sch;
> +	struct rq *rq = task_rq(p);
>  
>  	guard(rcu)();
>  	sch = scx_prog_sched(aux);
>  	if (unlikely(!sch || !scx_task_on_sched(sch, p)))
>  		return false;
>  
> -	set_task_slice(p, slice);
> +	/*
> +	 * Out of band: stash and apply under the rq lock at the next drain,
> +	 * where it is re-validated against @p's current owner.
> +	 */
> +	if (scx_locked_rq() != rq) {
> +		set_task_slice_oob(sch, p, slice);
> +		return true;
> +	}
> +
> +	/* under the rq lock: apply now, extensions gated on baseline access */
> +	if (slice > p->scx.slice &&
> +	    unlikely(scx_missing_caps(sch, cpu_of(rq), SCX_CAP_BASE)))

[Severity: High]
Does this direct write path allow a capability bypass and a data race for
blocked tasks?

If a BPF program on CPU A holds the rq_A lock and calls
scx_bpf_task_set_slice() on a blocked task 'p' that last ran on CPU A,
task_rq(p) will return rq_A. This matches scx_locked_rq(), allowing the code
to skip the out-of-band path.

Could try_to_wake_up() concurrently migrate 'p' to CPU B in this scenario?
Since try_to_wake_up() only needs to hold pi_lock (not the rq_A lock) while
changing p->cpu, the BPF program might directly write to p->scx.slice at the
same time update_curr_scx() is running on CPU B.

Additionally, since the capability check above uses the old CPU (rq_A) rather
than the new CPU the task is migrating to, would this allow a sub-scheduler
to bypass the SCX_CAP_BASE revocation check for the new CPU?

> +		__scx_add_event(sch, SCX_EV_SLICE_DENIED, 1);
> +	else
> +		set_task_slice(p, slice);
> +
>  	return true;
>  }

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