Re: [PATCH v4 sched_ext/for-7.3 13/40] sched_ext: Add shard boundaries to scx_bpf_cid_override()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unbounded allocation sizes for kmemdup before validation trigger kernel warnings and allocation failures.
--

--- Patch [13]: [PATCH v4 sched_ext/for-7.3 13/40] sched_ext: Add shard boundaries to scx_bpf_cid_override() ---
commit d662162dc44b1de0465d793b6ec2418996af7aef
Author: Tejun Heo <[email protected]>

sched_ext: Add shard boundaries to scx_bpf_cid_override()

This commit extends scx_bpf_cid_override() to accept explicit shard boundaries.
It also snapshots the caller's arrays before validating them to prevent
concurrent mutations from altering the data between validation and use.

> diff --git a/kernel/sched/ext/cid.c b/kernel/sched/ext/cid.c
> index f3c54f4223b43..6a22bead4c601 100644
> --- a/kernel/sched/ext/cid.c
> +++ b/kernel/sched/ext/cid.c
[ ... ]
> @@ -392,29 +392,66 @@ void scx_cpumask_to_cmask(const struct cpumask *src, struct scx_cmask *dst)
[ ... ]
>  	/*
> -	/* GFP_KERNEL alloc must happen before the rcu read section */
> +	 * GFP_KERNEL allocs must happen before the rcu read section. Snapshot
> +	 * the BPF-supplied arrays so a concurrent map mutation can't change
> +	 * them between validation and use.
> +	 */
>  	alloced = zalloc_cpumask_var(&seen, GFP_KERNEL);
> +	node_counts = kcalloc(nr_node_ids, sizeof(*node_counts), GFP_KERNEL);
> +	cpu_to_cid = kmemdup(cpu_to_cid_src, cpu_to_cid_src__sz, GFP_KERNEL);
> +	shard_start = kmemdup(shard_start_src, shard_start_src__sz, GFP_KERNEL);

[Severity: Medium]
Can a BPF program trigger a page allocator warning here by passing an
unbounded size for cpu_to_cid_src__sz or shard_start_src__sz?

The sizes are passed directly to kmemdup() before the length validation checks
occur below. Would it be safer to validate the expected array sizes before
attempting to allocate memory for the snapshots?

>  
>  	guard(rcu)();
>  
> @@ -422,17 +459,57 @@ __bpf_kfunc void scx_bpf_cid_override(const s32 *cpu_to_cid, u32 cpu_to_cid__sz,
>  	if (unlikely(!sch))
>  		return;
>  
> -	if (!alloced) {
> -		scx_error(sch, "scx_bpf_cid_override: failed to allocate cpumask");
> +	if (!alloced || !node_counts || !cpu_to_cid || !shard_start) {
> +		scx_error(sch, "scx_bpf_cid_override: allocation failed");
>  		return;
>  	}
>  
> -	if (cpu_to_cid__sz != nr_cpu_ids * sizeof(s32)) {
> -		scx_error(sch, "scx_bpf_cid_override: expected %zu bytes, got %u",
> -			  nr_cpu_ids * sizeof(s32), cpu_to_cid__sz);
> +	if (cpu_to_cid_src__sz != nr_cpu_ids * sizeof(s32)) {
> +		scx_error(sch, "scx_bpf_cid_override: cpu_to_cid expected %zu bytes, got %u",
> +			  nr_cpu_ids * sizeof(s32), cpu_to_cid_src__sz);
> +		return;
> +	}
> +
> +	if (!shard_start_src__sz || shard_start_src__sz % sizeof(s32)) {
> +		scx_error(sch, "scx_bpf_cid_override: invalid shard_start size %u",
> +			  shard_start_src__sz);
> +		return;
> +	}

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