Re: [PATCH sched_ext/for-7.3 29/32] sched_ext: Replay ecaps notifications suppressed by bypass

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Concurrent CPU hotplug, bypass toggle, or scx_bpf_sub_grant() crashes due to a NULL pshard dereference when a cpu-form sub-scheduler is linked but not yet rejected.
--

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

sched_ext: Replay ecaps notifications suppressed by bypass

scx_process_sync_ecaps() consumes ecaps syncs while the sched is bypassing
without delivering ops.sub_ecaps_updated(), leaving reported_ecaps stale.
This patch re-queues a sync for every (sched, cpu) with an undelivered delta
at the per-cpu bypass exit in scx_bypass(), next to the idle renotify
catch-up.

> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index 90caf76db8bf6..15edcf4f81ee2 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c
[ ... ]
> @@ -550,6 +550,41 @@ void scx_process_sync_ecaps(struct rq *rq, struct task_struct *prev)
>  		scx_schedule_reenq_local(rq, SCX_REENQ_CAP_REVOKE);
>  }
>  
> +/**
> + * scx_unbypass_replay_ecaps - Replay a bypass-suppressed ecaps notification
> + * @rq: rq of the cpu leaving bypass
> + * @sch: scheduler that just left bypass on @rq's cpu
> + *
[ ... ]
> + */
> +void scx_unbypass_replay_ecaps(struct rq *rq, struct scx_sched *sch)
> +{
> +	s32 cpu = cpu_of(rq);
> +	struct scx_sched_pcpu *pcpu = per_cpu_ptr(sch->pcpu, cpu);
> +	struct scx_pshard *ps;
> +	s32 cid;
> +
> +	lockdep_assert_rq_held(rq);
> +
> +	/* root holds every cap and never uses ecaps */
> +	if (!sch->level)
> +		return;
> +
> +	if (READ_ONCE(pcpu->ecaps) == pcpu->reported_ecaps)
> +		return;
> +
> +	cid = __scx_cpu_to_cid(cpu);
> +	ps = sch->pshard[scx_cid_to_shard[cid]];
> +
> +	guard(raw_spinlock)(&ps->lock);

[Severity: High]
This is a pre-existing issue and was not introduced by the patch under review,
but this new function also appears to be affected by it. Can this cause a NULL
pointer dereference if sch->pshard is NULL?

If a user attaches a cpu-form scheduler (where !sch->is_cid_type) as a
sub-scheduler, scx_alloc_pshards() returns 0 without allocating sch->pshard.
The scheduler is then added to the descendant tree via scx_link_sched().

Before it is rejected by scx_validate_ops() and unlinked, a concurrent
parent lifting bypass can trigger this new scx_unbypass_replay_ecaps()
path, unconditionally accessing sch->pshard.

This pre-existing issue also seems to affect scx_online_ecaps(),
scx_bpf_sub_grant(), and scx_bpf_sub_revoke() where pshard is accessed
unconditionally. Could this be triggered by a concurrent CPU hotplug or BPF call
during this window?

> +	queue_sync_ecaps(sch, cid);
> +}

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