Re: [PATCH sched_ext/for-7.3 16/32] sched_ext: Add per-shard cap delegation for sub-schedulers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Partial allocation failure in `scx_alloc_pshards()` leaves the `pshard` array exposed, causing a NULL pointer dereference if the BPF `ops.exit` hook reads its own caps.
--

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

sched_ext: Add per-shard cap delegation for sub-schedulers

This patch introduces per-shard capability delegation for sub-schedulers in
sched_ext. It adds the framework and locking around shards.

> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index 1e84f46201767..e7259623fa3c1 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c
[ ... ]
> @@ -733,6 +782,281 @@ __bpf_kfunc bool scx_bpf_sub_dispatch(u64 cgroup_id, const struct bpf_prog_aux *
>  				  true);
>  }
>  
[ ... ]
> +/**
> + * scx_bpf_sub_caps - Read self's or a direct child's cap cmasks
> + * @cgroup_id: 0 for self, or a direct child's cgroup id
> + * @caps: one or more SCX_CAP_* bits
> + * @out__ign: arena cmask to receive the union of @caps within its range
> + * @aux: implicit BPF argument
> + *
> + * Read the cap cmasks granted on each cid for self (@cgroup_id 0) or a direct
> + * child - the literal granted set. A sched can read only itself or a direct
> + * child.
> + *
> + * Return 0, -ENODEV if @cgroup_id names no direct child, or -EINVAL on bad
> + * inputs.
> + */
> +__bpf_kfunc s32 scx_bpf_sub_caps(u64 cgroup_id, u64 caps, struct scx_cmask *out__ign,
> +				 const struct bpf_prog_aux *aux)
> +{
[ ... ]
> +	/*
> +	 * The target's caps storage may not be set up yet (e.g. a self-read
> +	 * during ops.init_cids()).
> +	 */
> +	if (unlikely(!target->pshard)) {
> +		scx_error(sch, "scx_bpf_sub_caps() called before caps storage is initialized");
> +		return -ENODEV;
> +	}
> +
> +	ret = scx_cmask_ref_init(sch, out__ign, &ref);
> +	if (ret) {
> +		scx_error(sch, "invalid out (%d)", ret);
> +		return ret;
> +	}
> +
> +	for (si = ref.shard_first; si < ref.shard_end; si++) {
> +		const struct scx_cid_shard *shard = &scx_cid_shard_ranges[si];
> +		SCX_CMASK_DEFINE_SHARD(local_out, shard->base_cid, shard->nr_cids);
> +		u32 cap_bit;
> +
> +		scx_for_each_cap_bit(cap_bit, caps)
> +			scx_cmask_or(local_out, &target->pshard[si]->caps[cap_bit].cmask);

[Severity: High]
If alloc_pshard() fails inside scx_alloc_pshards(), will this leave
sch->pshard populated with some NULL elements?

kernel/sched/ext/sub.c:scx_alloc_pshards() {
...
	sch->pshard = kzalloc_objs(sch->pshard[0], scx_nr_cid_shards, GFP_KERNEL);
	if (!sch->pshard)
		return -ENOMEM;

	sch->nr_pshards = scx_nr_cid_shards;

	for (si = 0; si < scx_nr_cid_shards; si++) {
		sch->pshard[si] = alloc_pshard(sch, si, scx_shard_node[si]);
		if (!sch->pshard[si])
			return -ENOMEM;
	}
...
}

If this returns -ENOMEM, the err_disable path in scx_sub_enable_workfn() is
triggered, which eventually calls the scheduler's BPF ops.exit() hook.

If the BPF program then calls scx_bpf_sub_caps(0, ...) to read its own
capabilities, the earlier check for !target->pshard passes because the array
itself was successfully allocated.

Will this loop then unconditionally dereference target->pshard[si] and
trigger a NULL pointer dereference on the unallocated elements?

> +		scx_cmask_ref_copy(&ref, local_out);
> +	}
> +	return 0;
> +}

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