Re: [PATCHSET SLOP RFC 6/6] sched_ext: Convert scx_bpf_cid_override() to take an arena pointer

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing READ_ONCE() when reading from shared arena memory allows a TOCTOU race leading to out-of-bounds writes.
--

--- Patch [6]: [PATCHSET SLOP RFC 6/6] sched_ext: Convert scx_bpf_cid_override() to take an arena pointer ---
commit 0bba9b04cbf24d912ae66699e643f62ba90a630b
Author: Tejun Heo <[email protected]>

sched_ext: Convert scx_bpf_cid_override() to take an arena pointer

This commit changes scx_bpf_cid_override() to take an arena pointer for the
cpu_to_cid array instead of a verifier-visible buffer. It validates the
entry count against nr_cpu_ids, relying on the arena's guard region to
prevent out-of-bounds memory accesses.

> diff --git a/kernel/sched/ext/cid.c b/kernel/sched/ext/cid.c
> index af83084ec740a..872f699cb1acc 100644
> --- a/kernel/sched/ext/cid.c
> +++ b/kernel/sched/ext/cid.c
[ ... ]
> @@ -314,14 +314,24 @@ __bpf_kfunc void scx_bpf_cid_override(const s32 *cpu_to_cid, u32 cpu_to_cid__sz,
[ ... ]
> +	/*
> +	 * @cpu_to_cid__arena arrives rebased to the arena kernel mapping.
> +	 * nr_cpu_ids * sizeof(s32) stays within the guard region covered by
> +	 * arena fault recovery, so no explicit bounds check is needed.
> +	 */
>  	for_each_possible_cpu(cpu) {
> -		s32 c = cpu_to_cid[cpu];
> +		s32 c = cpu_to_cid__arena[cpu];
>  
>  		if (!cid_valid(sch, c))
>  			return;

[Severity: High]
This is a pre-existing issue, but since the array resides in shared
userspace-mapped memory (and now an mmaped arena), should READ_ONCE() be
used when reading cpu_to_cid__arena[cpu]?

Without READ_ONCE(), the compiler could potentially reload the value of c
from memory after the bounds check in cid_valid(sch, c). If userspace
concurrently modifies the arena memory between the check and its subsequent
use as an array index:

    if (cpumask_test_and_set_cpu(c, seen)) {
        scx_error(sch, "cid %d assigned to multiple cpus", c);
        return;
    }
    scx_cpu_to_cid_tbl[cpu] = c;
    scx_cid_to_cpu_tbl[c] = cpu;

Could this Time-of-Check to Time-of-Use (TOCTOU) race result in an
out-of-bounds write to scx_cid_to_cpu_tbl[c]?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6