Re: [PATCH v2] sched_ext: Reject NMI calls to lock-taking kfuncs

[liwanwu]

Hi Tejun,

Thanks for the review. v3 folds in your three points (single
scx_kf_allowed_ctx() macro with no wrapper, one reject in
bpf_iter_scx_dsq_new(), unlikely(!sch) in scx_bpf_destroy_dsq()).

Continuing the audit that the sashiko bot kicked off, I swept every
kfunc exposed to BPF_PROG_TYPE_TRACING through the any / idle / cid
context-filter sets. I found a second class of problems beyond the
lock-taking set already covered. The first subclass I plan to fold
into v4 and want to make sure you agree with the scope.

Three "any"-category kfuncs read scx_locked_rq() on their success path
and treat a non-NULL return as "the rq lock is already held by me",
which a false positive under NMI turns into either a data race or a
wrong result:

 - scx_bpf_task_set_slice() (the sashiko bot's finding) races its
   direct p->scx.slice write with update_curr_scx()'s RMW of the same
   field.
 - scx_bpf_dsq_nr_queued() resolves %SCX_DSQ_LOCAL to
   (scx_locked_rq() ?: this_rq()) and can therefore report the wrong
   rq's length.
 - scx_bpf_locked_rq() hands the interrupted context's rq to the BPF
   program, which may then operate on it as if the lock were owned.

A second, different subclass I have NOT folded in, and would like your
guidance on.

scx_bpf_pick_idle_cpu_node() and scx_bpf_pick_any_cpu_node() pass the
caller's @node and @flags straight through to scx_pick_idle_cpu(). When
@node != NUMA_NO_NODE and SCX_PICK_IDLE_IN_NODE is not set and no idle
CPU is found in the starting node, the search falls through to
pick_idle_cpu_from_online_nodes() (kernel/sched/ext/idle.c:151), which
writes the per-CPU nodemask per_cpu_unvisited (idle.c:146) under only
preempt_disable(). preempt_disable() does not mask NMIs, so an NMI that
lands on a CPU whose interrupted context is mid-way through that loop
clobbers the scratch nodemask the interrupted context is still
iterating. Same "irq/preempt-only per-CPU state is not NMI-safe" premise
behind scx_kick_cpu()'s check, just a scratch buffer rather than a
lock or a list.

Note this is a wrong-result / lost-scratch race for the interrupted
struct_ops context, not a hard lockup, and it needs CONFIG_NUMA plus
per-node idle tracking to be reachable. The non-node
scx_bpf_pick_idle_cpu()/pick_any_cpu() force NUMA_NO_NODE and never
reach the scratch, and every cid-set kfunc is an RCU-read lookup, so the
hazard is confined to those two node-variants.

Should these two issue classes be folded into this series or are they 
out of scope for now? Looking forward to your suggestion.

Thanks,
Wanwu

在 2026/9/2 14:44, Tejun Heo 写道:
> Hello,
>
> On Wed, Sep 02, 2026 at 10:31:24AM +0800, Wanwu Li wrote:
> > -	if (sch)
> > -		destroy_dsq(sch, dsq_id);
> > +	if (!sch)
> > +		return;
>
> unlikely(!sch) like the other kfuncs.
>
> > @@ -9756,6 +9755,9 @@ __bpf_kfunc struct task_struct *bpf_iter_scx_dsq_next(struct bpf_iter_scx_dsq *i
> >   	if (!kit->dsq)
> >   		return NULL;
> >
> > +	if (!scx_kf_allowed_ctx(kit->dsq->sched))
> > +		return NULL;
>
> new(), next() and destroy() always run in the same context, so reject in
> bpf_iter_scx_dsq_new() and drop the checks from next() and destroy(). With
> kit->dsq left NULL, both are no-ops.
>
> > +static __always_inline bool __scx_kf_allowed_ctx(struct scx_sched *sch,
> > +						 const char *who)
>
> No need to wrap.
>
> Thanks.