* [PATCH bpf-next 2/2] bpf: Consolidate sleepable checks in check_kfunc_call()
2026-02-25 13:49 [PATCH bpf-next 1/2] bpf: Consolidate sleepable checks in check_helper_call() Puranjay Mohan
@ 2026-02-25 13:49 ` Puranjay Mohan
2026-02-25 18:54 ` Mykyta Yatsenko
2026-02-26 9:18 ` Eduard Zingerman
2026-02-25 18:30 ` [PATCH bpf-next 1/2] bpf: Consolidate sleepable checks in check_helper_call() Mykyta Yatsenko
2026-02-26 9:12 ` Eduard Zingerman
2 siblings, 2 replies; 6+ messages in thread
From: Puranjay Mohan @ 2026-02-25 13:49 UTC (permalink / raw)
To: bpf
Cc: Puranjay Mohan, Puranjay Mohan, Alexei Starovoitov,
Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Mykyta Yatsenko,
kernel-team
check_kfunc_call() has multiple scattered checks that reject sleepable
kfuncs in various non-sleepable contexts (RCU, preempt-disabled, IRQ-
disabled). These are the same conditions already checked by
in_sleepable_context(), so replace them with a single consolidated
check.
This also simplifies the preempt lock tracking by flattening the nested
if/else structure into a linear chain: preempt_disable increments,
preempt_enable checks for underflow and decrements, and the sleepable
check uses in_sleepable_context() which covers all non-sleepable
contexts uniformly.
No functional change since in_sleepable_context() checks all the same
state (active_rcu_locks, active_preempt_locks, active_locks,
active_irq_id, in_sleepable).
Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
---
kernel/bpf/verifier.c | 35 ++++++++++++-----------------------
1 file changed, 12 insertions(+), 23 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index e8c4a5f8520d..c26139b96c6a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -14153,34 +14153,23 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
}
}));
}
- } else if (sleepable && env->cur_state->active_rcu_locks) {
- verbose(env, "kernel func %s is sleepable within rcu_read_lock region\n", func_name);
- return -EACCES;
- }
-
- if (in_rbtree_lock_required_cb(env) && (rcu_lock || rcu_unlock)) {
- verbose(env, "Calling bpf_rcu_read_{lock,unlock} in unnecessary rbtree callback\n");
- return -EACCES;
- }
-
- if (env->cur_state->active_preempt_locks) {
- if (preempt_disable) {
- env->cur_state->active_preempt_locks++;
- } else if (preempt_enable) {
- env->cur_state->active_preempt_locks--;
- } else if (sleepable) {
- verbose(env, "kernel func %s is sleepable within non-preemptible region\n", func_name);
- return -EACCES;
- }
} else if (preempt_disable) {
env->cur_state->active_preempt_locks++;
} else if (preempt_enable) {
- verbose(env, "unmatched attempt to enable preemption (kernel function %s)\n", func_name);
- return -EINVAL;
+ if (env->cur_state->active_preempt_locks == 0) {
+ verbose(env, "unmatched attempt to enable preemption (kernel function %s)\n",
+ func_name);
+ return -EINVAL;
+ }
+ env->cur_state->active_preempt_locks--;
+ } else if (sleepable && !in_sleepable_context(env)) {
+ verbose(env, "kernel func %s is sleepable within %s\n",
+ func_name, non_sleepable_context_description(env));
+ return -EACCES;
}
- if (env->cur_state->active_irq_id && sleepable) {
- verbose(env, "kernel func %s is sleepable within IRQ-disabled region\n", func_name);
+ if (in_rbtree_lock_required_cb(env) && (rcu_lock || rcu_unlock)) {
+ verbose(env, "Calling bpf_rcu_read_{lock,unlock} in unnecessary rbtree callback\n");
return -EACCES;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH bpf-next 2/2] bpf: Consolidate sleepable checks in check_kfunc_call()
2026-02-25 13:49 ` [PATCH bpf-next 2/2] bpf: Consolidate sleepable checks in check_kfunc_call() Puranjay Mohan
@ 2026-02-25 18:54 ` Mykyta Yatsenko
2026-02-26 9:18 ` Eduard Zingerman
1 sibling, 0 replies; 6+ messages in thread
From: Mykyta Yatsenko @ 2026-02-25 18:54 UTC (permalink / raw)
To: Puranjay Mohan, bpf
Cc: Puranjay Mohan, Puranjay Mohan, Alexei Starovoitov,
Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau,
Eduard Zingerman, Kumar Kartikeya Dwivedi, kernel-team
Puranjay Mohan <puranjay@kernel.org> writes:
> check_kfunc_call() has multiple scattered checks that reject sleepable
> kfuncs in various non-sleepable contexts (RCU, preempt-disabled, IRQ-
> disabled). These are the same conditions already checked by
> in_sleepable_context(), so replace them with a single consolidated
> check.
>
> This also simplifies the preempt lock tracking by flattening the nested
> if/else structure into a linear chain: preempt_disable increments,
> preempt_enable checks for underflow and decrements, and the sleepable
> check uses in_sleepable_context() which covers all non-sleepable
> contexts uniformly.
>
> No functional change since in_sleepable_context() checks all the same
> state (active_rcu_locks, active_preempt_locks, active_locks,
> active_irq_id, in_sleepable).
>
> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
> ---
> kernel/bpf/verifier.c | 35 ++++++++++++-----------------------
> 1 file changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e8c4a5f8520d..c26139b96c6a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -14153,34 +14153,23 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> }
> }));
> }
> - } else if (sleepable && env->cur_state->active_rcu_locks) {
> - verbose(env, "kernel func %s is sleepable within rcu_read_lock region\n", func_name);
> - return -EACCES;
> - }
> -
> - if (in_rbtree_lock_required_cb(env) && (rcu_lock || rcu_unlock)) {
> - verbose(env, "Calling bpf_rcu_read_{lock,unlock} in unnecessary rbtree callback\n");
> - return -EACCES;
> - }
> -
> - if (env->cur_state->active_preempt_locks) {
> - if (preempt_disable) {
> - env->cur_state->active_preempt_locks++;
> - } else if (preempt_enable) {
> - env->cur_state->active_preempt_locks--;
> - } else if (sleepable) {
> - verbose(env, "kernel func %s is sleepable within non-preemptible region\n", func_name);
> - return -EACCES;
> - }
> } else if (preempt_disable) {
> env->cur_state->active_preempt_locks++;
> } else if (preempt_enable) {
> - verbose(env, "unmatched attempt to enable preemption (kernel function %s)\n", func_name);
> - return -EINVAL;
> + if (env->cur_state->active_preempt_locks == 0) {
> + verbose(env, "unmatched attempt to enable preemption (kernel function %s)\n",
> + func_name);
> + return -EINVAL;
> + }
> + env->cur_state->active_preempt_locks--;
> + } else if (sleepable && !in_sleepable_context(env)) {
nit: it may be a little bit more readable if we put this check
separately, not in else if, so we get the next structure:
```
if (rcu_lock) {
} else if (rcu_unlock) {
} else if (preempt_disable) {
} else if (preempt_enable) {
}
if (sleepable && !in_sleepable_context(env)) {
}
```
the motivation is that logically this looks separated from the
active_*_lock accounting.
Overall the change looks like an improvement.
Acked-by: Mykyta Yatsenko <yatsenko@meta.com>
> + verbose(env, "kernel func %s is sleepable within %s\n",
> + func_name, non_sleepable_context_description(env));
> + return -EACCES;
> }
>
> - if (env->cur_state->active_irq_id && sleepable) {
> - verbose(env, "kernel func %s is sleepable within IRQ-disabled region\n", func_name);
> + if (in_rbtree_lock_required_cb(env) && (rcu_lock || rcu_unlock)) {
> + verbose(env, "Calling bpf_rcu_read_{lock,unlock} in unnecessary rbtree callback\n");
> return -EACCES;
> }
>
> --
> 2.47.3
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH bpf-next 2/2] bpf: Consolidate sleepable checks in check_kfunc_call()
2026-02-25 13:49 ` [PATCH bpf-next 2/2] bpf: Consolidate sleepable checks in check_kfunc_call() Puranjay Mohan
2026-02-25 18:54 ` Mykyta Yatsenko
@ 2026-02-26 9:18 ` Eduard Zingerman
1 sibling, 0 replies; 6+ messages in thread
From: Eduard Zingerman @ 2026-02-26 9:18 UTC (permalink / raw)
To: Puranjay Mohan, bpf
Cc: Puranjay Mohan, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Martin KaFai Lau, Kumar Kartikeya Dwivedi,
Mykyta Yatsenko, kernel-team
On Wed, 2026-02-25 at 05:49 -0800, Puranjay Mohan wrote:
> check_kfunc_call() has multiple scattered checks that reject sleepable
> kfuncs in various non-sleepable contexts (RCU, preempt-disabled, IRQ-
> disabled). These are the same conditions already checked by
> in_sleepable_context(), so replace them with a single consolidated
> check.
>
> This also simplifies the preempt lock tracking by flattening the nested
> if/else structure into a linear chain: preempt_disable increments,
> preempt_enable checks for underflow and decrements, and the sleepable
> check uses in_sleepable_context() which covers all non-sleepable
> contexts uniformly.
>
> No functional change since in_sleepable_context() checks all the same
> state (active_rcu_locks, active_preempt_locks, active_locks,
> active_irq_id, in_sleepable).
>
> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
> ---
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
[...]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: Consolidate sleepable checks in check_helper_call()
2026-02-25 13:49 [PATCH bpf-next 1/2] bpf: Consolidate sleepable checks in check_helper_call() Puranjay Mohan
2026-02-25 13:49 ` [PATCH bpf-next 2/2] bpf: Consolidate sleepable checks in check_kfunc_call() Puranjay Mohan
@ 2026-02-25 18:30 ` Mykyta Yatsenko
2026-02-26 9:12 ` Eduard Zingerman
2 siblings, 0 replies; 6+ messages in thread
From: Mykyta Yatsenko @ 2026-02-25 18:30 UTC (permalink / raw)
To: Puranjay Mohan, bpf
Cc: Puranjay Mohan, Puranjay Mohan, Alexei Starovoitov,
Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau,
Eduard Zingerman, Kumar Kartikeya Dwivedi, kernel-team
Puranjay Mohan <puranjay@kernel.org> writes:
> check_helper_call() prints the error message for every
> env->cur_state->active* element when calling a sleepable helper.
> Consolidate all of them into a single print statement.
>
> The check for env->cur_state->active_locks was not part of the removed
> print statements and will not be triggered with the consolidated print
> as well because it is checked in do_check() before check_helper_call()
> is even reached.
>
> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
> ---
Acked-by: Mykyta Yatsenko <yatsenko@meta.com>
> kernel/bpf/verifier.c | 40 ++++++++++++++++++----------------------
> 1 file changed, 18 insertions(+), 22 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1153a828ce8d..e8c4a5f8520d 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -11549,6 +11549,19 @@ static inline bool in_sleepable_context(struct bpf_verifier_env *env)
> in_sleepable(env);
> }
>
> +static const char *non_sleepable_context_description(struct bpf_verifier_env *env)
> +{
> + if (env->cur_state->active_rcu_locks)
> + return "rcu_read_lock region";
> + if (env->cur_state->active_preempt_locks)
> + return "non-preemptible region";
> + if (env->cur_state->active_irq_id)
> + return "IRQ-disabled region";
> + if (env->cur_state->active_locks)
> + return "lock region";
> + return "non-sleepable context";
> +}
> +
> static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> int *insn_idx_p)
> {
> @@ -11609,28 +11622,11 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
> return err;
> }
>
> - if (env->cur_state->active_rcu_locks) {
> - if (fn->might_sleep) {
> - verbose(env, "sleepable helper %s#%d in rcu_read_lock region\n",
> - func_id_name(func_id), func_id);
> - return -EINVAL;
> - }
> - }
> -
> - if (env->cur_state->active_preempt_locks) {
> - if (fn->might_sleep) {
> - verbose(env, "sleepable helper %s#%d in non-preemptible region\n",
> - func_id_name(func_id), func_id);
> - return -EINVAL;
> - }
> - }
> -
> - if (env->cur_state->active_irq_id) {
> - if (fn->might_sleep) {
> - verbose(env, "sleepable helper %s#%d in IRQ-disabled region\n",
> - func_id_name(func_id), func_id);
> - return -EINVAL;
> - }
> + if (fn->might_sleep && !in_sleepable_context(env)) {
> + verbose(env, "sleepable helper %s#%d in %s\n",
> + func_id_name(func_id), func_id,
> + non_sleepable_context_description(env));
> + return -EINVAL;
> }
>
> /* Track non-sleepable context for helpers. */
>
> base-commit: f620af11c27b8ec9994a39fe968aa778112d1566
> --
> 2.47.3
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH bpf-next 1/2] bpf: Consolidate sleepable checks in check_helper_call()
2026-02-25 13:49 [PATCH bpf-next 1/2] bpf: Consolidate sleepable checks in check_helper_call() Puranjay Mohan
2026-02-25 13:49 ` [PATCH bpf-next 2/2] bpf: Consolidate sleepable checks in check_kfunc_call() Puranjay Mohan
2026-02-25 18:30 ` [PATCH bpf-next 1/2] bpf: Consolidate sleepable checks in check_helper_call() Mykyta Yatsenko
@ 2026-02-26 9:12 ` Eduard Zingerman
2 siblings, 0 replies; 6+ messages in thread
From: Eduard Zingerman @ 2026-02-26 9:12 UTC (permalink / raw)
To: Puranjay Mohan, bpf
Cc: Puranjay Mohan, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Martin KaFai Lau, Kumar Kartikeya Dwivedi,
Mykyta Yatsenko, kernel-team
On Wed, 2026-02-25 at 05:49 -0800, Puranjay Mohan wrote:
[...]
> @@ -11609,28 +11622,11 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn
> *insn
> return err;
> }
>
> - if (env->cur_state->active_rcu_locks) {
> - if (fn->might_sleep) {
> - verbose(env, "sleepable helper %s#%d in rcu_read_lock region\n",
> - func_id_name(func_id), func_id);
> - return -EINVAL;
> - }
> - }
> -
> - if (env->cur_state->active_preempt_locks) {
> - if (fn->might_sleep) {
> - verbose(env, "sleepable helper %s#%d in non-preemptible region\n",
> - func_id_name(func_id), func_id);
> - return -EINVAL;
> - }
> - }
> -
> - if (env->cur_state->active_irq_id) {
> - if (fn->might_sleep) {
> - verbose(env, "sleepable helper %s#%d in IRQ-disabled region\n",
> - func_id_name(func_id), func_id);
> - return -EINVAL;
> - }
> + if (fn->might_sleep && !in_sleepable_context(env)) {
> + verbose(env, "sleepable helper %s#%d in %s\n",
> + func_id_name(func_id), func_id,
> + non_sleepable_context_description(env));
> + return -EINVAL;
A few checks above there is:
if (!in_sleepable(env) && fn->might_sleep) {
verbose(env, "helper call might sleep in a non-sleepable prog\n");
return -EINVAL;
}
'fn->might_sleep && !in_sleepable_context(env)' covers this case,
do we want to remove it as well?
^ permalink raw reply [flat|nested] 6+ messages in thread