Re: [PATCH sched_ext/for-7.3 10/32] sched_ext: Add shard boundaries to scx_bpf_cid_override()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Concurrent modification of userspace/BPF-accessible arrays `cpu_to_cid` and `shard_start` bypasses validation and leads to out-of-bounds writes.
- [High] Changing the signature of `scx_bpf_cid_override` without renaming the kfunc breaks backward compatibility and `compat.bpf.h` versioning, unconditionally causing load failures on older kernels.
--

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

sched_ext: Add shard boundaries to scx_bpf_cid_override()

This commit extends scx_bpf_cid_override() to accept an explicit shard layout
alongside the cpu-to-cid mapping. It updates the validation and assignment
logic to handle shard assignments and modifies the tools/sched_ext compat
wrapper and qmap tests to supply the new arguments.

> diff --git a/kernel/sched/ext/cid.c b/kernel/sched/ext/cid.c
> index 9d75b9311978c..bd0467e8a8d29 100644
> --- a/kernel/sched/ext/cid.c
> +++ b/kernel/sched/ext/cid.c
[ ... ]
> @@ -442,13 +511,56 @@ __bpf_kfunc void scx_bpf_cid_override(const s32 *cpu_to_cid, u32 cpu_to_cid__sz,
>  			scx_error(sch, "cid %d assigned to multiple cpus", c);
>  			return;
>  		}
> +	}
> +
> +	for_each_possible_cpu(cpu) {
> +		s32 c = cpu_to_cid[cpu];
> +
>  		scx_cpu_to_cid_tbl[cpu] = c;
>  		scx_cid_to_cpu_tbl[c] = cpu;
>  	}

[Severity: Critical]
Since cpu_to_cid and shard_start point to BPF map memory which can be mmap'ed
and modified concurrently by userspace, can this cause a TOCTOU race leading to
out-of-bounds writes?

If a buggy or malicious userspace program modifies cpu_to_cid[cpu] after the
first validation loop finishes but before this second loop reads it, could 'c'
exceed num_possible_cpus() and cause scx_cid_to_cpu_tbl[c] = cpu to write out
of bounds?

The shard_start array also appears to be read repeatedly across multiple loops
after validation. Would it be safer to kmemdup() these arrays before
validation to ensure consistent state?

[ ... ]
> diff --git a/tools/sched_ext/include/scx/compat.bpf.h b/tools/sched_ext/include/scx/compat.bpf.h
> index 133058578668d..cf469d5ff9ca0 100644
> --- a/tools/sched_ext/include/scx/compat.bpf.h
> +++ b/tools/sched_ext/include/scx/compat.bpf.h
[ ... ]
> @@ -122,15 +122,18 @@ static inline bool scx_bpf_sub_dispatch(u64 cgroup_id)
>  }
>  
>  /*
> - * v7.2: scx_bpf_cid_override() for explicit cpu->cid mapping. Ignore if
> + * v7.3: scx_bpf_cid_override() for explicit cid and shard mapping. Ignore if
>   * missing.
>   */
> -void scx_bpf_cid_override___compat(const s32 *cpu_to_cid, u32 cpu_to_cid__sz) __ksym __weak;
> +void scx_bpf_cid_override___compat(const s32 *cpu_to_cid, u32 cpu_to_cid__sz,
> +				    const s32 *shard_start, u32 shard_start__sz) __ksym __weak;
>  
> -static inline void scx_bpf_cid_override(const s32 *cpu_to_cid, u32 cpu_to_cid__sz)
> +static inline void scx_bpf_cid_override(const s32 *cpu_to_cid, u32 cpu_to_cid__sz,
> +					 const s32 *shard_start, u32 shard_start__sz)
>  {
>  	if (bpf_ksym_exists(scx_bpf_cid_override___compat))
> -		return scx_bpf_cid_override___compat(cpu_to_cid, cpu_to_cid__sz);
> +		scx_bpf_cid_override___compat(cpu_to_cid, cpu_to_cid__sz,
> +					      shard_start, shard_start__sz);
>  }

[Severity: High]
Does changing the signature of scx_bpf_cid_override() without renaming the
kfunc break backward compatibility for BPF schedulers loaded on older kernels?

On older kernels that only have the 2-argument version of the kfunc,
bpf_ksym_exists() will still match by name and return true. However, because
the compatibility wrapper now makes a 4-argument call, the BPF verifier will
reject the program due to a BTF signature mismatch.

Should the kfunc be renamed (e.g., scx_bpf_cid_override_v2) so
bpf_ksym_exists() correctly returns false on kernels that don't support the
new signature?

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