public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf v1 1/2] bpf: Fix exception exit lock checking for subprogs
@ 2026-03-17  0:13 Ihor Solodrai
  2026-03-17  0:13 ` [PATCH bpf v1 2/2] selftests/bpf: Add tests for bpf_throw lock leak from subprogs Ihor Solodrai
  2026-03-17  1:02 ` [PATCH bpf v1 1/2] bpf: Fix exception exit lock checking for subprogs bot+bpf-ci
  0 siblings, 2 replies; 4+ messages in thread
From: Ihor Solodrai @ 2026-03-17  0:13 UTC (permalink / raw)
  To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Eduard Zingerman, Martin KaFai Lau
  Cc: Kumar Kartikeya Dwivedi, bpf, linux-kernel, kernel-team

process_bpf_exit_full() passes check_lock = !curframe to
check_resource_leak(), which is false in cases when bpf_throw() is
called from a static subprog. This makes check_resource_leak() to skip
validation of active_rcu_locks, active_preempt_locks, and
active_irq_id on exception exits from subprogs.

At runtime bpf_throw() unwinds the stack via ORC without releasing any
user-acquired locks, which may cause various issues as the result.

Fix by setting check_lock = true for exception exits regardless of
curframe, since exceptions bypass all intermediate frame
cleanup. Update the error message prefix to "bpf_throw" for exception
exits to distinguish them from normal BPF_EXIT. Update relevant
existing selftests to match the new message.

The spin_lock case is not affected because they are already checked [1]
at the call site in do_check_insn() before bpf_throw can run.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/bpf/verifier.c?h=v7.0-rc4#n21098

Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>

---

This bug was discovered by an AI bot. You can find the original
AI-generated investigation here:
https://github.com/theihor/bpf/commits/ai-tinkering-20260310-0745/

---
 kernel/bpf/verifier.c                               | 3 ++-
 tools/testing/selftests/bpf/progs/exceptions_fail.c | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index df22bfc572e2..5c0e6809024f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -20911,7 +20911,8 @@ static int process_bpf_exit_full(struct bpf_verifier_env *env,
 	 * state when it exits.
 	 */
 	int err = check_resource_leak(env, exception_exit,
-				      !env->cur_state->curframe,
+				      exception_exit || !env->cur_state->curframe,
+				      exception_exit ? "bpf_throw" :
 				      "BPF_EXIT instruction in main prog");
 	if (err)
 		return err;
diff --git a/tools/testing/selftests/bpf/progs/exceptions_fail.c b/tools/testing/selftests/bpf/progs/exceptions_fail.c
index 8a0fdff89927..1dd2703c7676 100644
--- a/tools/testing/selftests/bpf/progs/exceptions_fail.c
+++ b/tools/testing/selftests/bpf/progs/exceptions_fail.c
@@ -131,7 +131,7 @@ int reject_subprog_with_lock(void *ctx)
 }
 
 SEC("?tc")
-__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_rcu_read_lock-ed region")
+__failure __msg("bpf_throw cannot be used inside bpf_rcu_read_lock-ed region")
 int reject_with_rcu_read_lock(void *ctx)
 {
 	bpf_rcu_read_lock();
@@ -147,7 +147,7 @@ __noinline static int throwing_subprog(struct __sk_buff *ctx)
 }
 
 SEC("?tc")
-__failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_rcu_read_lock-ed region")
+__failure __msg("bpf_throw cannot be used inside bpf_rcu_read_lock-ed region")
 int reject_subprog_with_rcu_read_lock(void *ctx)
 {
 	bpf_rcu_read_lock();
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH bpf v1 2/2] selftests/bpf: Add tests for bpf_throw lock leak from subprogs
  2026-03-17  0:13 [PATCH bpf v1 1/2] bpf: Fix exception exit lock checking for subprogs Ihor Solodrai
@ 2026-03-17  0:13 ` Ihor Solodrai
  2026-03-17  1:02 ` [PATCH bpf v1 1/2] bpf: Fix exception exit lock checking for subprogs bot+bpf-ci
  1 sibling, 0 replies; 4+ messages in thread
From: Ihor Solodrai @ 2026-03-17  0:13 UTC (permalink / raw)
  To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Eduard Zingerman, Martin KaFai Lau
  Cc: Kumar Kartikeya Dwivedi, bpf, linux-kernel, kernel-team

Add test cases to ensure the verifier correctly rejects bpf_throw from
subprogs when RCU or preempt locks are held:

  * reject_subprog_throw_rcu_lock: always-throwing subprog called while
    caller holds bpf_rcu_read_lock
  * reject_subprog_rcu_lock_throw: subprog acquires bpf_rcu_read_lock and
    then calls bpf_throw
  * reject_subprog_throw_preempt_lock: always-throwing subprog called while
    caller holds bpf_preempt_disable

Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 .../selftests/bpf/progs/exceptions_fail.c     | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/exceptions_fail.c b/tools/testing/selftests/bpf/progs/exceptions_fail.c
index 1dd2703c7676..9d1c560aac84 100644
--- a/tools/testing/selftests/bpf/progs/exceptions_fail.c
+++ b/tools/testing/selftests/bpf/progs/exceptions_fail.c
@@ -8,6 +8,9 @@
 #include "bpf_experimental.h"
 
 extern void bpf_rcu_read_lock(void) __ksym;
+extern void bpf_rcu_read_unlock(void) __ksym;
+extern void bpf_preempt_disable(void) __ksym;
+extern void bpf_preempt_enable(void) __ksym;
 
 #define private(name) SEC(".bss." #name) __hidden __attribute__((aligned(8)))
 
@@ -346,4 +349,45 @@ int reject_exception_throw_cb_diff(struct __sk_buff *ctx)
 	return 0;
 }
 
+__noinline static int always_throws(void)
+{
+	bpf_throw(0);
+	return 0;
+}
+
+SEC("?tc")
+__failure __msg("bpf_throw cannot be used inside bpf_rcu_read_lock-ed region")
+int reject_subprog_throw_rcu_lock(void *ctx)
+{
+	bpf_rcu_read_lock();
+	always_throws();
+	bpf_rcu_read_unlock();
+	return 0;
+}
+
+__noinline static int rcu_lock_then_throw(void)
+{
+	bpf_rcu_read_lock();
+	bpf_throw(0);
+	return 0;
+}
+
+SEC("?tc")
+__failure __msg("bpf_throw cannot be used inside bpf_rcu_read_lock-ed region")
+int reject_subprog_rcu_lock_throw(void *ctx)
+{
+	rcu_lock_then_throw();
+	return 0;
+}
+
+SEC("?tc")
+__failure __msg("bpf_throw cannot be used inside bpf_preempt_disable-ed region")
+int reject_subprog_throw_preempt_lock(void *ctx)
+{
+	bpf_preempt_disable();
+	always_throws();
+	bpf_preempt_enable();
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf v1 1/2] bpf: Fix exception exit lock checking for subprogs
  2026-03-17  0:13 [PATCH bpf v1 1/2] bpf: Fix exception exit lock checking for subprogs Ihor Solodrai
  2026-03-17  0:13 ` [PATCH bpf v1 2/2] selftests/bpf: Add tests for bpf_throw lock leak from subprogs Ihor Solodrai
@ 2026-03-17  1:02 ` bot+bpf-ci
  2026-03-17  1:22   ` Kumar Kartikeya Dwivedi
  1 sibling, 1 reply; 4+ messages in thread
From: bot+bpf-ci @ 2026-03-17  1:02 UTC (permalink / raw)
  To: ihor.solodrai, ast, andrii, daniel, eddyz87, martin.lau
  Cc: memxor, bpf, linux-kernel, kernel-team, ast, andrii, daniel,
	martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 1114 bytes --]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -20911,7 +20911,8 @@ static int process_bpf_exit_full(struct bpf_verifier_env *env,
>  	 * state when it exits.
>  	 */
>  	int err = check_resource_leak(env, exception_exit,
> -				      !env->cur_state->curframe,
> +				      exception_exit || !env->cur_state->curframe,
> +				      exception_exit ? "bpf_throw" :
>  				      "BPF_EXIT instruction in main prog");
>  	if (err)
>  		return err;

This looks like a bug fix but appears to be missing a Fixes: tag.
The buggy check_lock = !curframe logic was introduced when the
inline code in do_check() was refactored into process_bpf_exit_full()
and check_resource_leak() in commit 2d419c44658f ("bpf: add fsession
support"). Should this have:

Fixes: 2d419c44658f ("bpf: add fsession support")


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/23172639489

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf v1 1/2] bpf: Fix exception exit lock checking for subprogs
  2026-03-17  1:02 ` [PATCH bpf v1 1/2] bpf: Fix exception exit lock checking for subprogs bot+bpf-ci
@ 2026-03-17  1:22   ` Kumar Kartikeya Dwivedi
  0 siblings, 0 replies; 4+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-03-17  1:22 UTC (permalink / raw)
  To: bot+bpf-ci
  Cc: ihor.solodrai, ast, andrii, daniel, eddyz87, martin.lau, bpf,
	linux-kernel, kernel-team, martin.lau, yonghong.song, clm

On Tue, 17 Mar 2026 at 02:02, <bot+bpf-ci@kernel.org> wrote:
>
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -20911,7 +20911,8 @@ static int process_bpf_exit_full(struct bpf_verifier_env *env,
> >        * state when it exits.
> >        */
> >       int err = check_resource_leak(env, exception_exit,
> > -                                   !env->cur_state->curframe,
> > +                                   exception_exit || !env->cur_state->curframe,
> > +                                   exception_exit ? "bpf_throw" :
> >                                     "BPF_EXIT instruction in main prog");

Fix LGTM. Thanks.

> >       if (err)
> >               return err;
>
> This looks like a bug fix but appears to be missing a Fixes: tag.
> The buggy check_lock = !curframe logic was introduced when the
> inline code in do_check() was refactored into process_bpf_exit_full()
> and check_resource_leak() in commit 2d419c44658f ("bpf: add fsession
> support"). Should this have:
>
> Fixes: 2d419c44658f ("bpf: add fsession support")
>

The fixes tag is wrong. It goes all the way back to f18b03fabaa9b.
I was scratching my head for a while, since we already have one test
to prevent this, but it turns out it's broken.
If we look at test reject_subprog_with_rcu_read_lock, it made the
mistake of returning directly.
Since the throw validation wasn't correct, the error instead comes
because of not closing the RCU section and exiting the program.
If we do the unlock without returning directly it starts failing
(i.e., unexpectedly succeeding).

I would rather change that test to fix it here in this commit, then
add more cases for IRQ, preempt lock etc. around it in the next one to
increase coverage.
With that the commit log could be made more clearer.

pw-bot: cr

>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/23172639489

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-03-17  1:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-17  0:13 [PATCH bpf v1 1/2] bpf: Fix exception exit lock checking for subprogs Ihor Solodrai
2026-03-17  0:13 ` [PATCH bpf v1 2/2] selftests/bpf: Add tests for bpf_throw lock leak from subprogs Ihor Solodrai
2026-03-17  1:02 ` [PATCH bpf v1 1/2] bpf: Fix exception exit lock checking for subprogs bot+bpf-ci
2026-03-17  1:22   ` Kumar Kartikeya Dwivedi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox