Re: [PATCH v2 sched_ext/for-7.3 20/36] sched_ext: Add coalescing sub_caps_updated() notifier for sub-schedulers

[sashiko-bot@kernel.org]

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

New issues:
- [High] Missing private stack request for recursive BPF callback `sub_caps_updated` leads to kernel stack exhaustion and local DoS.

Pre-existing issues:
- [High] Missing validation for BPF arena presence in sub-scheduler capabilities kfuncs leading to NULL pointer dereference and page faults.
--

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

sched_ext: Add coalescing sub_caps_updated() notifier for sub-schedulers

Wire up ops_cid.sub_caps_updated() to notify sub-scheds of cap changes.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index b00acbc3e16ae..1fa656e2bbb43 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c

[ ... ]

> @@ -7501,6 +7503,7 @@ static struct sched_ext_ops_cid __bpf_ops_sched_ext_ops_cid = {
>  #endif
>  	.sub_attach		= sched_ext_ops__sub_attach,
>  	.sub_detach		= sched_ext_ops__sub_detach,
> +	.sub_caps_updated	= sched_ext_ops__sub_caps_updated,

[Severity: High]
Does this new callback need to request a private stack?

The commit message notes that a child receiving this notification can call
grant/revoke on its own children, which cascades recursively down its
subtree.

Because sub_caps_updated was not added to the switch statement in
kernel/sched/ext/ext.c:bpf_scx_check_member() to enable
prog->aux->priv_stack_requested = true, this recursive cycle executes on
the standard kernel stack.

With scx_bpf_sub_grant() allocating multiple SCX_CMASK_DEFINE_SHARD
structures (consuming roughly 320+ bytes per frame) plus the BPF program
stack overhead, can this unbounded synchronous C-stack recursion exhaust
the kernel stack for moderately deep cgroup hierarchies?

>  	.cid_online		= sched_ext_ops__cpu_online,
>  	.cid_offline		= sched_ext_ops__cpu_offline,

[ ... ]

> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index 36803b6df6318..5bd3b8a1da6b8 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c

[ ... ]

> @@ -879,6 +990,7 @@ __bpf_kfunc s32 scx_bpf_sub_grant(u64 cgroup_id, u64 caps,
>  		SCX_CMASK_DEFINE_SHARD(slice, 0, SCX_CID_SHARD_MAX_CPUS);
>  		struct scx_pshard *pps = parent->pshard[si];
>  		struct scx_pshard *cps = child->pshard[si];
> +		u64 granted_caps = 0;
>  		u32 cap_bit;
>  
>  		scx_cmask_ref_shard(&ref, si, slice);

[Severity: High]
This is a pre-existing issue, but can these pshard pointers be NULL if
the BPF program does not define an arena?

In kernel/sched/ext/sub.c:scx_alloc_pshards(), if sch->arena_pool is NULL,
the pshard arrays are never allocated. Later, when scx_bpf_sub_grant() is
called, sub_cap_preamble() only verifies scx_is_cid_type(), allowing
execution to proceed even without an arena.

This leads to two potential issues:

1. When scx_cmask_ref_init() is called prior to this loop, passing a NULL
pointer because arena_kern_base is 0 causes a NULL pointer dereference at
READ_ONCE(kern_src->base).

2. Within this loop, parent->pshard[si] and child->pshard[si] are
unconditionally dereferenced, which would cause a NULL pointer dereference
since the pshard array is NULL.

Should sub_cap_preamble() or scx_alloc_pshards() ensure the arena is
fully initialized before permitting these sub-scheduler capability kfuncs
to execute?

> @@ -886,6 +998,9 @@ __bpf_kfunc s32 scx_bpf_sub_grant(u64 cgroup_id, u64 caps,
>  			continue;
>  
>  		SCX_CMASK_DEFINE_SHARD(granted_cids, slice->base, slice->nr_cids);
> +		SCX_CMASK_DEFINE_SHARD(changed_cids, slice->base, slice->nr_cids);
> +		SCX_CMASK_DEFINE_SHARD(delta, slice->base, slice->nr_cids);
> +
>  		scx_cmask_copy(granted_cids, slice);

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