Re: [PATCH 2/3] sched_ext: Track bits[] storage size in struct scx_cmask

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Uninitialized per-CPU cmask scratch buffer silently breaks CPU affinity tracking
- [Medium] Missing bounds validation in BPF helper `__cmask_init()` permits arena out-of-bounds access
--

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

sched_ext: Track bits[] storage size in struct scx_cmask

This patch adds tracking for the allocation size of bits[] in struct
scx_cmask to allow validation of active range reshapes. It splits the
bit-range API, introduces scx_cmask_clear() and scx_cmask_fill(), and
updates the BPF mirror functions.

> diff --git a/kernel/sched/ext_cid.c b/kernel/sched/ext_cid.c
> index bdd8ef8eae3dc..f50319c5c65e1 100644
> --- a/kernel/sched/ext_cid.c
> +++ b/kernel/sched/ext_cid.c

[ ... ]

> @@ -274,10 +316,9 @@ void scx_cpumask_to_cmask(const struct cpumask *src, struct scx_cmask *dst)
>   * Initialize @dst to cover the full cid space [0, num_possible_cpus()) and
>   * set the bit for each cid whose cpu is in @src.
> + * Clear @dst's active range and set the bit for each cid whose cpu is in
> + * @src and lies within that range. Out-of-range cids are silently ignored.
>   */
>  void scx_cpumask_to_cmask(const struct cpumask *src, struct scx_cmask *dst)
>  {
>  	s32 cpu;
>  
> -	scx_cmask_init(dst, 0, num_possible_cpus());
> +	scx_cmask_clear(dst);

Since dst (scx_set_cmask_scratch) is zero-initialized via __alloc_percpu(),
does it ever get its nr_cids explicitly initialized now that the call to
scx_cmask_init() has been removed?

If m->nr_cids remains 0, scx_cmask_clear() will return immediately. The
subsequent loop calling __scx_cmask_set() would then ignore all CPUs because
the bounds check in __scx_cmask_contains() fails for a 0-length range.

Could this result in BPF schedulers receiving an empty cmask in their
ops_cid.set_cmask() callback, breaking task affinity tracking?

>  	for_each_cpu(cpu, src) {
>  		s32 cid = __scx_cpu_to_cid(cpu);

[ ... ]

> diff --git a/tools/sched_ext/include/scx/cid.bpf.h b/tools/sched_ext/include/scx/cid.bpf.h
> index b9dcc14870d3a..211f4077f43ac 100644
> --- a/tools/sched_ext/include/scx/cid.bpf.h
> +++ b/tools/sched_ext/include/scx/cid.bpf.h
> @@ -44,20 +44,71 @@ static __always_inline u64 __arena *__cmask_word(const struct scx_cmask __arena
>  	return (u64 __arena *)&m->bits[cid / 64 - m->base / 64];
>  }
>  
> -static __always_inline void cmask_init(struct scx_cmask __arena *m, u32 base, u32 nr_cids)
> +/**
> + * __cmask_init - Initialize @m with explicit storage capacity
> + * @m: cmask to initialize
> + * @base: first cid of the active range
> + * @nr_cids: number of cids in the active range
> + * @alloc_cids: storage capacity in cids, at least @nr_cids
> + *
> + * Use when storage is sized larger than the initial active range. All of
> + * bits[] is zeroed.
> + */
> +static __always_inline void __cmask_init(struct scx_cmask __arena *m, u32 base,
> +					 u32 nr_cids, u32 alloc_cids)
>  {
> -	u32 nr_words = CMASK_NR_WORDS(nr_cids), i;
> +	u32 alloc_words = CMASK_NR_WORDS(alloc_cids), i;
>  
>  	m->base = base;
>  	m->nr_cids = nr_cids;

Should this helper validate that nr_cids does not exceed alloc_cids?

The C implementation (__scx_cmask_init) safely clamps nr_cids to alloc_cids.
Without a similar check here or a call to scx_bpf_error(), passing an
oversized nr_cids allows __cmask_contains() to pass bounds checks for bits
beyond alloc_words.

Could this allow BPF programs to bypass intended constraints and corrupt
their own arena structures during cmask_test() or __cmask_word() calls?

> +	m->alloc_words = alloc_words;
>  
>  	bpf_for(i, 0, CMASK_MAX_WORDS) {

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