BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next v2 0/2] bpf: Fix loop detection for re-arming async callbacks
@ 2026-09-24 16:26 Puranjay Mohan
  2026-09-24 16:26 ` [PATCH bpf-next v2 1/2] bpf: Look at frame 0 when telling async callback entries apart Puranjay Mohan
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Puranjay Mohan @ 2026-09-24 16:26 UTC (permalink / raw)
  To: bpf
  Cc: Puranjay Mohan, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Song Liu, Yonghong Song

Changelog:
v1: https://lore.kernel.org/all/20260923140152.4005097-1-puranjay@kernel.org/
Changes in v2:
- Patch 1: also fix push_callback_call(), which numbered a new entry
  from the caller frame. When the callback re-arms from a subprog that
  frame is not frame 0 and its count is 0, so every entry was numbered 1,
  is_state_visited() never saw a difference, and a loop in such a
  callback was still rejected.
- Patch 2: loop_cb() now re-arms through a __noinline subprog, so the
  caller frame at bpf_timer_set_callback() is not the callback's own
  frame. The v1 test re-armed from frame 0 and passed without the
  push_callback_call() fix.

push_async_cb() sets in_async_callback_fn and async_entry_cnt on the
callback's own frame, which is frame 0 of the fresh state it starts, and
setup_func_entry() copies neither, so a subprog called by the callback
carries neither. Two places read them from the innermost frame instead.

is_state_visited() skips the infinite loop check when two states differ in
async_entry_cnt, since seeing the same state on a second entry into an
async callback is not a loop. push_callback_call() assigns that count as
the caller's plus one.

A callback which re-arms itself and calls a subprog therefore has its two
entries compared at a loop inside that subprog, where the innermost frame
is the subprog's, and both entries are numbered 1 anyway, so it is
rejected:

  infinite loop detected at insn 57

Patch 1 reads both from frame 0 in both places, which push_async_cb() makes
the callback's frame at any call depth. A loop within a single entry still
has a matching count and is still caught.

Patch 2 covers both directions: a timer callback which re-arms through
bpf_timer_set_callback() from a subprog and reaches a bounded loop through
another one, which fails to load without patch 1, and a callback which
never returns, which must still be rejected either way. Nothing covered an
async callback before, so neither direction was tested.

Puranjay Mohan (2):
  bpf: Look at frame 0 when telling async callback entries apart
  selftests/bpf: Add timer tests for a re-arming callback with a loop

 kernel/bpf/states.c                           |  4 +-
 kernel/bpf/verifier.c                         |  2 +-
 .../testing/selftests/bpf/prog_tests/timer.c  | 33 ++++++++++
 tools/testing/selftests/bpf/progs/timer.c     | 60 ++++++++++++++++++-
 .../selftests/bpf/progs/timer_failure.c       | 29 +++++++++
 5 files changed, 124 insertions(+), 4 deletions(-)


base-commit: 4f3a5eae895b9995e93425a75235d8f1f3268caa
-- 
2.53.0-Meta

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

* [PATCH bpf-next v2 1/2] bpf: Look at frame 0 when telling async callback entries apart
  2026-09-24 16:26 [PATCH bpf-next v2 0/2] bpf: Fix loop detection for re-arming async callbacks Puranjay Mohan
@ 2026-09-24 16:26 ` Puranjay Mohan
  2026-09-24 17:09   ` bot+bpf-ci
  2026-09-24 16:26 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add timer tests for a re-arming callback with a loop Puranjay Mohan
  2026-09-24 21:30 ` [PATCH bpf-next v2 0/2] bpf: Fix loop detection for re-arming async callbacks patchwork-bot+netdevbpf
  2 siblings, 1 reply; 5+ messages in thread
From: Puranjay Mohan @ 2026-09-24 16:26 UTC (permalink / raw)
  To: bpf
  Cc: Puranjay Mohan, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Song Liu, Yonghong Song

push_async_cb() sets in_async_callback_fn and async_entry_cnt on the
callback's own frame, which is frame 0 of the fresh state it starts, and
setup_func_entry() copies neither, so a subprog called by the callback
carries neither. Two places read them from the innermost frame instead.

is_state_visited() skips the infinite loop check when two states differ
in async_entry_cnt, because seeing the same state on a second entry into
an async callback is not a loop. A callback which re-arms itself and
calls a subprog therefore compares the two entries at a loop inside that
subprog, where the innermost frame is the subprog's and has no flag set,
and the state is rejected:

  infinite loop detected at insn 60

push_callback_call() numbers a new entry as the caller's count plus one.
When the callback re-arms itself from a subprog, the caller is that
subprog's frame with a count of 0, so every entry is numbered 1, the
check above never sees a difference, and a loop anywhere in such a
callback is rejected the same way.

Read the flag and the count from frame 0 in both places. A loop within a
single entry still has a matching count and is still caught, and frame 0
of a non-async state does not have the flag set, so nothing else changes.

The two other readers are already correct: check_return_code() reads
frame[0], and the one in do_check() is reached only after an early return
for curframe != 0.

Fixes: bfc6bb74e4f1 ("bpf: Implement verifier support for validation of async callbacks.")
Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
---
 kernel/bpf/states.c   | 4 ++--
 kernel/bpf/verifier.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
index 66fb11b6c6a76..32e141aa6a117 100644
--- a/kernel/bpf/states.c
+++ b/kernel/bpf/states.c
@@ -1273,10 +1273,10 @@ int bpf_is_state_visited(struct bpf_verifier_env *env, int insn_idx)
 			continue;
 
 		if (sl->state.branches) {
-			struct bpf_func_state *frame = sl->state.frame[sl->state.curframe];
+			struct bpf_func_state *frame = sl->state.frame[0];
 
 			if (frame->in_async_callback_fn &&
-			    frame->async_entry_cnt != cur->frame[cur->curframe]->async_entry_cnt) {
+			    frame->async_entry_cnt != cur->frame[0]->async_entry_cnt) {
 				/* Different async_entry_cnt means that the verifier is
 				 * processing another entry into async callback.
 				 * Seeing the same state is not an indication of infinite
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index fec5a1ae6a4da..957ce8692b1a2 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10936,7 +10936,7 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins
 		if (IS_ERR(async_cb))
 			return PTR_ERR(async_cb);
 		callee = async_cb->frame[0];
-		callee->async_entry_cnt = caller->async_entry_cnt + 1;
+		callee->async_entry_cnt = state->frame[0]->async_entry_cnt + 1;
 
 		/* Convert bpf_timer_set_callback() args into timer callback args */
 		err = set_callee_state_cb(env, caller, callee, insn_idx);
-- 
2.53.0-Meta


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

* [PATCH bpf-next v2 2/2] selftests/bpf: Add timer tests for a re-arming callback with a loop
  2026-09-24 16:26 [PATCH bpf-next v2 0/2] bpf: Fix loop detection for re-arming async callbacks Puranjay Mohan
  2026-09-24 16:26 ` [PATCH bpf-next v2 1/2] bpf: Look at frame 0 when telling async callback entries apart Puranjay Mohan
@ 2026-09-24 16:26 ` Puranjay Mohan
  2026-09-24 21:30 ` [PATCH bpf-next v2 0/2] bpf: Fix loop detection for re-arming async callbacks patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: Puranjay Mohan @ 2026-09-24 16:26 UTC (permalink / raw)
  To: bpf
  Cc: Puranjay Mohan, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Song Liu, Yonghong Song

Existing timer callbacks either do not re-arm through
bpf_timer_set_callback(), which is what starts another async callback
entry, or do not reach a loop once they do. Cover that: loop_cb() re-arms
itself and reaches a bounded loop through sum_to(), which is static and
not inlined, so the loop is walked in a frame below the callback's rather
than in a separately verified global subprog.

The re-arm goes through rearm(), also static and not inlined, so the
caller frame at bpf_timer_set_callback() is not the callback's own frame
and carries no async_entry_cnt of its own. That covers both frames the
preceding fix corrects; without it the program is rejected with
"infinite loop detected".

Telling async callback entries apart must not stop the verifier catching
a real loop in a callback, and no existing test covers that either, so
also check that a callback which never returns is still rejected.

Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
---
 .../testing/selftests/bpf/prog_tests/timer.c  | 33 ++++++++++
 tools/testing/selftests/bpf/progs/timer.c     | 60 ++++++++++++++++++-
 .../selftests/bpf/progs/timer_failure.c       | 29 +++++++++
 3 files changed, 121 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/timer.c b/tools/testing/selftests/bpf/prog_tests/timer.c
index 09ff21e1ad2f0..178223190b8b7 100644
--- a/tools/testing/selftests/bpf/prog_tests/timer.c
+++ b/tools/testing/selftests/bpf/prog_tests/timer.c
@@ -262,6 +262,34 @@ static int timer_cancel_async(struct timer *timer_skel)
 	return 0;
 }
 
+/*
+ * A timer callback which re-arms itself and reaches a loop through a call:
+ * the two entries into the callback meet at the loop, in a frame of its own.
+ */
+static int timer_loop_rearm(struct timer *timer_skel)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	int err, prog_fd, i;
+
+	err = timer__attach(timer_skel);
+	if (!ASSERT_OK(err, "timer_attach"))
+		return err;
+
+	timer_skel->bss->loop_rearm = 1;
+
+	prog_fd = bpf_program__fd(timer_skel->progs.test_loop_rearm);
+	err = bpf_prog_test_run_opts(prog_fd, &topts);
+	if (!ASSERT_OK(err, "test_run"))
+		return err;
+
+	for (i = 0; i < 100 && timer_skel->bss->loop_sum < 120 * 2; i++)
+		usleep(1000);
+
+	timer__detach(timer_skel);
+	ASSERT_EQ(timer_skel->bss->loop_sum, 120 * 2, "loop_sum");
+	return 0;
+}
+
 static void test_timer(int (*timer_test_fn)(struct timer *timer_skel))
 {
 	struct timer *timer_skel = NULL;
@@ -287,6 +315,11 @@ void serial_test_timer(void)
 	RUN_TESTS(timer_failure);
 }
 
+void serial_test_timer_loop_rearm(void)
+{
+	test_timer(timer_loop_rearm);
+}
+
 void serial_test_timer_stress(void)
 {
 	test_timer(timer_stress);
diff --git a/tools/testing/selftests/bpf/progs/timer.c b/tools/testing/selftests/bpf/progs/timer.c
index d6d5fefcd9b13..aa966dbed02ea 100644
--- a/tools/testing/selftests/bpf/progs/timer.c
+++ b/tools/testing/selftests/bpf/progs/timer.c
@@ -6,6 +6,7 @@
 #include <errno.h>
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
+#include "bpf_experimental.h"
 
 #define CLOCK_MONOTONIC 1
 #define CLOCK_BOOTTIME 7
@@ -57,9 +58,12 @@ struct {
 	__type(key, int);
 	__type(value, struct elem);
 } abs_timer SEC(".maps"), soft_timer_pinned SEC(".maps"), abs_timer_pinned SEC(".maps"),
-	race_array SEC(".maps");
+	race_array SEC(".maps"), loop_array SEC(".maps");
 
 __u64 bss_data;
+__u64 loop_sum;
+int loop_rearm;		/* number of times loop_cb() re-arms itself */
+__u64 zero;
 __u64 abs_data;
 __u64 err;
 __u64 ok;
@@ -139,6 +143,60 @@ static int timer_cb1(void *map, int *key, struct bpf_timer *timer)
 	return 0;
 }
 
+/*
+ * Static and not inlined, so the loop is walked in a frame below the
+ * callback's rather than in a separately verified global subprog.
+ */
+static __noinline int sum_to(__u64 n)
+{
+	__u64 i, sum = 0;
+
+	for (i = zero; i < n && can_loop; i++)
+		sum += i;
+
+	return sum;
+}
+
+static int loop_cb(void *map, int *key, struct elem *val);
+
+/*
+ * Re-arms from a frame below the callback's, where the caller frame carries
+ * no async_entry_cnt of its own.
+ */
+static __noinline void rearm(struct elem *val)
+{
+	if (loop_rearm > 0) {
+		loop_rearm--;
+		/* set_callback is what starts another async callback entry */
+		bpf_timer_set_callback(&val->t, loop_cb);
+		bpf_timer_start(&val->t, 0, 0);
+	}
+}
+
+/* Re-arms itself and reaches a bounded loop through a call. */
+static int loop_cb(void *map, int *key, struct elem *val)
+{
+	loop_sum += sum_to(16);
+	rearm(val);
+	return 0;
+}
+
+SEC("fentry/bpf_fentry_test1")
+int BPF_PROG2(test_loop_rearm, int, a)
+{
+	struct bpf_timer *timer;
+	int key = 0;
+
+	timer = bpf_map_lookup_elem(&loop_array, &key);
+	if (!timer)
+		return 0;
+
+	bpf_timer_init(timer, &loop_array, CLOCK_MONOTONIC);
+	bpf_timer_set_callback(timer, loop_cb);
+	bpf_timer_start(timer, 0, 0);
+	return 0;
+}
+
 SEC("fentry/bpf_fentry_test1")
 int BPF_PROG2(test1, int, a)
 {
diff --git a/tools/testing/selftests/bpf/progs/timer_failure.c b/tools/testing/selftests/bpf/progs/timer_failure.c
index 5a2e9dabf1c6c..0538269101cac 100644
--- a/tools/testing/selftests/bpf/progs/timer_failure.c
+++ b/tools/testing/selftests/bpf/progs/timer_failure.c
@@ -66,3 +66,32 @@ long BPF_PROG2(test_bad_ret, int, a)
 
 	return 0;
 }
+
+/*
+ * A real loop inside an async callback must still be rejected: both entries
+ * into the callback have the same async_entry_cnt, so telling entries apart
+ * does not apply here.
+ */
+static int timer_cb_infinite_loop(void *map, int *key, struct elem *val)
+{
+	for (;;) {}
+
+	return 0;
+}
+
+SEC("fentry/bpf_fentry_test1")
+__failure __msg("infinite loop detected")
+long BPF_PROG2(test_infinite_loop_cb, int, a)
+{
+	struct bpf_timer *timer;
+	int key = 0;
+
+	timer = bpf_map_lookup_elem(&timer_map, &key);
+	if (timer) {
+		bpf_timer_init(timer, &timer_map, CLOCK_BOOTTIME);
+		bpf_timer_set_callback(timer, timer_cb_infinite_loop);
+		bpf_timer_start(timer, 1000, 0);
+	}
+
+	return 0;
+}
-- 
2.53.0-Meta


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

* Re: [PATCH bpf-next v2 1/2] bpf: Look at frame 0 when telling async callback entries apart
  2026-09-24 16:26 ` [PATCH bpf-next v2 1/2] bpf: Look at frame 0 when telling async callback entries apart Puranjay Mohan
@ 2026-09-24 17:09   ` bot+bpf-ci
  0 siblings, 0 replies; 5+ messages in thread
From: bot+bpf-ci @ 2026-09-24 17:09 UTC (permalink / raw)
  To: puranjay, bpf
  Cc: puranjay, ast, daniel, andrii, martin.lau, eddyz87, memxor, song,
	yonghong.song, ast, andrii, daniel, martin.lau, eddyz87,
	yonghong.song, mason, ihor.solodrai

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

> push_async_cb() sets in_async_callback_fn and async_entry_cnt on the
> callback's own frame, which is frame 0 of the fresh state it starts, and
> setup_func_entry() copies neither, so a subprog called by the callback
> carries neither.

Is the attribution here accurate? Looking at the verifier code,
push_async_cb() doesn't directly set either field. It kzalloc()s frame 0
and calls init_func_state() on it, which initializes callsite, frameno,
subprogno, callback_ret_range and registers.

async_entry_cnt is actually set by push_callback_call() after
push_async_cb() returns:

    callee = async_cb->frame[0];
    callee->async_entry_cnt = state->frame[0]->async_entry_cnt + 1;

And in_async_callback_fn is set by the set_callee_state_cb() callbacks:
set_timer_callback_state(), set_task_work_schedule_callback_state() and
set_rcu_callback_state(), each with:

    callee->in_async_callback_fn = true;

> The two other readers are already correct: check_return_code() reads
> frame[0], and the one in do_check() is reached only after an early return
> for curframe != 0.

Doesn't this leave out print_verifier_state()? In kernel/bpf/log.c, it reads
the flag:

    if (state->in_async_callback_fn)
        verbose(env, " async_cb");

print_verifier_state() only affects log output and reads the flag from the
frame it is printing, so leaving it unchanged is fine. The reader credited to
do_check() is actually in process_bpf_exit_full(), which do_check_insn()
calls.

Would naming the functions that actually set the fields
(push_callback_call() and the set_*_callback_state() callbacks) and covering
the log.c reader make the changelog more accurate? The conclusions in the
message hold, and the fix itself is correct, but for a subtle verifier change
it would help to have the attribution match the code.


---
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/36029511690

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

* Re: [PATCH bpf-next v2 0/2] bpf: Fix loop detection for re-arming async callbacks
  2026-09-24 16:26 [PATCH bpf-next v2 0/2] bpf: Fix loop detection for re-arming async callbacks Puranjay Mohan
  2026-09-24 16:26 ` [PATCH bpf-next v2 1/2] bpf: Look at frame 0 when telling async callback entries apart Puranjay Mohan
  2026-09-24 16:26 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add timer tests for a re-arming callback with a loop Puranjay Mohan
@ 2026-09-24 21:30 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-24 21:30 UTC (permalink / raw)
  To: Puranjay Mohan
  Cc: bpf, ast, daniel, andrii, martin.lau, eddyz87, memxor, song,
	yonghong.song

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Thu, 24 Sep 2026 09:26:05 -0700 you wrote:
> Changelog:
> v1: https://lore.kernel.org/all/20260923140152.4005097-1-puranjay@kernel.org/
> Changes in v2:
> - Patch 1: also fix push_callback_call(), which numbered a new entry
>   from the caller frame. When the callback re-arms from a subprog that
>   frame is not frame 0 and its count is 0, so every entry was numbered 1,
>   is_state_visited() never saw a difference, and a loop in such a
>   callback was still rejected.
> - Patch 2: loop_cb() now re-arms through a __noinline subprog, so the
>   caller frame at bpf_timer_set_callback() is not the callback's own
>   frame. The v1 test re-armed from frame 0 and passed without the
>   push_callback_call() fix.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v2,1/2] bpf: Look at frame 0 when telling async callback entries apart
    https://git.kernel.org/bpf/bpf-next/c/e55220162aae
  - [bpf-next,v2,2/2] selftests/bpf: Add timer tests for a re-arming callback with a loop
    https://git.kernel.org/bpf/bpf-next/c/84bff37db2ad

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2026-09-24 21:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-24 16:26 [PATCH bpf-next v2 0/2] bpf: Fix loop detection for re-arming async callbacks Puranjay Mohan
2026-09-24 16:26 ` [PATCH bpf-next v2 1/2] bpf: Look at frame 0 when telling async callback entries apart Puranjay Mohan
2026-09-24 17:09   ` bot+bpf-ci
2026-09-24 16:26 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add timer tests for a re-arming callback with a loop Puranjay Mohan
2026-09-24 21:30 ` [PATCH bpf-next v2 0/2] bpf: Fix loop detection for re-arming async callbacks patchwork-bot+netdevbpf

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