* [PATCH] sched_ext: Reject NMI calls to lock-taking kfuncs
@ 2026-09-01 9:56 Wanwu Li
2026-09-01 19:51 ` Andrea Righi
2026-09-01 21:16 ` Tejun Heo
0 siblings, 2 replies; 13+ messages in thread
From: Wanwu Li @ 2026-09-01 9:56 UTC (permalink / raw)
To: Tejun Heo, David Vernet, Andrea Righi, Changwoo Min
Cc: liwanwu, sched-ext, linux-kernel
commit e06ece82d7b0 ("sched_ext: Report NMI kicks with scx_error()") made
scx_bpf_kick_cpu() reject NMI calls, and its cover letter describes the
reachability: sched_ext kfuncs in the "any" category "are callable from
tracing progs that can attach to functions running in NMI", and an unlucky
call from there "could deadlock the machine". The fix in that series made
the error/exit path lock-free so scx_error() is safe to call from NMI. That
closes the *error* path of every kfunc, but not a kfunc's own
business-logic lock acquisition on its success path.
The remaining lock-taking kfuncs that scx_kfunc_context_filter() exposes to
BPF_PROG_TYPE_TRACING have the same hazard: if an NMI lands on a CPU whose
interrupted context already holds the lock, the kfunc's raw spinlock
acquisition spins forever and hard-locks the CPU:
- scx_bpf_destroy_dsq() -> dsq->lock
- scx_bpf_dsq_reenq() -> rq's deferred_reenq_lock
- scx_bpf_cpuperf_set() / scx_bpf_cidperf_set() -> rq->lock
- scx_bpf_sub_grant() / scx_bpf_sub_revoke() -> pshard lock
(via the shared sub_cap_preamble())
- bpf_iter_scx_dsq_next() / bpf_iter_scx_dsq_destroy() -> dsq->lock
(bpf_iter_scx_dsq_new() is lockless and needs no guard)
As things stand, there is no scenario for reenqueueing, iterating a DSQ,
setting a performance target or granting sub-caps from NMI. The guards
defend against a buggy or malicious BPF program turning an "any"-category
kfunc into a machine-wide hard-lockup through the door that
scx_kfunc_context_filter() already opens. This matches the intent of
scx_bpf_kick_cpu()'s NMI check, which the commit cited above added not to
enable an NMI use case but to surface such a bug as a clean abort.
Route all of them through a new scx_kfunc_nmi_safe() helper and reuse
scx_bpf_kick_cpu()'s existing in_nmi() check - now shared with its cid
equivalent scx_bpf_kick_cid() through scx_kick_cpu() - so the rule is
stated once and the coverage is auditable from one place. scx_error() is
already NMI-safe (commit f883dbb64ca5 ("sched_ext: Make exit claiming
lock-free")), so the reject-abort cannot deadlock the lock acquisition.
Read-only members of the reachable sets (dsq_peek, dsq_nr_queued,
cpuperf_cur/cap, sub_caps, the idle cpumask helpers and the cid lookups)
take no scheduler lock on the path a tracing program reaches them, and were
audited to that effect; they are correctly left unguarded. The select_cpu
kfuncs do take pi_lock, but scx_kfunc_context_filter() only exposes the
any/idle/cid sets to BPF_PROG_TYPE_TRACING, and struct_ops run in task
context, so no lock-taking path here is reachable from NMI.
Signed-off-by: Wanwu Li <liwanwu@kylinos.cn>
---
kernel/sched/ext/ext.c | 27 ++++++++++++++++++++-------
kernel/sched/ext/internal.h | 22 ++++++++++++++++++++++
kernel/sched/ext/sub.c | 3 +++
3 files changed, 45 insertions(+), 7 deletions(-)
diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 10af28a9f2c0..a0b886975e1d 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -5108,6 +5108,9 @@ static void destroy_dsq(struct scx_sched *sch, u64 dsq_id)
struct scx_dispatch_q *dsq;
unsigned long flags;
+ if (!scx_kfunc_nmi_safe("scx_bpf_destroy_dsq()", sch))
+ return;
+
rcu_read_lock();
dsq = find_user_dsq(sch, dsq_id);
@@ -9518,14 +9521,8 @@ void scx_kick_cpu(struct scx_sched *sch, s32 cpu, u64 flags)
struct rq *this_rq;
unsigned long irq_flags;
- /*
- * The per-cpu kick list is guarded only by local_irq_save(), which does
- * not mask NMIs, so kicking from NMI could corrupt it and is unsupported.
- */
- if (unlikely(in_nmi())) {
- scx_error(sch, "scx_bpf_kick_cpu() called from NMI");
+ if (!scx_kfunc_nmi_safe("scx_bpf_kick_cpu()", sch))
return;
- }
local_irq_save(irq_flags);
@@ -9756,6 +9753,9 @@ __bpf_kfunc struct task_struct *bpf_iter_scx_dsq_next(struct bpf_iter_scx_dsq *i
if (!kit->dsq)
return NULL;
+ if (!scx_kfunc_nmi_safe(__func__, kit->dsq->sched))
+ return NULL;
+
guard(raw_spinlock_irqsave)(&kit->dsq->lock);
return nldsq_cursor_next_task(&kit->cursor, kit->dsq);
@@ -9777,6 +9777,9 @@ __bpf_kfunc void bpf_iter_scx_dsq_destroy(struct bpf_iter_scx_dsq *it)
if (!list_empty(&kit->cursor.node)) {
unsigned long flags;
+ if (!scx_kfunc_nmi_safe(__func__, kit->dsq->sched))
+ return;
+
raw_spin_lock_irqsave(&kit->dsq->lock, flags);
list_del_init(&kit->cursor.node);
raw_spin_unlock_irqrestore(&kit->dsq->lock, flags);
@@ -9857,6 +9860,9 @@ __bpf_kfunc void scx_bpf_dsq_reenq(u64 dsq_id, u64 reenq_flags,
return;
}
+ if (!scx_kfunc_nmi_safe(__func__, sch))
+ return;
+
/* not specifying any filter bits is the same as %SCX_REENQ_ANY */
if (!(reenq_flags & __SCX_REENQ_FILTER_MASK))
reenq_flags |= SCX_REENQ_ANY;
@@ -10244,6 +10250,9 @@ __bpf_kfunc void scx_bpf_cpuperf_set(s32 cpu, u32 perf, const struct bpf_prog_au
if (unlikely(!sch))
return;
+ if (!scx_kfunc_nmi_safe(__func__, sch))
+ return;
+
scx_cpuperf_set(sch, cpu, perf);
}
@@ -10269,6 +10278,10 @@ __bpf_kfunc s32 scx_bpf_cidperf_set(s32 cid, u32 perf,
sch = scx_prog_sched(aux);
if (unlikely(!sch))
return -ENODEV;
+
+ if (!scx_kfunc_nmi_safe(__func__, sch))
+ return -EBUSY;
+
cpu = scx_cid_to_cpu(sch, cid);
if (cpu < 0)
return cpu;
diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h
index 27bbf5e04d90..49b165effd67 100644
--- a/kernel/sched/ext/internal.h
+++ b/kernel/sched/ext/internal.h
@@ -2091,6 +2091,28 @@ extern struct scx_sched *scx_enabling_sub_sched;
#define scx_error(sch, fmt, args...) \
scx_exit((sch), SCX_EXIT_ERROR, 0, fmt, ##args)
+/*
+ * sched_ext kfuncs that take scheduler locks are not NMI-safe: a
+ * BPF_PROG_TYPE_TRACING program can be attached to a function that runs in
+ * NMI, and scx_kfunc_context_filter() lets such a program call every kfunc in
+ * the any/cid/idle sets. Acquiring the rq, dsq or pshard raw spinlocks - or
+ * touching the irq-masking-only kick list - from NMI while the interrupted
+ * context on the same CPU already holds them deadlocks (or corrupts) it.
+ * scx_bpf_kick_cpu() was the first guard; route all of them through here.
+ *
+ * Returns true when the caller may proceed, false when running from NMI and
+ * the kfunc must bail without touching locks. scx_error() is NMI-safe (see the
+ * lock-free ->aborting claim).
+ */
+static inline bool scx_kfunc_nmi_safe(const char *who, struct scx_sched *sch)
+{
+ if (unlikely(in_nmi())) {
+ scx_error(sch, "%s called from NMI", who);
+ return false;
+ }
+ return true;
+}
+
/**
* scx_root_protected_live - Root sched for paths that only run while live
*
diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
index 0554448835bd..b5125a871562 100644
--- a/kernel/sched/ext/sub.c
+++ b/kernel/sched/ext/sub.c
@@ -2265,6 +2265,9 @@ static s32 sub_cap_preamble(u64 cgroup_id, u64 caps, const struct bpf_prog_aux *
if (unlikely(!parent))
return -ENODEV;
+ if (!scx_kfunc_nmi_safe("sub-cap kfuncs", parent))
+ return -EBUSY;
+
if (!scx_is_cid_type()) {
scx_error(parent, "sub-cap kfuncs require a cid-form scheduler");
return -EOPNOTSUPP;
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH] sched_ext: Reject NMI calls to lock-taking kfuncs 2026-09-01 9:56 [PATCH] sched_ext: Reject NMI calls to lock-taking kfuncs Wanwu Li @ 2026-09-01 19:51 ` Andrea Righi 2026-09-01 20:13 ` Tejun Heo 2026-09-01 21:16 ` Tejun Heo 1 sibling, 1 reply; 13+ messages in thread From: Andrea Righi @ 2026-09-01 19:51 UTC (permalink / raw) To: Wanwu Li; +Cc: Tejun Heo, David Vernet, Changwoo Min, sched-ext, linux-kernel Hi Wanwu, On Tue, Sep 01, 2026 at 05:56:52PM +0800, Wanwu Li wrote: > commit e06ece82d7b0 ("sched_ext: Report NMI kicks with scx_error()") made > scx_bpf_kick_cpu() reject NMI calls, and its cover letter describes the > reachability: sched_ext kfuncs in the "any" category "are callable from > tracing progs that can attach to functions running in NMI", and an unlucky > call from there "could deadlock the machine". The fix in that series made > the error/exit path lock-free so scx_error() is safe to call from NMI. That > closes the *error* path of every kfunc, but not a kfunc's own > business-logic lock acquisition on its success path. > > The remaining lock-taking kfuncs that scx_kfunc_context_filter() exposes to > BPF_PROG_TYPE_TRACING have the same hazard: if an NMI lands on a CPU whose > interrupted context already holds the lock, the kfunc's raw spinlock > acquisition spins forever and hard-locks the CPU: > > - scx_bpf_destroy_dsq() -> dsq->lock > - scx_bpf_dsq_reenq() -> rq's deferred_reenq_lock > - scx_bpf_cpuperf_set() / scx_bpf_cidperf_set() -> rq->lock > - scx_bpf_sub_grant() / scx_bpf_sub_revoke() -> pshard lock > (via the shared sub_cap_preamble()) > - bpf_iter_scx_dsq_next() / bpf_iter_scx_dsq_destroy() -> dsq->lock > (bpf_iter_scx_dsq_new() is lockless and needs no guard) > > As things stand, there is no scenario for reenqueueing, iterating a DSQ, > setting a performance target or granting sub-caps from NMI. The guards > defend against a buggy or malicious BPF program turning an "any"-category > kfunc into a machine-wide hard-lockup through the door that > scx_kfunc_context_filter() already opens. This matches the intent of > scx_bpf_kick_cpu()'s NMI check, which the commit cited above added not to > enable an NMI use case but to surface such a bug as a clean abort. The deadlock scenario makes sense to me, but I wonder whether we should prevent these kfuncs from being called by tracing programs altogether instead of adding runtime checks to the scheduler paths. AFAICS, the lock-taking and state-changing kfuncs do not have a meaningful use from BPF_PROG_TYPE_TRACING. We could move them out of scx_kfunc_ids_any into a separate set registered only for BPF_PROG_TYPE_STRUCT_OPS. The read-only kfuncs could remain available to tracing programs. This should include: - scx_bpf_kick_cpu() / scx_bpf_kick_cid() - scx_bpf_destroy_dsq() - scx_bpf_dsq_reenq() / scx_bpf_reenqueue_local___v2() - bpf_iter_scx_dsq_{new,next,destroy}() - scx_bpf_cpuperf_set() / scx_bpf_cidperf_set() - scx_bpf_sub_grant() / scx_bpf_sub_revoke() (... maybe others that I'm missing ...) That would reject invalid programs at verification time, avoid the runtime overhead and make the API boundary explicit: tracing programs can observe sched_ext state, while only sched_ext schedulers can modify it. If BPF_PROG_TYPE_SYSCALL registration is needed for test_run or selftests, we can retain that separately since it cannot execute from NMI. What do you think? Thanks, -Andrea > > Route all of them through a new scx_kfunc_nmi_safe() helper and reuse > scx_bpf_kick_cpu()'s existing in_nmi() check - now shared with its cid > equivalent scx_bpf_kick_cid() through scx_kick_cpu() - so the rule is > stated once and the coverage is auditable from one place. scx_error() is > already NMI-safe (commit f883dbb64ca5 ("sched_ext: Make exit claiming > lock-free")), so the reject-abort cannot deadlock the lock acquisition. > > Read-only members of the reachable sets (dsq_peek, dsq_nr_queued, > cpuperf_cur/cap, sub_caps, the idle cpumask helpers and the cid lookups) > take no scheduler lock on the path a tracing program reaches them, and were > audited to that effect; they are correctly left unguarded. The select_cpu > kfuncs do take pi_lock, but scx_kfunc_context_filter() only exposes the > any/idle/cid sets to BPF_PROG_TYPE_TRACING, and struct_ops run in task > context, so no lock-taking path here is reachable from NMI. > > Signed-off-by: Wanwu Li <liwanwu@kylinos.cn> > --- > kernel/sched/ext/ext.c | 27 ++++++++++++++++++++------- > kernel/sched/ext/internal.h | 22 ++++++++++++++++++++++ > kernel/sched/ext/sub.c | 3 +++ > 3 files changed, 45 insertions(+), 7 deletions(-) > > diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c > index 10af28a9f2c0..a0b886975e1d 100644 > --- a/kernel/sched/ext/ext.c > +++ b/kernel/sched/ext/ext.c > @@ -5108,6 +5108,9 @@ static void destroy_dsq(struct scx_sched *sch, u64 dsq_id) > struct scx_dispatch_q *dsq; > unsigned long flags; > > + if (!scx_kfunc_nmi_safe("scx_bpf_destroy_dsq()", sch)) > + return; > + > rcu_read_lock(); > > dsq = find_user_dsq(sch, dsq_id); > @@ -9518,14 +9521,8 @@ void scx_kick_cpu(struct scx_sched *sch, s32 cpu, u64 flags) > struct rq *this_rq; > unsigned long irq_flags; > > - /* > - * The per-cpu kick list is guarded only by local_irq_save(), which does > - * not mask NMIs, so kicking from NMI could corrupt it and is unsupported. > - */ > - if (unlikely(in_nmi())) { > - scx_error(sch, "scx_bpf_kick_cpu() called from NMI"); > + if (!scx_kfunc_nmi_safe("scx_bpf_kick_cpu()", sch)) > return; > - } > > local_irq_save(irq_flags); > > @@ -9756,6 +9753,9 @@ __bpf_kfunc struct task_struct *bpf_iter_scx_dsq_next(struct bpf_iter_scx_dsq *i > if (!kit->dsq) > return NULL; > > + if (!scx_kfunc_nmi_safe(__func__, kit->dsq->sched)) > + return NULL; > + > guard(raw_spinlock_irqsave)(&kit->dsq->lock); > > return nldsq_cursor_next_task(&kit->cursor, kit->dsq); > @@ -9777,6 +9777,9 @@ __bpf_kfunc void bpf_iter_scx_dsq_destroy(struct bpf_iter_scx_dsq *it) > if (!list_empty(&kit->cursor.node)) { > unsigned long flags; > > + if (!scx_kfunc_nmi_safe(__func__, kit->dsq->sched)) > + return; > + > raw_spin_lock_irqsave(&kit->dsq->lock, flags); > list_del_init(&kit->cursor.node); > raw_spin_unlock_irqrestore(&kit->dsq->lock, flags); > @@ -9857,6 +9860,9 @@ __bpf_kfunc void scx_bpf_dsq_reenq(u64 dsq_id, u64 reenq_flags, > return; > } > > + if (!scx_kfunc_nmi_safe(__func__, sch)) > + return; > + > /* not specifying any filter bits is the same as %SCX_REENQ_ANY */ > if (!(reenq_flags & __SCX_REENQ_FILTER_MASK)) > reenq_flags |= SCX_REENQ_ANY; > @@ -10244,6 +10250,9 @@ __bpf_kfunc void scx_bpf_cpuperf_set(s32 cpu, u32 perf, const struct bpf_prog_au > if (unlikely(!sch)) > return; > > + if (!scx_kfunc_nmi_safe(__func__, sch)) > + return; > + > scx_cpuperf_set(sch, cpu, perf); > } > > @@ -10269,6 +10278,10 @@ __bpf_kfunc s32 scx_bpf_cidperf_set(s32 cid, u32 perf, > sch = scx_prog_sched(aux); > if (unlikely(!sch)) > return -ENODEV; > + > + if (!scx_kfunc_nmi_safe(__func__, sch)) > + return -EBUSY; > + > cpu = scx_cid_to_cpu(sch, cid); > if (cpu < 0) > return cpu; > diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h > index 27bbf5e04d90..49b165effd67 100644 > --- a/kernel/sched/ext/internal.h > +++ b/kernel/sched/ext/internal.h > @@ -2091,6 +2091,28 @@ extern struct scx_sched *scx_enabling_sub_sched; > #define scx_error(sch, fmt, args...) \ > scx_exit((sch), SCX_EXIT_ERROR, 0, fmt, ##args) > > +/* > + * sched_ext kfuncs that take scheduler locks are not NMI-safe: a > + * BPF_PROG_TYPE_TRACING program can be attached to a function that runs in > + * NMI, and scx_kfunc_context_filter() lets such a program call every kfunc in > + * the any/cid/idle sets. Acquiring the rq, dsq or pshard raw spinlocks - or > + * touching the irq-masking-only kick list - from NMI while the interrupted > + * context on the same CPU already holds them deadlocks (or corrupts) it. > + * scx_bpf_kick_cpu() was the first guard; route all of them through here. > + * > + * Returns true when the caller may proceed, false when running from NMI and > + * the kfunc must bail without touching locks. scx_error() is NMI-safe (see the > + * lock-free ->aborting claim). > + */ > +static inline bool scx_kfunc_nmi_safe(const char *who, struct scx_sched *sch) > +{ > + if (unlikely(in_nmi())) { > + scx_error(sch, "%s called from NMI", who); > + return false; > + } > + return true; > +} > + > /** > * scx_root_protected_live - Root sched for paths that only run while live > * > diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c > index 0554448835bd..b5125a871562 100644 > --- a/kernel/sched/ext/sub.c > +++ b/kernel/sched/ext/sub.c > @@ -2265,6 +2265,9 @@ static s32 sub_cap_preamble(u64 cgroup_id, u64 caps, const struct bpf_prog_aux * > if (unlikely(!parent)) > return -ENODEV; > > + if (!scx_kfunc_nmi_safe("sub-cap kfuncs", parent)) > + return -EBUSY; > + > if (!scx_is_cid_type()) { > scx_error(parent, "sub-cap kfuncs require a cid-form scheduler"); > return -EOPNOTSUPP; > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] sched_ext: Reject NMI calls to lock-taking kfuncs 2026-09-01 19:51 ` Andrea Righi @ 2026-09-01 20:13 ` Tejun Heo 2026-09-01 20:28 ` Andrea Righi 0 siblings, 1 reply; 13+ messages in thread From: Tejun Heo @ 2026-09-01 20:13 UTC (permalink / raw) To: Andrea Righi Cc: Wanwu Li, David Vernet, Changwoo Min, sched-ext, linux-kernel Hello, On Tue, Sep 01, 2026 at 09:51:37PM +0200, Andrea Righi wrote: > AFAICS, the lock-taking and state-changing kfuncs do not have a meaningful use > from BPF_PROG_TYPE_TRACING. We could move them out of scx_kfunc_ids_any into a > separate set registered only for BPF_PROG_TYPE_STRUCT_OPS. The read-only kfuncs > could remain available to tracing programs. > > This should include: > - scx_bpf_kick_cpu() / scx_bpf_kick_cid() > - scx_bpf_destroy_dsq() > - scx_bpf_dsq_reenq() / scx_bpf_reenqueue_local___v2() > - bpf_iter_scx_dsq_{new,next,destroy}() > - scx_bpf_cpuperf_set() / scx_bpf_cidperf_set() > - scx_bpf_sub_grant() / scx_bpf_sub_revoke() > (... maybe others that I'm missing ...) Hmm... but it *is* useful to be able to e.g. kick a CPU or trigger reenq from a trace event, no? Thanks. -- tejun ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] sched_ext: Reject NMI calls to lock-taking kfuncs 2026-09-01 20:13 ` Tejun Heo @ 2026-09-01 20:28 ` Andrea Righi 0 siblings, 0 replies; 13+ messages in thread From: Andrea Righi @ 2026-09-01 20:28 UTC (permalink / raw) To: Tejun Heo; +Cc: Wanwu Li, David Vernet, Changwoo Min, sched-ext, linux-kernel On Tue, Sep 01, 2026 at 10:13:27AM -1000, Tejun Heo wrote: > Hello, > > On Tue, Sep 01, 2026 at 09:51:37PM +0200, Andrea Righi wrote: > > AFAICS, the lock-taking and state-changing kfuncs do not have a meaningful use > > from BPF_PROG_TYPE_TRACING. We could move them out of scx_kfunc_ids_any into a > > separate set registered only for BPF_PROG_TYPE_STRUCT_OPS. The read-only kfuncs > > could remain available to tracing programs. > > > > This should include: > > - scx_bpf_kick_cpu() / scx_bpf_kick_cid() > > - scx_bpf_destroy_dsq() > > - scx_bpf_dsq_reenq() / scx_bpf_reenqueue_local___v2() > > - bpf_iter_scx_dsq_{new,next,destroy}() > > - scx_bpf_cpuperf_set() / scx_bpf_cidperf_set() > > - scx_bpf_sub_grant() / scx_bpf_sub_revoke() > > (... maybe others that I'm missing ...) > > Hmm... but it *is* useful to be able to e.g. kick a CPU or trigger reenq > from a trace event, no? Kicking a CPU, yes - scx_pair is actually using that. I'm less convinced about the reenq case. We used to have the famous reenq from sched_switch use case, but now that we have ENQ_IMMED and deadline server, are there other concrete use cases for it? Maybe we should decide tracing visibility per kfunc rather than exposing every "any" kfunc and adding the runtime check to all of them. But this can be revised/improved later. In the meantime this seems to fix a real issue and the approach looks correct, so: Acked-by: Andrea Righi <arighi@nvidia.com> Thanks, -Andrea ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] sched_ext: Reject NMI calls to lock-taking kfuncs 2026-09-01 9:56 [PATCH] sched_ext: Reject NMI calls to lock-taking kfuncs Wanwu Li 2026-09-01 19:51 ` Andrea Righi @ 2026-09-01 21:16 ` Tejun Heo 2026-09-02 2:31 ` [PATCH v2] " Wanwu Li 1 sibling, 1 reply; 13+ messages in thread From: Tejun Heo @ 2026-09-01 21:16 UTC (permalink / raw) To: Wanwu Li Cc: David Vernet, Andrea Righi, Changwoo Min, Emil Tsalapatis, sched-ext, linux-kernel Hello, On Tue, Sep 01, 2026 at 05:56:52PM +0800, Wanwu Li wrote: > + if (!scx_kfunc_nmi_safe(__func__, sch)) > + return -EBUSY; The return value doesn't matter much as the scheduler is being terminated. Whatever it is, use the same one everywhere. -EBUSY suggests retrying would help. Let's use -EDEADLK which is what's being avoided. > +/* > + * sched_ext kfuncs that take scheduler locks are not NMI-safe: a > + * BPF_PROG_TYPE_TRACING program can be attached to a function that runs in > + * NMI, and scx_kfunc_context_filter() lets such a program call every kfunc in > + * the any/cid/idle sets. Acquiring the rq, dsq or pshard raw spinlocks - or > + * touching the irq-masking-only kick list - from NMI while the interrupted > + * context on the same CPU already holds them deadlocks (or corrupts) it. > + * scx_bpf_kick_cpu() was the first guard; route all of them through here. > + * > + * Returns true when the caller may proceed, false when running from NMI and > + * the kfunc must bail without touching locks. scx_error() is NMI-safe (see the > + * lock-free ->aborting claim). > + */ Too long. Something like: /* * 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. */ > +static inline bool scx_kfunc_nmi_safe(const char *who, struct scx_sched *sch) scx_kfunc_nmi_safe() reads as if it's testing a property of the kfunc. Let's call it scx_kf_allowed_ctx() to match scx_kf_allowed(). Also, instead of passing @who from each call site, make it a macro wrapper which passes __func__ to the inline function. Thanks. -- tejun ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] sched_ext: Reject NMI calls to lock-taking kfuncs 2026-09-01 21:16 ` Tejun Heo @ 2026-09-02 2:31 ` Wanwu Li 2026-09-02 6:44 ` Tejun Heo 0 siblings, 1 reply; 13+ messages in thread From: Wanwu Li @ 2026-09-02 2:31 UTC (permalink / raw) To: Tejun Heo, Andrea Righi Cc: David Vernet, Changwoo Min, Emil Tsalapatis, sched-ext, linux-kernel, Wanwu Li commit e06ece82d7b0 ("sched_ext: Report NMI kicks with scx_error()") made scx_bpf_kick_cpu() reject NMI calls, and its cover letter describes the reachability: sched_ext kfuncs in the "any" category "are callable from tracing progs that can attach to functions running in NMI", and an unlucky call from there "could deadlock the machine". The fix in that series made the error/exit path lock-free so scx_error() is safe to call from NMI. That closes the *error* path of every kfunc, but not a kfunc's own business-logic lock acquisition on its success path. The remaining lock-taking kfuncs that scx_kfunc_context_filter() exposes to BPF_PROG_TYPE_TRACING have the same hazard: if an NMI lands on a CPU whose interrupted context already holds the lock, the kfunc's raw spinlock acquisition spins forever and hard-locks the CPU: - scx_bpf_destroy_dsq() -> dsq->lock - scx_bpf_dsq_reenq() -> rq's deferred_reenq_lock - scx_bpf_cpuperf_set() / scx_bpf_cidperf_set() -> rq->lock - scx_bpf_sub_grant() / scx_bpf_sub_revoke() -> pshard lock (via the shared sub_cap_preamble()) - bpf_iter_scx_dsq_next() / bpf_iter_scx_dsq_destroy() -> dsq->lock (bpf_iter_scx_dsq_new() is lockless and needs no guard) As things stand, there is no scenario for reenqueueing, iterating a DSQ, setting a performance target or granting sub-caps from NMI. The guards defend against a buggy or malicious BPF program turning an "any"-category kfunc into a machine-wide hard-lockup through the door that scx_kfunc_context_filter() already opens. This matches the intent of scx_bpf_kick_cpu()'s NMI check, which the commit cited above added not to enable an NMI use case but to surface such a bug as a clean abort. Route all of them through a new scx_kf_allowed_ctx() helper and reuse scx_bpf_kick_cpu()'s existing in_nmi() check - now shared with its cid equivalent scx_bpf_kick_cid() through scx_kick_cpu() - so the rule is stated once and the coverage is auditable from one place. scx_error() is already NMI-safe (commit f883dbb64ca5 ("sched_ext: Make exit claiming lock-free")), so the reject-abort cannot deadlock the lock acquisition. Kfuncs with an error return report -EDEADLK, the situation being avoided. Read-only members of the reachable sets (dsq_peek, dsq_nr_queued, cpuperf_cur/cap, sub_caps, the idle cpumask helpers and the cid lookups) take no scheduler lock on the path a tracing program reaches them, and were audited to that effect; they are correctly left unguarded. The select_cpu kfuncs do take pi_lock, but scx_kfunc_context_filter() only exposes the any/idle/cid sets to BPF_PROG_TYPE_TRACING, and struct_ops run in task context, so no lock-taking path here is reachable from NMI. Link: https://lore.kernel.org/r/20260901095652.1009104-1-liwanwu@kylinos.cn Acked-by: Andrea Righi <arighi@nvidia.com> Signed-off-by: Wanwu Li <liwanwu@kylinos.cn> --- Changes in v2: - Rename the helper to scx_kf_allowed_ctx(), implemented as a macro that passes __func__ to the inline __scx_kf_allowed_ctx() so callers no longer spell out @who (Tejun Heo). - Report -EDEADLK instead of -EBUSY where a kfunc bails out with an error (Tejun Heo). - Condense the in-code comment to the suggested wording (Tejun Heo); the extended rationale now lives in this commit message. - Move the destroy_dsq() guard into the scx_bpf_destroy_dsq() kfunc itself so that __func__ reports the user-visible name and the internal callers of destroy_dsq() are unaffected. scx_kick_cpu() now reports itself in the error message (shared with scx_bpf_kick_cid()). - Carry over Acked-by from Andrea Righi (v1). kernel/sched/ext/ext.c | 33 ++++++++++++++++++++++++--------- kernel/sched/ext/internal.h | 17 +++++++++++++++++ kernel/sched/ext/sub.c | 3 +++ 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c index 10af28a9f2c0..09ce239e9c4b 100644 --- a/kernel/sched/ext/ext.c +++ b/kernel/sched/ext/ext.c @@ -9518,14 +9518,8 @@ void scx_kick_cpu(struct scx_sched *sch, s32 cpu, u64 flags) struct rq *this_rq; unsigned long irq_flags; - /* - * The per-cpu kick list is guarded only by local_irq_save(), which does - * not mask NMIs, so kicking from NMI could corrupt it and is unsupported. - */ - if (unlikely(in_nmi())) { - scx_error(sch, "scx_bpf_kick_cpu() called from NMI"); + if (!scx_kf_allowed_ctx(sch)) return; - } local_irq_save(irq_flags); @@ -9693,8 +9687,13 @@ __bpf_kfunc void scx_bpf_destroy_dsq(u64 dsq_id, const struct bpf_prog_aux *aux) guard(rcu)(); sch = scx_prog_sched(aux); - if (sch) - destroy_dsq(sch, dsq_id); + if (!sch) + return; + + if (!scx_kf_allowed_ctx(sch)) + return; + + destroy_dsq(sch, dsq_id); } /** @@ -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; + guard(raw_spinlock_irqsave)(&kit->dsq->lock); return nldsq_cursor_next_task(&kit->cursor, kit->dsq); @@ -9777,6 +9779,9 @@ __bpf_kfunc void bpf_iter_scx_dsq_destroy(struct bpf_iter_scx_dsq *it) if (!list_empty(&kit->cursor.node)) { unsigned long flags; + if (!scx_kf_allowed_ctx(kit->dsq->sched)) + return; + raw_spin_lock_irqsave(&kit->dsq->lock, flags); list_del_init(&kit->cursor.node); raw_spin_unlock_irqrestore(&kit->dsq->lock, flags); @@ -9857,6 +9862,9 @@ __bpf_kfunc void scx_bpf_dsq_reenq(u64 dsq_id, u64 reenq_flags, return; } + if (!scx_kf_allowed_ctx(sch)) + return; + /* not specifying any filter bits is the same as %SCX_REENQ_ANY */ if (!(reenq_flags & __SCX_REENQ_FILTER_MASK)) reenq_flags |= SCX_REENQ_ANY; @@ -10244,6 +10252,9 @@ __bpf_kfunc void scx_bpf_cpuperf_set(s32 cpu, u32 perf, const struct bpf_prog_au if (unlikely(!sch)) return; + if (!scx_kf_allowed_ctx(sch)) + return; + scx_cpuperf_set(sch, cpu, perf); } @@ -10269,6 +10280,10 @@ __bpf_kfunc s32 scx_bpf_cidperf_set(s32 cid, u32 perf, sch = scx_prog_sched(aux); if (unlikely(!sch)) return -ENODEV; + + if (!scx_kf_allowed_ctx(sch)) + return -EDEADLK; + cpu = scx_cid_to_cpu(sch, cid); if (cpu < 0) return cpu; diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h index 27bbf5e04d90..d7f48ec63e01 100644 --- a/kernel/sched/ext/internal.h +++ b/kernel/sched/ext/internal.h @@ -2091,6 +2091,23 @@ 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. + */ +static __always_inline bool __scx_kf_allowed_ctx(struct scx_sched *sch, + const char *who) +{ + if (unlikely(in_nmi())) { + scx_error(sch, "%s called from NMI", who); + return false; + } + return true; +} + +#define scx_kf_allowed_ctx(sch) __scx_kf_allowed_ctx((sch), __func__) + /** * scx_root_protected_live - Root sched for paths that only run while live * diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c index 0554448835bd..7d444f7aa7ab 100644 --- a/kernel/sched/ext/sub.c +++ b/kernel/sched/ext/sub.c @@ -2265,6 +2265,9 @@ static s32 sub_cap_preamble(u64 cgroup_id, u64 caps, const struct bpf_prog_aux * if (unlikely(!parent)) return -ENODEV; + if (!__scx_kf_allowed_ctx(parent, "sub-cap kfuncs")) + return -EDEADLK; + if (!scx_is_cid_type()) { scx_error(parent, "sub-cap kfuncs require a cid-form scheduler"); return -EOPNOTSUPP; -- 2.25.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] sched_ext: Reject NMI calls to lock-taking kfuncs 2026-09-02 2:31 ` [PATCH v2] " Wanwu Li @ 2026-09-02 6:44 ` Tejun Heo 2026-09-02 9:36 ` [PATCH v3] " Wanwu Li 2026-09-02 13:48 ` [PATCH v2] " liwanwu 0 siblings, 2 replies; 13+ messages in thread From: Tejun Heo @ 2026-09-02 6:44 UTC (permalink / raw) To: Wanwu Li Cc: Andrea Righi, David Vernet, Changwoo Min, Emil Tsalapatis, sched-ext, linux-kernel 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. -- tejun ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3] sched_ext: Reject NMI calls to lock-taking kfuncs 2026-09-02 6:44 ` Tejun Heo @ 2026-09-02 9:36 ` Wanwu Li 2026-09-02 9:51 ` sashiko-bot 2026-09-02 23:05 ` Tejun Heo 2026-09-02 13:48 ` [PATCH v2] " liwanwu 1 sibling, 2 replies; 13+ messages in thread From: Wanwu Li @ 2026-09-02 9:36 UTC (permalink / raw) To: tj; +Cc: arighi, changwoo, emil, linux-kernel, liwanwu, sched-ext, void commit e06ece82d7b0 ("sched_ext: Report NMI kicks with scx_error()") made scx_bpf_kick_cpu() reject NMI calls, and its cover letter describes the reachability: sched_ext kfuncs in the "any" category "are callable from tracing progs that can attach to functions running in NMI", and an unlucky call from there "could deadlock the machine". The fix in that series made the error/exit path lock-free so scx_error() is safe to call from NMI. That closes the *error* path of every kfunc, but not a kfunc's own business-logic lock acquisition on its success path. The remaining lock-taking kfuncs that scx_kfunc_context_filter() exposes to BPF_PROG_TYPE_TRACING have the same hazard: if an NMI lands on a CPU whose interrupted context already holds the lock, the kfunc's raw spinlock acquisition spins forever and hard-locks the CPU: - scx_bpf_destroy_dsq() -> dsq->lock - scx_bpf_dsq_reenq() -> rq's deferred_reenq_lock - scx_bpf_cpuperf_set() / scx_bpf_cidperf_set() -> rq->lock - scx_bpf_sub_grant() / scx_bpf_sub_revoke() -> pshard lock (via the shared sub_cap_preamble()) - bpf_iter_scx_dsq_next() / bpf_iter_scx_dsq_destroy() -> dsq->lock (the reject is placed in bpf_iter_scx_dsq_new(), which is otherwise lockless, so next() and destroy() are no-ops while it leaves kit->dsq NULL) As things stand, there is no scenario for reenqueueing, iterating a DSQ, setting a performance target or granting sub-caps from NMI. The guards defend against a buggy or malicious BPF program turning an "any"-category kfunc into a machine-wide hard-lockup through the door that scx_kfunc_context_filter() already opens. This matches the intent of scx_bpf_kick_cpu()'s NMI check, which the commit cited above added not to enable an NMI use case but to surface such a bug as a clean abort. Route all of them through a new scx_kf_allowed_ctx() helper and reuse scx_bpf_kick_cpu()'s existing in_nmi() check - now shared with its cid equivalent scx_bpf_kick_cid() through scx_kick_cpu() - so the rule is stated once and the coverage is auditable from one place. scx_error() is already NMI-safe (commit f883dbb64ca5 ("sched_ext: Make exit claiming lock-free")), so the reject-abort cannot deadlock the lock acquisition. Kfuncs with an error return report -EDEADLK, the situation being avoided. Read-only members of the reachable sets (dsq_peek, dsq_nr_queued, cpuperf_cur/cap, sub_caps, the idle cpumask helpers and the cid lookups) take no scheduler lock on the path a tracing program reaches them, and were audited to that effect; they are correctly left unguarded. The select_cpu kfuncs do take pi_lock, but scx_kfunc_context_filter() only exposes the any/idle/cid sets to BPF_PROG_TYPE_TRACING, and struct_ops run in task context, so no lock-taking path here is reachable from NMI. Link: https://lore.kernel.org/r/20260901095652.1009104-1-liwanwu@kylinos.cn Acked-by: Andrea Righi <arighi@nvidia.com> Signed-off-by: Wanwu Li <liwanwu@kylinos.cn> --- V2 -> V3: - Collapse the __scx_kf_allowed_ctx() inline + scx_kf_allowed_ctx() macro into a single macro that takes the function name implicitly (Tejun Heo). - Guard DSQ iteration once in bpf_iter_scx_dsq_new(); drop the redundant checks in bpf_iter_scx_dsq_next()/destroy() (Tejun Heo). - Use unlikely(!sch) in scx_bpf_destroy_dsq() (Tejun Heo). v2: https://lore.kernel.org/r/20260902023124.1422942-1-liwanwu@kylinos.cn v1: https://lore.kernel.org/r/20260901095652.1009104-1-liwanwu@kylinos.cn kernel/sched/ext/ext.c | 30 +++++++++++++++++++++--------- kernel/sched/ext/internal.h | 16 ++++++++++++++++ kernel/sched/ext/sub.c | 3 +++ 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c index 10af28a9f2c0..3f8bde1dbb4c 100644 --- a/kernel/sched/ext/ext.c +++ b/kernel/sched/ext/ext.c @@ -9518,14 +9518,8 @@ void scx_kick_cpu(struct scx_sched *sch, s32 cpu, u64 flags) struct rq *this_rq; unsigned long irq_flags; - /* - * The per-cpu kick list is guarded only by local_irq_save(), which does - * not mask NMIs, so kicking from NMI could corrupt it and is unsupported. - */ - if (unlikely(in_nmi())) { - scx_error(sch, "scx_bpf_kick_cpu() called from NMI"); + if (!scx_kf_allowed_ctx(sch)) return; - } local_irq_save(irq_flags); @@ -9693,8 +9687,13 @@ __bpf_kfunc void scx_bpf_destroy_dsq(u64 dsq_id, const struct bpf_prog_aux *aux) guard(rcu)(); sch = scx_prog_sched(aux); - if (sch) - destroy_dsq(sch, dsq_id); + if (unlikely(!sch)) + return; + + if (!scx_kf_allowed_ctx(sch)) + return; + + destroy_dsq(sch, dsq_id); } /** @@ -9731,6 +9730,9 @@ __bpf_kfunc int bpf_iter_scx_dsq_new(struct bpf_iter_scx_dsq *it, u64 dsq_id, if (unlikely(!sch)) return -ENODEV; + if (!scx_kf_allowed_ctx(sch)) + return -EDEADLK; + if (flags & ~__SCX_DSQ_ITER_USER_FLAGS) return -EINVAL; @@ -9857,6 +9859,9 @@ __bpf_kfunc void scx_bpf_dsq_reenq(u64 dsq_id, u64 reenq_flags, return; } + if (!scx_kf_allowed_ctx(sch)) + return; + /* not specifying any filter bits is the same as %SCX_REENQ_ANY */ if (!(reenq_flags & __SCX_REENQ_FILTER_MASK)) reenq_flags |= SCX_REENQ_ANY; @@ -10244,6 +10249,9 @@ __bpf_kfunc void scx_bpf_cpuperf_set(s32 cpu, u32 perf, const struct bpf_prog_au if (unlikely(!sch)) return; + if (!scx_kf_allowed_ctx(sch)) + return; + scx_cpuperf_set(sch, cpu, perf); } @@ -10269,6 +10277,10 @@ __bpf_kfunc s32 scx_bpf_cidperf_set(s32 cid, u32 perf, sch = scx_prog_sched(aux); if (unlikely(!sch)) return -ENODEV; + + if (!scx_kf_allowed_ctx(sch)) + return -EDEADLK; + cpu = scx_cid_to_cpu(sch, cid); if (cpu < 0) return cpu; diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h index 27bbf5e04d90..ca8ebb11f03c 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; \ +}) + /** * scx_root_protected_live - Root sched for paths that only run while live * diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c index 0554448835bd..78349acd0a67 100644 --- a/kernel/sched/ext/sub.c +++ b/kernel/sched/ext/sub.c @@ -2265,6 +2265,9 @@ static s32 sub_cap_preamble(u64 cgroup_id, u64 caps, const struct bpf_prog_aux * if (unlikely(!parent)) return -ENODEV; + if (!scx_kf_allowed_ctx(parent)) + return -EDEADLK; + if (!scx_is_cid_type()) { scx_error(parent, "sub-cap kfuncs require a cid-form scheduler"); return -EOPNOTSUPP; -- 2.25.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3] sched_ext: Reject NMI calls to lock-taking kfuncs 2026-09-02 9:36 ` [PATCH v3] " Wanwu Li @ 2026-09-02 9:51 ` sashiko-bot 2026-09-02 23:05 ` Tejun Heo 1 sibling, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-09-02 9:51 UTC (permalink / raw) To: Wanwu Li; +Cc: sched-ext 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3] sched_ext: Reject NMI calls to lock-taking kfuncs 2026-09-02 9:36 ` [PATCH v3] " Wanwu Li 2026-09-02 9:51 ` sashiko-bot @ 2026-09-02 23:05 ` Tejun Heo 2026-09-03 1:46 ` liwanwu 1 sibling, 1 reply; 13+ messages in thread From: Tejun Heo @ 2026-09-02 23:05 UTC (permalink / raw) To: Wanwu Li Cc: Andrea Righi, Changwoo Min, Emil Tsalapatis, David Vernet, sched-ext, linux-kernel Hello, On Wed, Sep 02, 2026 at 05:36:11PM +0800, Wanwu Li wrote: > +#define scx_kf_allowed_ctx(sch) \ > +({ \ > + bool __allowed = true; \ Applied to sched_ext/for-7.4 with the following changes: - "No need to wrap" in my v2 reply was about the line wrap of the function signature. Restored the inline function + macro wrapper form: static __always_inline bool __scx_kf_allowed_ctx(struct scx_sched *sch, const char *who) { if (unlikely(in_nmi())) { scx_error(sch, "%s called from NMI", who); return false; } return true; } #define scx_kf_allowed_ctx(sch) __scx_kf_allowed_ctx((sch), __func__) - struct_ops don't run in task context (e.g. ops.tick() runs from the tick interrupt). They just never run in NMI. Updated the last paragraph of the description accordingly. Thanks. -- tejun ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3] sched_ext: Reject NMI calls to lock-taking kfuncs 2026-09-02 23:05 ` Tejun Heo @ 2026-09-03 1:46 ` liwanwu 0 siblings, 0 replies; 13+ messages in thread From: liwanwu @ 2026-09-03 1:46 UTC (permalink / raw) To: Tejun Heo Cc: Andrea Righi, Changwoo Min, Emil Tsalapatis, David Vernet, sched-ext, linux-kernel Thanks for the correction and the guidance. I'll send a follow-up patch to fix the remaining issues. 在 2026/9/3 07:05, Tejun Heo 写道: > Hello, > > On Wed, Sep 02, 2026 at 05:36:11PM +0800, Wanwu Li wrote: >> +#define scx_kf_allowed_ctx(sch) \ >> +({ \ >> + bool __allowed = true; \ > > Applied to sched_ext/for-7.4 with the following changes: > > - "No need to wrap" in my v2 reply was about the line wrap of the function > signature. Restored the inline function + macro wrapper form: > > static __always_inline bool __scx_kf_allowed_ctx(struct scx_sched *sch, const char *who) > { > if (unlikely(in_nmi())) { > scx_error(sch, "%s called from NMI", who); > return false; > } > return true; > } > > #define scx_kf_allowed_ctx(sch) __scx_kf_allowed_ctx((sch), __func__) > > - struct_ops don't run in task context (e.g. ops.tick() runs from the tick > interrupt). They just never run in NMI. Updated the last paragraph of the > description accordingly. > > Thanks. > > -- > tejun ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] sched_ext: Reject NMI calls to lock-taking kfuncs 2026-09-02 6:44 ` Tejun Heo 2026-09-02 9:36 ` [PATCH v3] " Wanwu Li @ 2026-09-02 13:48 ` liwanwu 2026-09-02 21:51 ` Tejun Heo 1 sibling, 1 reply; 13+ messages in thread From: liwanwu @ 2026-09-02 13:48 UTC (permalink / raw) To: Tejun Heo Cc: Andrea Righi, David Vernet, Changwoo Min, Emil Tsalapatis, sched-ext, linux-kernel 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. > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] sched_ext: Reject NMI calls to lock-taking kfuncs 2026-09-02 13:48 ` [PATCH v2] " liwanwu @ 2026-09-02 21:51 ` Tejun Heo 0 siblings, 0 replies; 13+ messages in thread From: Tejun Heo @ 2026-09-02 21:51 UTC (permalink / raw) To: liwanwu Cc: Andrea Righi, David Vernet, Changwoo Min, Emil Tsalapatis, sched-ext, linux-kernel Hello, On Wed, Sep 02, 2026 at 09:48:28PM +0800, liwanwu wrote: > 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: Let's add the in_nmi() test to scx_locked_rq() so that it returns NULL from NMI. That sends all three down their unlocked paths. > 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. That probably needs to be irqsave'd. preempt_disable() doesn't mask IRQs either and the idle kfuncs can be called from IRQ-enabled contexts. As for NMI, if someone is calling pick_idle from NMI, they're asking for it. As long as the machine doesn't crash, it doesn't matter. Thanks. -- tejun ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-09-03 1:46 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-01 9:56 [PATCH] sched_ext: Reject NMI calls to lock-taking kfuncs Wanwu Li 2026-09-01 19:51 ` Andrea Righi 2026-09-01 20:13 ` Tejun Heo 2026-09-01 20:28 ` Andrea Righi 2026-09-01 21:16 ` Tejun Heo 2026-09-02 2:31 ` [PATCH v2] " Wanwu Li 2026-09-02 6:44 ` Tejun Heo 2026-09-02 9:36 ` [PATCH v3] " Wanwu Li 2026-09-02 9:51 ` sashiko-bot 2026-09-02 23:05 ` Tejun Heo 2026-09-03 1:46 ` liwanwu 2026-09-02 13:48 ` [PATCH v2] " liwanwu 2026-09-02 21:51 ` Tejun Heo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox