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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] scx_bpf_task_set_slice() incorrectly assumes exclusive access based on scx_locked_rq(), causing data corruption in NMI context.
--

commit 2ad0cc408fc1de5da96ae5cb10c0e31d7633a82a
Author: Wanwu Li <liwanwu@kylinos.cn>

    sched_ext: Reject NMI calls to lock-taking kfuncs

This commit introduces the scx_kf_allowed_ctx() helper to reject NMI calls to
lock-taking kfuncs in sched_ext. It applies this check to several kfuncs that
take raw spinlocks to prevent CPU hard-lockups when a tracing program runs in an
NMI context.

Link: https://lore.kernel.org/r/20260901095652.1009104-1-liwanwu@kylinos.cn

> diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h
> index 27bbf5e04d900..ca8ebb11f03ca 100644
> --- a/kernel/sched/ext/internal.h
> +++ b/kernel/sched/ext/internal.h
> @@ -2091,6 +2091,22 @@ extern struct scx_sched *scx_enabling_sub_sched;
>  #define scx_error(sch, fmt, args...)						\
>  	scx_exit((sch), SCX_EXIT_ERROR, 0, fmt, ##args)
>  
> +/*
> + * Tracing progs can call kfuncs from NMI. Kfuncs that take scheduler locks or
> + * touch the kick lists, which are only protected by irq masking, can't run
> + * there, so abort the scheduler instead. scx_error() is NMI-safe.
> + */
> +#define scx_kf_allowed_ctx(sch)						\
> +({										\
> +	bool __allowed = true;							\
> +										\
> +	if (unlikely(in_nmi())) {						\
> +		scx_error((sch), "%s called from NMI", __func__);		\
> +		__allowed = false;						\
> +	}									\
> +	__allowed;								\
> +})
> +

[Severity: High]
This is a pre-existing issue, but I noticed an NMI-safety gap in
scx_bpf_task_set_slice() that seems to have been missed in this patch's
auditing of kfuncs exposed to BPF_PROG_TYPE_TRACING.

When a BPF tracing program executes in an NMI context (e.g., via a perf event)
and calls scx_bpf_task_set_slice(), if the NMI interrupted a context that
already holds the target task's runqueue lock (such as update_curr_scx()):

kernel/sched/ext/ext.c:scx_bpf_task_set_slice() {
    ...
	locked_rq = scx_locked_rq();
	if (!locked_rq ||
	    (READ_ONCE(p->scx.runnable_cpu) != cpu_of(locked_rq) &&
	     !task_current(locked_rq, p))) {
		set_task_slice_oob(sch, p, slice);
		return true;
	}
    ...
}

Will scx_locked_rq() incorrectly return the interrupted context's locked runqueue
rather than indicating the lock is not held by the NMI itself?

If so, won't this cause the NMI to bypass the NMI-safe out-of-band request path
(set_task_slice_oob()) and directly write to p->scx.slice?

Could this write race with the interrupted context, which may simultaneously
perform a non-atomic read-modify-write on the same variable, causing data
corruption or a lost update? 

Should scx_bpf_task_set_slice() also use scx_kf_allowed_ctx() to reject NMI
calls?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902093611.52651-1-liwanwu@kylinos.cn?part=1