Re: [PATCH v3 sched_ext/for-7.3 28/36] 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] A TOCTOU race in `scx_bpf_task_set_slice()` allows a BPF scheduler to corrupt a task's CPU slice and bypass capability revocation.
--

--- Patch [28]: [PATCH v3 sched_ext/for-7.3 28/36] sched_ext: Tie cpu occupancy to SCX_CAP_BASE through the task slice ---
commit 8aeba2787a919bb08e865800095c6140014e82bf
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 SCX_CAP_BASE through the task slice to
ensure sub-schedulers respect revocable capabilities. It introduces out-of-band
slice requests when the runqueue lock is not held.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index dd605acf0f572..1428630d2dd4c 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c

[ ... ]

> @@ -8634,20 +8786,45 @@ __bpf_kfunc_start_defs();
>   * @slice: time slice to set in nsecs
>   * @aux: implicit BPF argument to access bpf_prog_aux hidden from BPF progs
>   *
> - * Set @p's time slice to @slice. Returns %true on success, %false if the
> - * calling scheduler doesn't have authority over @p.
> + * Set @p's time slice. @p must be on the calling scheduler. The value is
> + * applied whether or not the caller holds @p's rq lock - see the slice write
> + * rules above for the ownership model.
> + *
> + * Raising the slice is honored only while the scheduler holds %SCX_CAP_BASE on
> + * @p's cpu, otherwise it is counted in %SCX_EV_SLICE_DENIED. Shortening is
> + * always allowed. On the stashed path the slice is packed into an atomic64_t
> + * with the scheduler id and a flag bit, so a slice too large to fit is clamped
> + * and counted in %SCX_EV_SLICE_CLAMPED. %SCX_SLICE_INF is preserved.
> + *
> + * Return %true on success, %false if @p is not on the calling scheduler.
>   */
>  __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);
> +	/*
> +	 * Directly write only when we are the owner of @p. See the slice write
> +	 * rules above.
> +	 */
> +	if (scx_locked_rq() != rq || !task_on_rq_queued(p)) {

[Severity: High]
Can a Time-of-Check to Time-of-Use (TOCTOU) race happen here with remote
wakeups?

If a BPF program on CPU 0 calls scx_bpf_task_set_slice() on a sleeping task
that last ran on CPU 0 while CPU 0 holds its runqueue lock:

1. rq = task_rq(p) locklessly reads p->cpu as 0, caching CPU 0's runqueue.

2. Concurrently, CPU 1 wakes up the task, updates p->cpu = 1, takes CPU 1's
   runqueue lock, and sets p->on_rq = TASK_ON_RQ_QUEUED.

3. CPU 0 checks scx_locked_rq() != rq (false, as both are CPU 0's runqueue)
   and !task_on_rq_queued(p) (false, since CPU 1 just enqueued it).

4. CPU 0 incorrectly believes it safely holds the lock for the task's current
   runqueue, and proceeds to the direct write path.

> +		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]
If the race described above occurs, won't cpu_of(rq) check capabilities
against the stale CPU 0 instead of CPU 1?

Furthermore, wouldn't the subsequent set_task_slice() call execute
concurrently while the task is actually running on CPU 1, potentially
corrupting the time slice and bypassing capability revocation?

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

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