[PATCH bpf v1 0/8] Misc bug fixes - part 5

[PATCH bpf v1 0/8] Misc bug fixes - part 5

[Kumar Kartikeya Dwivedi]

A set of miscellaneous fixes for bugs reported by Nicholas. See commit
logs for details.

Kumar Kartikeya Dwivedi (8):
  bpf: Make post-verification instruction rewrites killable
  selftests/bpf: Test killing a loader during instruction rewrites
  bpf: Preserve packet pointer class displacement in regsafe()
  selftests/bpf: Test packet pointer class displacement pruning
  bpf: Reject fall-through across subprogram boundaries
  selftests/bpf: Test poisoned subprogram terminator
  bpf: Assign lock identity to callback map values
  selftests/bpf: Check callback map value lock identity

 include/linux/bpf_verifier.h                  |   2 +
 kernel/bpf/cfg.c                              |  21 +++
 kernel/bpf/fixups.c                           |  21 ++-
 kernel/bpf/states.c                           |  42 +++++-
 kernel/bpf/verifier.c                         |  14 ++
 .../selftests/bpf/prog_tests/core_reloc_raw.c | 113 +++++++++++++++
 .../bpf/prog_tests/prog_load_signal.c         | 131 ++++++++++++++++++
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../bpf/progs/verifier_callback_lock.c        |  73 ++++++++++
 .../progs/verifier_xdp_direct_packet_access.c |  44 ++++++
 10 files changed, 460 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/prog_load_signal.c
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_callback_lock.c


base-commit: b75a000f2ac15f4778ddd6d9298d60b24ad776fa
-- 
2.53.0

[PATCH bpf v1 1/8] bpf: Make post-verification instruction rewrites killable

[Kumar Kartikeya Dwivedi]

After do_check() returns, the verifier runs several instruction rewrite
passes. Some of them patch or remove one instruction at a time. Each
operation moves the remaining instruction and auxiliary-data arrays and
adjusts all branch offsets, making the overall work quadratic in the
program length.

A privileged loader can submit 131072 unconditional jumps by zero followed
by a valid return. Verification finishes quickly, but bpf_opt_remove_nops()
then spends a long time removing each jump separately. Since this
post-verification work neither checks for signals nor reschedules, a pending
SIGKILL cannot terminate the task until the rewrite finishes.

Make bpf_patch_insn_data() and verifier_remove_insns() common cancellation
and rescheduling points. These helpers run from BPF_PROG_LOAD process
context, and bpf_patch_insn_data() can already sleep while reallocating
auxiliary data. Callers already handle NULL or propagate an error, so a
fatal signal can abort without leaving a partially accepted program visible.

This does not reduce the quadratic cost of the rewrite passes, but it makes
the work preemptible and allows a killed loader to be torn down promptly.

Fixes: 52875a04f4b2 ("bpf: verifier: remove dead code")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/fixups.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
index 52d3cec33672..9401fffcedfd 100644
--- a/kernel/bpf/fixups.c
+++ b/kernel/bpf/fixups.c
@@ -8,6 +8,7 @@
 #include <linux/bsearch.h>
 #include <linux/sort.h>
 #include <linux/perf_event.h>
+#include <linux/sched/signal.h>
 #include <net/xdp.h>
 #include "disasm.h"
 
@@ -306,12 +307,28 @@ static void adjust_poke_descs(struct bpf_prog *prog, u32 off, u32 len)
 	}
 }
 
+/*
+ * Some post-verification instruction rewriting passes require an
+ * O(prog->len) operation per instruction. Keep their shared primitives
+ * killable and preemptible.
+ */
+static bool bpf_rewrite_must_abort(void)
+{
+	if (fatal_signal_pending(current))
+		return true;
+	cond_resched();
+	return false;
+}
+
 struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 off,
 				     const struct bpf_insn *patch, u32 len)
 {
 	struct bpf_prog *new_prog;
 	struct bpf_insn_aux_data *new_data = NULL;
 
+	if (bpf_rewrite_must_abort())
+		return NULL;
+
 	if (len > 1) {
 		new_data = vrealloc(env->insn_aux_data,
 				    array_size(env->prog->len + len - 1,
@@ -523,6 +540,9 @@ static int verifier_remove_insns(struct bpf_verifier_env *env, u32 off, u32 cnt)
 	unsigned int orig_prog_len = env->prog->len;
 	int err;
 
+	if (bpf_rewrite_must_abort())
+		return -EINTR;
+
 	if (bpf_prog_is_offloaded(env->prog->aux))
 		bpf_prog_offload_remove_insns(env, off, cnt);
 
@@ -2666,4 +2686,3 @@ int bpf_remove_fastcall_spills_fills(struct bpf_verifier_env *env)
 
 	return 0;
 }
-
-- 
2.53.0

Re: [PATCH bpf v1 1/8] bpf: Make post-verification instruction rewrites killable

[bot+bpf-ci]

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

> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 52d3cec33672..9401fffcedfd 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c

[ ... ]

> @@ -306,12 +307,28 @@ static void adjust_poke_descs(struct bpf_prog *prog, u32 off, u32 len)
>  	}
>  }
>  
> +/*
> + * Some post-verification instruction rewriting passes require an
> + * O(prog->len) operation per instruction. Keep their shared primitives
> + * killable and preemptible.
> + */
> +static bool bpf_rewrite_must_abort(void)
> +{
> +	if (fatal_signal_pending(current))
> +		return true;
> +	cond_resched();
> +	return false;
> +}
> +
>  struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 off,
>  				     const struct bpf_insn *patch, u32 len)
>  {
>  	struct bpf_prog *new_prog;
>  	struct bpf_insn_aux_data *new_data = NULL;
>  
> +	if (bpf_rewrite_must_abort())
> +		return NULL;
> +

Is this NULL return actually a cancellation point for every caller of
bpf_patch_insn_data()?

Besides the verifier rewrite passes, bpf_patch_insn_data() is also reached
from bpf_jit_blind_constants(), and on that path a NULL return is
deliberately treated as non-fatal.

bpf_jit_subprogs() maps the blinding failure to -ENOMEM and clears
jit_requested/blinding_requested:

kernel/bpf/fixups.c:bpf_jit_subprogs() {
	...
	prog = bpf_jit_blind_constants(env, prog);
	if (IS_ERR(prog)) {
		err = -ENOMEM;
		prog = orig_prog;
		goto out_restore;
	}
	...
out_cleanup:
	/* cleanup main prog to be interpreted */
	prog->jit_requested = 0;
	prog->blinding_requested = 0;
	return err;
}

but bpf_fixup_call_args() only propagates 0 and -EFAULT out of it:

kernel/bpf/fixups.c:bpf_fixup_call_args() {
	if (env->prog->jit_requested &&
	    !bpf_prog_is_offloaded(env->prog->aux)) {
		err = bpf_jit_subprogs(env);
		if (err == 0)
			return 0;
		if (err == -EFAULT)
			return err;
	}
#ifndef CONFIG_BPF_JIT_ALWAYS_ON
	...
	err = 0;
#endif
	return err;
}

so with CONFIG_BPF_JIT_ALWAYS_ON=n the -ENOMEM is overwritten by the
interpreter-fallback path and bpf_check() carries on.

The other caller documents the same policy:

kernel/bpf/core.c:bpf_prog_jit_compile() {
	prog = bpf_jit_blind_constants(env, prog);
	/*
	 * If blinding was requested and we failed during blinding, we must fall
	 * back to the interpreter.
	 */
	if (IS_ERR(prog))
		goto out_restore;
}

and __bpf_prog_select_runtime() only turns that into an error when
fp->jit_required is set.

So if the fatal signal only becomes pending after bpf_do_misc_fixups() and
bpf_opt_subreg_zext_lo32_rnd_hi32() have run, that is during
bpf_fixup_call_args() or the later __bpf_prog_select_runtime() call,
BPF_PROG_LOAD still returns 0 with the jit and constant blinding silently
skipped.  A load under net.core.bpf_jit_harden can then end up unblinded
rather than rejected.

The changelog says:

>     Callers already handle NULL or propagate an error, so a
>     fatal signal can abort without leaving a partially accepted program visible.

Should that be narrowed to the verifier rewrite passes, or would it be
better to keep the abort check out of a helper that the constant blinding
path also uses?

[ ... ]


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

[PATCH bpf v1 2/8] selftests/bpf: Test killing a loader during instruction rewrites

[Kumar Kartikeya Dwivedi]

Exercise fatal-signal handling after verifier exploration has completed. Load
a 32768-instruction control program to estimate the time needed for linear
verification, then load the same-sized program made of unconditional jumps by
zero in a child process.

The latter reaches the quadratic bpf_opt_remove_nops() rewrite. Send SIGKILL
after four control-load durations and require the child to be reaped within
one second. Without cancellation points in the rewrite helpers, the killed
child stays in BPF_PROG_LOAD until all no-ops have been removed.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../bpf/prog_tests/prog_load_signal.c         | 131 ++++++++++++++++++
 1 file changed, 131 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/prog_load_signal.c

diff --git a/tools/testing/selftests/bpf/prog_tests/prog_load_signal.c b/tools/testing/selftests/bpf/prog_tests/prog_load_signal.c
new file mode 100644
index 000000000000..0f78db3bccc7
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/prog_load_signal.c
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+#include <test_progs.h>
+
+#define NOP_CNT		32768
+#define MIN_KILL_DELAY_NS	(100ULL * 1000 * 1000)
+#define REAP_TIMEOUT_NS		(1000ULL * 1000 * 1000)
+
+static __u64 monotonic_ns(void)
+{
+	struct timespec ts;
+
+	clock_gettime(CLOCK_MONOTONIC, &ts);
+	return ts.tv_sec * 1000000000ULL + ts.tv_nsec;
+}
+
+static void sleep_ns(__u64 duration)
+{
+	struct timespec ts = {
+		.tv_sec = duration / 1000000000ULL,
+		.tv_nsec = duration % 1000000000ULL,
+	};
+
+	while (nanosleep(&ts, &ts) && errno == EINTR)
+		;
+}
+
+static int waitpid_timeout(pid_t pid, int *status, __u64 timeout)
+{
+	__u64 deadline = monotonic_ns() + timeout;
+	int ret;
+
+	do {
+		ret = waitpid(pid, status, WNOHANG);
+		if (ret)
+			return ret;
+		usleep(1000);
+	} while (monotonic_ns() < deadline);
+
+	return 0;
+}
+
+void test_prog_load_signal(void)
+{
+	struct bpf_insn *insns = NULL;
+	__u64 start, control_time, kill_delay;
+	int pipefd[2] = { -1, -1 };
+	int prog_fd = -1, status = 0;
+	pid_t pid = -1;
+	char byte;
+	int i, ret;
+
+	insns = calloc(NOP_CNT + 2, sizeof(*insns));
+	if (!ASSERT_OK_PTR(insns, "calloc"))
+		return;
+
+	for (i = 0; i < NOP_CNT; i++)
+		insns[i] = BPF_MOV64_REG(BPF_REG_1, BPF_REG_1);
+	insns[NOP_CNT] = BPF_MOV64_IMM(BPF_REG_0, 0);
+	insns[NOP_CNT + 1] = BPF_EXIT_INSN();
+
+	start = monotonic_ns();
+	prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, NULL, "GPL",
+				insns, NOP_CNT + 2, NULL);
+	control_time = monotonic_ns() - start;
+	if (!ASSERT_GE(prog_fd, 0, "control_prog_load"))
+		goto cleanup;
+	close(prog_fd);
+	prog_fd = -1;
+
+	for (i = 0; i < NOP_CNT; i++)
+		insns[i] = BPF_JMP_IMM(BPF_JA, 0, 0, 0);
+
+	if (!ASSERT_OK(pipe(pipefd), "pipe"))
+		goto cleanup;
+
+	pid = fork();
+	if (!ASSERT_GE(pid, 0, "fork"))
+		goto cleanup;
+	if (!pid) {
+		close(pipefd[0]);
+		if (write(pipefd[1], "x", 1) != 1)
+			_exit(1);
+		close(pipefd[1]);
+		prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, NULL, "GPL",
+					insns, NOP_CNT + 2, NULL);
+		if (prog_fd >= 0)
+			close(prog_fd);
+		_exit(prog_fd < 0);
+	}
+
+	close(pipefd[1]);
+	pipefd[1] = -1;
+	ret = read(pipefd[0], &byte, 1);
+	if (!ASSERT_EQ(ret, 1, "child_ready"))
+		goto cleanup;
+
+	/*
+	 * Allow linear verification to finish before sending SIGKILL. The nop
+	 * removal pass is quadratic, so four control-load times still leaves a
+	 * wide window in which an affected kernel is rewriting instructions.
+	 */
+	kill_delay = MAX(control_time * 4, MIN_KILL_DELAY_NS);
+	sleep_ns(kill_delay);
+	if (!ASSERT_OK(kill(pid, SIGKILL), "kill"))
+		goto cleanup;
+
+	start = monotonic_ns();
+	ret = waitpid_timeout(pid, &status, REAP_TIMEOUT_NS);
+	if (!ASSERT_EQ(ret, pid, "prog_load_killable")) {
+		fprintf(stderr, "control load %llu us, child still alive %llu us after SIGKILL\n",
+			control_time / 1000, (monotonic_ns() - start) / 1000);
+		goto cleanup;
+	}
+	pid = -1;
+	ASSERT_TRUE(WIFSIGNALED(status) && WTERMSIG(status) == SIGKILL,
+		    "killed_by_sigkill");
+
+cleanup:
+	if (prog_fd >= 0)
+		close(prog_fd);
+	if (pipefd[0] >= 0)
+		close(pipefd[0]);
+	if (pipefd[1] >= 0)
+		close(pipefd[1]);
+	if (pid > 0) {
+		kill(pid, SIGKILL);
+		waitpid(pid, &status, 0);
+	}
+	free(insns);
+}
-- 
2.53.0

Re: [PATCH bpf v1 2/8] selftests/bpf: Test killing a loader during instruction rewrites

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Race condition causing false test failures on extremely fast hardware or optimized verifiers.
--

commit e86296a4aae66b4d0778df9e0379e5e9bc8a6bff
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Subject: selftests/bpf: Test killing a loader during instruction rewrites

This commit adds a selftest to verify that a child process stuck in the
verifier's quadratic instruction rewrite phase can be successfully terminated
by a fatal signal.

> diff --git a/tools/testing/selftests/bpf/prog_tests/prog_load_signal.c b/tools/testing/selftests/bpf/prog_tests/prog_load_signal.c
> new file mode 100644
> index 0000000000000..0f78db3bccc76
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/prog_load_signal.c
[ ... ]
> +	if (!pid) {
> +		close(pipefd[0]);
> +		if (write(pipefd[1], "x", 1) != 1)
> +			_exit(1);
> +		close(pipefd[1]);
> +		prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, NULL, "GPL",
> +					insns, NOP_CNT + 2, NULL);
> +		if (prog_fd >= 0)
> +			close(prog_fd);
> +		_exit(prog_fd < 0);
> +	}

[Severity: Medium]
Could the child process exit too quickly on very fast hardware or with an
optimized verifier? If the child completes the load before the parent sends
SIGKILL, it will exit normally and become a zombie.

[ ... ]
> +	/*
> +	 * Allow linear verification to finish before sending SIGKILL. The nop
> +	 * removal pass is quadratic, so four control-load times still leaves a
> +	 * wide window in which an affected kernel is rewriting instructions.
> +	 */
> +	kill_delay = MAX(control_time * 4, MIN_KILL_DELAY_NS);
> +	sleep_ns(kill_delay);

Here, kill_delay enforces a minimum delay of 100ms (MIN_KILL_DELAY_NS). Is it
possible that on modern CPUs with high cache bandwidth, the verifier's 32,768
instruction rewrite could finish in under 100ms?

If control_time is very small, the parent will sleep for 100ms, which might
give the child ample time to finish bpf_prog_load() and exit.

[ ... ]
> +	start = monotonic_ns();
> +	ret = waitpid_timeout(pid, &status, REAP_TIMEOUT_NS);
> +	if (!ASSERT_EQ(ret, pid, "prog_load_killable")) {
> +		fprintf(stderr, "control load %llu us, child still alive %llu us after SIGKILL\n",
> +			control_time / 1000, (monotonic_ns() - start) / 1000);
> +		goto cleanup;
> +	}
> +	pid = -1;
> +	ASSERT_TRUE(WIFSIGNALED(status) && WTERMSIG(status) == SIGKILL,
> +		    "killed_by_sigkill");

If the child finished early and exited normally as described above, waitpid
will retrieve a normal exit status. This assertion will then incorrectly fail
because the child wasn't killed by the signal.

Could this lead to flaky test failures on fast CI machines?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905070003.3193366-1-memxor@gmail.com?part=2

Re: [PATCH bpf v1 2/8] selftests/bpf: Test killing a loader during instruction rewrites

[Kumar Kartikeya Dwivedi]

On Sat Sep 5, 2026 at 9:13 AM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Race condition causing false test failures on extremely fast hardware or optimized verifiers.
> --
>
> commit e86296a4aae66b4d0778df9e0379e5e9bc8a6bff
> Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> Subject: selftests/bpf: Test killing a loader during instruction rewrites
>
> This commit adds a selftest to verify that a child process stuck in the
> verifier's quadratic instruction rewrite phase can be successfully terminated
> by a fatal signal.
>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/prog_load_signal.c b/tools/testing/selftests/bpf/prog_tests/prog_load_signal.c
>> new file mode 100644
>> index 0000000000000..0f78db3bccc76
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/prog_load_signal.c
> [ ... ]
>> +	if (!pid) {
>> +		close(pipefd[0]);
>> +		if (write(pipefd[1], "x", 1) != 1)
>> +			_exit(1);
>> +		close(pipefd[1]);
>> +		prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, NULL, "GPL",
>> +					insns, NOP_CNT + 2, NULL);
>> +		if (prog_fd >= 0)
>> +			close(prog_fd);
>> +		_exit(prog_fd < 0);
>> +	}
>
> [Severity: Medium]
> Could the child process exit too quickly on very fast hardware or with an
> optimized verifier? If the child completes the load before the parent sends
> SIGKILL, it will exit normally and become a zombie.
>
> [ ... ]
>> +	/*
>> +	 * Allow linear verification to finish before sending SIGKILL. The nop
>> +	 * removal pass is quadratic, so four control-load times still leaves a
>> +	 * wide window in which an affected kernel is rewriting instructions.
>> +	 */
>> +	kill_delay = MAX(control_time * 4, MIN_KILL_DELAY_NS);
>> +	sleep_ns(kill_delay);
>
> Here, kill_delay enforces a minimum delay of 100ms (MIN_KILL_DELAY_NS). Is it
> possible that on modern CPUs with high cache bandwidth, the verifier's 32,768
> instruction rewrite could finish in under 100ms?
>
> If control_time is very small, the parent will sleep for 100ms, which might
> give the child ample time to finish bpf_prog_load() and exit.
>
> [ ... ]
>> +	start = monotonic_ns();
>> +	ret = waitpid_timeout(pid, &status, REAP_TIMEOUT_NS);
>> +	if (!ASSERT_EQ(ret, pid, "prog_load_killable")) {
>> +		fprintf(stderr, "control load %llu us, child still alive %llu us after SIGKILL\n",
>> +			control_time / 1000, (monotonic_ns() - start) / 1000);
>> +		goto cleanup;
>> +	}
>> +	pid = -1;
>> +	ASSERT_TRUE(WIFSIGNALED(status) && WTERMSIG(status) == SIGKILL,
>> +		    "killed_by_sigkill");
>
> If the child finished early and exited normally as described above, waitpid
> will retrieve a normal exit status. This assertion will then incorrectly fail
> because the child wasn't killed by the signal.
>
> Could this lead to flaky test failures on fast CI machines?

I tested this locally and it works well, so the test was at least useful in that
regard. It is plausible it ends up being flaky (doens't seem to be right now), I
will leave it for the final person applying to decide whether to skip this patch
in the series or not.

Re: [PATCH bpf v1 2/8] selftests/bpf: Test killing a loader during instruction rewrites

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/prog_load_signal.c b/tools/testing/selftests/bpf/prog_tests/prog_load_signal.c
> new file mode 100644
> index 000000000000..0f78db3bccc7
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/prog_load_signal.c
> @@ -0,0 +1,131 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
> +#include <test_progs.h>
> +
> +#define NOP_CNT		32768
> +#define MIN_KILL_DELAY_NS	(100ULL * 1000 * 1000)
> +#define REAP_TIMEOUT_NS		(1000ULL * 1000 * 1000)
> +
> +static __u64 monotonic_ns(void)
> +{
> +	struct timespec ts;
> +
> +	clock_gettime(CLOCK_MONOTONIC, &ts);
> +	return ts.tv_sec * 1000000000ULL + ts.tv_nsec;
> +}
                ^^^

A subsystem pattern flags this as potentially concerning: monotonic_ns()
re-implements a helper that the BPF selftests test-util header already
provides.

tools/testing/selftests/bpf/testing_helpers.h defines get_time_ns():

    static inline __u64 get_time_ns(void)
    {
        struct timespec t;

        clock_gettime(CLOCK_MONOTONIC, &t);

        return (u64)t.tv_sec * 1000000000 + t.tv_nsec;
    }

That header is already pulled in by test_progs.h, so get_time_ns() is in
scope here with no extra include. It's the helper the rest of prog_tests/
uses for exactly this purpose:

  kprobe_multi_test.c
  reg_bounds.c
  lru_lock_nmi.c
  perf_link.c
  test_tc_edt.c
  uprobe_multi_test.c

The new monotonic_ns() body is functionally identical (the only difference is
the missing (u64) cast on tv_sec, which is harmless here because the
multiplier is 1000000000ULL).

> +
> +static void sleep_ns(__u64 duration)
> +{
> +	struct timespec ts = {
> +		.tv_sec = duration / 1000000000ULL,
> +		.tv_nsec = duration % 1000000000ULL,
> +	};
> +
> +	while (nanosleep(&ts, &ts) && errno == EINTR)
> +		;
> +}
> +
> +static int waitpid_timeout(pid_t pid, int *status, __u64 timeout)
> +{
> +	__u64 deadline = monotonic_ns() + timeout;
> +	int ret;
> +
> +	do {
> +		ret = waitpid(pid, status, WNOHANG);
> +		if (ret)
> +			return ret;
> +		usleep(1000);
> +	} while (monotonic_ns() < deadline);
> +
> +	return 0;
> +}
              ^^^

Similarly, waitpid_timeout() hand-rolls a deadline/usleep(1000) polling loop,
and lru_lock_nmi.c and perf_link.c show the established get_time_ns()-based
deadline idiom in this directory. Would using the shared helper keep a single
implementation to fix and keep the call sites consistent with the rest of the
suite?

[ ... ]


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

[PATCH bpf v1 3/8] bpf: Preserve packet pointer class displacement in regsafe()

[Kumar Kartikeya Dwivedi]

regsafe() maps packet pointer IDs between states and checks that each
current register range is a subset of the corresponding explored
register range. It does not, however, preserve the displacement between
registers that share a packet pointer ID.

This is unsound because packet range is shared by ID. A bounds check on
one class member updates every member, and a later access can consume the
range through another member. Commit 022ac0750883 ("bpf: use reg->var_off
instead of reg->off for pointers") folded the fixed pointer offset into
r64 and removed the old off equality check, so two individually narrower
registers can prune even when their displacement has changed. The
explored path can then license an out-of-bounds packet access on the
pruned path.

Requiring equal r64 bases would prevent the bug, but would also reject a
safe uniform translation of the whole class. Instead, record the base
translation seen for the first packet pointer in each ID mapping and
require every subsequent member to have the same translation. This keeps
the relative displacement invariant while retaining pruning for uniformly
translated classes. Packet pointers without an ID remain unaffected.

Fixes: 022ac0750883 ("bpf: use reg->var_off instead of reg->off for pointers")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 include/linux/bpf_verifier.h |  2 ++
 kernel/bpf/states.c          | 42 ++++++++++++++++++++++++++++++++++--
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 36b65797877d..5cf92ce18520 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -854,6 +854,8 @@ struct backtrack_state {
 struct bpf_id_pair {
 	u32 old;
 	u32 cur;
+	s32 pkt_ptr_delta;
+	bool pkt_ptr_delta_set;
 };
 
 struct bpf_idmap {
diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
index 66fb11b6c6a7..2f8f3fe164b0 100644
--- a/kernel/bpf/states.c
+++ b/kernel/bpf/states.c
@@ -339,6 +339,7 @@ static bool check_ids(u32 old_id, u32 cur_id, struct bpf_idmap *idmap)
 	if (idmap->cnt < BPF_ID_MAP_SIZE) {
 		map[idmap->cnt].old = old_id;
 		map[idmap->cnt].cur = cur_id;
+		map[idmap->cnt].pkt_ptr_delta_set = false;
 		idmap->cnt++;
 		return true;
 	}
@@ -352,6 +353,43 @@ static bool check_ids(u32 old_id, u32 cur_id, struct bpf_idmap *idmap)
 	return false;
 }
 
+static bool check_pkt_ptr_ids(const struct bpf_reg_state *old,
+			      const struct bpf_reg_state *cur,
+			      struct bpf_idmap *idmap)
+{
+	struct bpf_id_pair *map = idmap->map;
+	s64 delta;
+	unsigned int i;
+
+	if (!check_ids(old->id, cur->id, idmap))
+		return false;
+	if (!old->id)
+		return true;
+
+	/*
+	 * Packet range is shared by all pointers with the same ID. Preserve
+	 * their relative displacement, while allowing the whole class to move.
+	 * Packet pointer offsets are bounded by BPF_MAX_VAR_OFF, so the delta
+	 * between two valid offsets fits in s32.
+	 */
+	delta = (s64)(cur->r64.base - old->r64.base);
+	if (delta < S32_MIN || delta > S32_MAX)
+		return false;
+
+	for (i = 0; i < idmap->cnt; i++) {
+		if (map[i].old != old->id)
+			continue;
+		if (!map[i].pkt_ptr_delta_set) {
+			map[i].pkt_ptr_delta = delta;
+			map[i].pkt_ptr_delta_set = true;
+			return true;
+		}
+		return map[i].pkt_ptr_delta == delta;
+	}
+
+	return false;
+}
+
 /*
  * Compare scalar register IDs for state equivalence.
  *
@@ -632,8 +670,8 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
 		} else if (rold->range > rcur->range) {
 			return false;
 		}
-		/* id relations must be preserved */
-		if (!check_ids(rold->id, rcur->id, idmap))
+		/* id relations and intra-class displacement must be preserved */
+		if (!check_pkt_ptr_ids(rold, rcur, idmap))
 			return false;
 		/* new val must satisfy old val knowledge */
 		return range_within(rold, rcur) &&
-- 
2.53.0

Re: [PATCH bpf v1 3/8] bpf: Preserve packet pointer class displacement in regsafe()

[bot+bpf-ci]

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

> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 36b65797877d..5cf92ce18520 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -854,6 +854,8 @@ struct backtrack_state {
>  struct bpf_id_pair {
>  	u32 old;
>  	u32 cur;
> +	s32 pkt_ptr_delta;
> +	bool pkt_ptr_delta_set;
>  };
>
>  struct bpf_idmap {
> diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
> index 66fb11b6c6a7..2f8f3fe164b0 100644
> --- a/kernel/bpf/states.c
> +++ b/kernel/bpf/states.c
> @@ -339,6 +339,7 @@ static bool check_ids(u32 old_id, u32 cur_id, struct bpf_idmap *idmap)
>  	if (idmap->cnt < BPF_ID_MAP_SIZE) {
>  		map[idmap->cnt].old = old_id;
>  		map[idmap->cnt].cur = cur_id;
> +		map[idmap->cnt].pkt_ptr_delta_set = false;
>  		idmap->cnt++;
>  		return true;
>  	}
> @@ -352,6 +353,43 @@ static bool check_ids(u32 old_id, u32 cur_id, struct bpf_idmap *idmap)
>  	return false;
>  }
>
> +static bool check_pkt_ptr_ids(const struct bpf_reg_state *old,
> +			      const struct bpf_reg_state *cur,
> +			      struct bpf_idmap *idmap)
> +{
> +	struct bpf_id_pair *map = idmap->map;
> +	s64 delta;
> +	unsigned int i;
> +
> +	if (!check_ids(old->id, cur->id, idmap))
> +		return false;
> +	if (!old->id)
> +		return true;
> +
> +	/*
> +	 * Packet range is shared by all pointers with the same ID. Preserve
> +	 * their relative displacement, while allowing the whole class to move.
> +	 * Packet pointer offsets are bounded by BPF_MAX_VAR_OFF, so the delta
> +	 * between two valid offsets fits in s32.
> +	 */
> +	delta = (s64)(cur->r64.base - old->r64.base);
> +	if (delta < S32_MIN || delta > S32_MAX)
> +		return false;
> +
> +	for (i = 0; i < idmap->cnt; i++) {
> +		if (map[i].old != old->id)
> +			continue;
> +		if (!map[i].pkt_ptr_delta_set) {
> +			map[i].pkt_ptr_delta = delta;
> +			map[i].pkt_ptr_delta_set = true;
> +			return true;
> +		}
> +		return map[i].pkt_ptr_delta == delta;
> +	}
> +
> +	return false;
> +}

This isn't a bug, but since check_ids() at the top has already located
(or just inserted) the entry for old->id, would it be worth having it
hand the slot back so check_pkt_ptr_ids() does not repeat the scan?
That would also let the unreachable return false after the loop go away.

> +
>  /*
>   * Compare scalar register IDs for state equivalence.
>   *

[ ... ]


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

[PATCH bpf v1 4/8] selftests/bpf: Test packet pointer class displacement pruning

[Kumar Kartikeya Dwivedi]

Add two paths whose packet pointer ranges are individually compatible at
a join but whose members have different relative displacements. The first
path proves an eight-byte access through one member. On the second path,
the same guard only proves that the access starts before data_end.

An affected verifier prunes the second path and accepts the program. With
packet pointer class displacement preserved, it explores that path and
rejects the out-of-bounds access.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../progs/verifier_xdp_direct_packet_access.c | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_xdp_direct_packet_access.c b/tools/testing/selftests/bpf/progs/verifier_xdp_direct_packet_access.c
index 0b86d95a4133..692a8468f3d7 100644
--- a/tools/testing/selftests/bpf/progs/verifier_xdp_direct_packet_access.c
+++ b/tools/testing/selftests/bpf/progs/verifier_xdp_direct_packet_access.c
@@ -5,6 +5,50 @@
 #include <bpf/bpf_helpers.h>
 #include "bpf_misc.h"
 
+SEC("xdp")
+__description("XDP pkt regsafe preserves packet pointer class displacement")
+__failure __msg("R2 min value is outside of the allowed memory range")
+__flag(BPF_F_ANY_ALIGNMENT)
+__naked void pkt_regsafe_class_displacement(void)
+{
+	asm volatile ("\
+	r8 = *(u32 *)(r1 + %[xdp_md_data_end]);\
+	r9 = *(u32 *)(r1 + %[xdp_md_data]);\
+	r6 = r9;\
+	r6 += 8;\
+	if r6 > r8 goto l_exit_%=;\
+	r0 = *(u64 *)(r9 + 0);\
+	r4 = r0;\
+	r4 &= 15;\
+	r7 = r0;\
+	r7 >>= 63;\
+	if r7 != 0 goto l_path_b_%=;\
+	r2 = r9;\
+	r2 += r4;\
+	r3 = r2;\
+	r3 += 4;\
+	goto l_join_%=;\
+l_path_b_%=: \
+	r4 &= 3;\
+	r4 += 8;\
+	r2 = r9;\
+	r2 += r4;\
+	r3 = r2;\
+	r3 -= 4;\
+l_join_%=: \
+	r5 = r3;\
+	r5 += 4;\
+	if r5 > r8 goto l_exit_%=;\
+	r0 = *(u64 *)(r2 + 0);\
+l_exit_%=: \
+	r0 = 0;\
+	exit;\
+"	:
+	: __imm_const(xdp_md_data, offsetof(struct xdp_md, data)),
+	  __imm_const(xdp_md_data_end, offsetof(struct xdp_md, data_end))
+	: __clobber_all);
+}
+
 SEC("xdp")
 __description("XDP pkt read, pkt_end mangling, bad access 1")
 __failure __msg("R3 pointer arithmetic on pkt_end")
-- 
2.53.0

Re: [PATCH bpf v1 4/8] selftests/bpf: Test packet pointer class displacement pruning

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/progs/verifier_xdp_direct_packet_access.c b/tools/testing/selftests/bpf/progs/verifier_xdp_direct_packet_access.c
> index 0b86d95a4133..692a8468f3d7 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_xdp_direct_packet_access.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_xdp_direct_packet_access.c
> @@ -5,6 +5,50 @@
>  #include <bpf/bpf_helpers.h>
>  #include "bpf_misc.h"
>  
> +SEC("xdp")
> +__description("XDP pkt regsafe preserves packet pointer class displacement")
> +__failure __msg("R2 min value is outside of the allowed memory range")
> +__flag(BPF_F_ANY_ALIGNMENT)
> +__naked void pkt_regsafe_class_displacement(void)
> +{
> +	asm volatile ("\
> +	r8 = *(u32 *)(r1 + %[xdp_md_data_end]);\
> +	r9 = *(u32 *)(r1 + %[xdp_md_data]);\
> +	r6 = r9;\
> +	r6 += 8;\
> +	if r6 > r8 goto l_exit_%=;\
> +	r0 = *(u64 *)(r9 + 0);\
> +	r4 = r0;\
> +	r4 &= 15;\
> +	r7 = r0;\
> +	r7 >>= 63;\
> +	if r7 != 0 goto l_path_b_%=;\
> +	r2 = r9;\
> +	r2 += r4;\
> +	r3 = r2;\
> +	r3 += 4;\
> +	goto l_join_%=;\
> +l_path_b_%=: \
> +	r4 &= 3;\
> +	r4 += 8;\
> +	r2 = r9;\
> +	r2 += r4;\
> +	r3 = r2;\
> +	r3 -= 4;\
> +l_join_%=: \
> +	r5 = r3;\
> +	r5 += 4;\
> +	if r5 > r8 goto l_exit_%=;\
> +	r0 = *(u64 *)(r2 + 0);\
> +l_exit_%=: \
> +	r0 = 0;\
> +	exit;\
> +"	:
> +	: __imm_const(xdp_md_data, offsetof(struct xdp_md, data)),
> +	  __imm_const(xdp_md_data_end, offsetof(struct xdp_md, data_end))
> +	: __clobber_all);
> +}

A subsystem pattern flags this as potentially concerning:

The test only detects the bug if the verifier happens to checkpoint a
state at the join label. Nothing in the test pins that. do_check() calls
bpf_is_state_visited() at the goto l_join target (marked a prune point
in kernel/bpf/cfg.c), but a state is stored there only if the heuristic
in kernel/bpf/states.c holds: env->jmps_processed - env->prev_jmps_processed
>= 2 && env->insn_processed - env->prev_insn_processed >= 8.

For this program the first path reaches the join with 3 jumps and 16
processed instructions and no previous checkpoint, so the state is stored
today and the test does discriminate. If that ever stops being true
(heuristic retuned, one more instruction or jump added to the test, an
extra checkpoint created earlier), the second path is simply walked
instead of pruned and it still fails with the exact asserted string, so
the test would keep passing on a kernel without 6ab7e7c0e9c6 and silently
stop testing anything.

The analogous regsafe pruning test in the tree pins this explicitly:
tools/testing/selftests/bpf/progs/verifier_linked_scalars.c uses
__flag(BPF_F_TEST_STATE_FREQ) on scalars_alu32_alu64_regsafe_pruning()
(85 progs in the tree use that flag). Adding it here would force a
checkpoint at every prune point and make the pruning attempt independent
of the heuristic.

Related gap in what the test pins: the kernel patch makes two claims -
that displacement-changing classes no longer prune, and that safe uniform
translation of the whole class still prunes - and only the first is
covered. Would a companion __success case where every member of the
packet-pointer class is translated by the same amount help guard
check_pkt_ptr_ids()'s single-delta allowance against a future tightening
to plain r64.base equality?


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

[PATCH bpf v1 5/8] bpf: Reject fall-through across subprogram boundaries

[Kumar Kartikeya Dwivedi]

check_subprogs() verifies that each subprogram ends in an exit or an
unconditional jump, preventing control flow from falling through into
the next subprogram. However, this check runs before CO-RE relocations
are applied.

When a relocation cannot be resolved, bpf_core_patch_insn() poisons its
target by replacing it with an invalid BPF_CALL. A relocation targeting
the terminal instruction of a non-final subprogram can therefore create
a fall-through edge into the next subprogram after the invariant was
checked.

The per-subprogram DFS in bpf_compute_postorder() then visits the next
subprogram twice and writes past its prog->len-sized postorder array.
Stack liveness analysis relies on the same containment and can access its
per-subprogram arrays out of bounds as well.

Reject fall-through edges whose endpoints belong to different
subprograms in push_insn(). This reestablishes the invariant on the final
instruction stream at the common CFG edge insertion point. Cross-subprog
pseudo-call edges remain valid because they are represented as branch
edges.

Fixes: efcda22aa541 ("bpf: compute instructions postorder per subprogram")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/cfg.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
index 842c7d1eabcc..e3c904328ede 100644
--- a/kernel/bpf/cfg.c
+++ b/kernel/bpf/cfg.c
@@ -102,6 +102,7 @@ enum {
  */
 static int push_insn(int t, int w, int e, struct bpf_verifier_env *env)
 {
+	struct bpf_subprog_info *subprog;
 	int *insn_stack = env->cfg.insn_stack;
 	int *insn_state = env->cfg.insn_state;
 
@@ -121,6 +122,26 @@ static int push_insn(int t, int w, int e, struct bpf_verifier_env *env)
 		return -EINVAL;
 	}
 
+	/*
+	 * check_subprogs() prevents control flow from falling through a
+	 * subprogram boundary, but runs before CO-RE relocations can rewrite an
+	 * instruction. Reestablish the invariant on the final instruction stream
+	 * before constructing the CFG used by later per-subprogram passes.
+	 */
+	if (e == FALLTHROUGH) {
+		subprog = bpf_find_containing_subprog(env, t);
+		if (w < subprog->start || w >= (subprog + 1)->start) {
+			verbose_linfo(env, t, "%d: ", t);
+			verbose(env, "fall-through out of subprog from insn %d to %d\n", t, w);
+			bpf_diag_program_structure(
+				env, t, "fall-through leaves subprogram",
+				"Keep fall-through control flow inside the current subprogram.",
+				"Instruction %d falls through to instruction %d outside its subprogram.",
+				t, w);
+			return -EINVAL;
+		}
+	}
+
 	if (e == BRANCH) {
 		/* mark branch target for state pruning */
 		mark_prune_point(env, w);
-- 
2.53.0

[PATCH bpf v1 6/8] selftests/bpf: Test poisoned subprogram terminator

[Kumar Kartikeya Dwivedi]

Add a raw CO-RE test with two subprograms and a relocation that targets
the first subprogram's terminal exit. Resolving the relocation fails and
poisons the exit into a call instruction.

Verify that the original program loads without the relocation, and that
the poisoned program is rejected while constructing the CFG due to its
cross-subprogram fall-through edge. On an unfixed kernel the verifier
instead reaches the invalid call after traversing the malformed CFG.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/prog_tests/core_reloc_raw.c | 113 ++++++++++++++++++
 1 file changed, 113 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc_raw.c b/tools/testing/selftests/bpf/prog_tests/core_reloc_raw.c
index a18d3680fb16..c350fbb95845 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc_raw.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc_raw.c
@@ -14,6 +14,117 @@
 
 static char log[16 * 1024];
 
+static int load_core_relo_subprog(int btf_fd, int main_id, int sub_id,
+				  int enum_id, int access_str_off, bool relocate)
+{
+	struct bpf_insn insns[] = {
+		BPF_CALL_REL(2),
+		BPF_MOV64_IMM(BPF_REG_0, 0),
+		BPF_EXIT_INSN(),
+		BPF_MOV64_IMM(BPF_REG_0, 0),
+		BPF_EXIT_INSN(),
+	};
+	struct bpf_func_info funcs[] = {
+		{ .insn_off = 0, .type_id = main_id },
+		{ .insn_off = 3, .type_id = sub_id },
+	};
+	struct bpf_core_relo relo = {
+		.insn_off = 2 * sizeof(struct bpf_insn),
+		.type_id = enum_id,
+		.access_str_off = access_str_off,
+		.kind = BPF_CORE_ENUMVAL_VALUE,
+	};
+	union bpf_attr attr = {
+		.prog_type = BPF_PROG_TYPE_SOCKET_FILTER,
+		.insn_cnt = ARRAY_SIZE(insns),
+		.insns = (__u64)insns,
+		.license = (__u64)"GPL",
+		.log_buf = (__u64)log,
+		.log_size = sizeof(log),
+		.log_level = 1,
+		.prog_btf_fd = btf_fd,
+		.func_info_rec_size = sizeof(struct bpf_func_info),
+		.func_info = (__u64)funcs,
+		.func_info_cnt = ARRAY_SIZE(funcs),
+	};
+
+	if (relocate) {
+		attr.core_relo_cnt = 1;
+		attr.core_relos = (__u64)&relo;
+		attr.core_relo_rec_size = sizeof(relo);
+	}
+	memset(log, 0, sizeof(log));
+	return sys_bpf_prog_load(&attr, sizeof(attr), 1);
+}
+
+static void test_poisoned_subprog_terminator(void)
+{
+	const void *raw_btf;
+	struct btf *btf = NULL;
+	__u32 raw_btf_size;
+	int access_str_off, btf_fd = -1, enum_id;
+	int int_id, main_id, prog_fd = -1, proto_id, sub_id;
+
+	btf = btf__new_empty();
+	if (!ASSERT_OK_PTR(btf, "btf_new_empty"))
+		return;
+	int_id = btf__add_int(btf, "int", 4, BTF_INT_SIGNED);
+	if (!ASSERT_GT(int_id, 0, "add_int"))
+		goto cleanup;
+	proto_id = btf__add_func_proto(btf, int_id);
+	if (!ASSERT_GT(proto_id, 0, "add_func_proto"))
+		goto cleanup;
+	main_id = btf__add_func(btf, "main_fn", BTF_FUNC_STATIC, proto_id);
+	if (!ASSERT_GT(main_id, 0, "add_main_func"))
+		goto cleanup;
+	sub_id = btf__add_func(btf, "sub_fn", BTF_FUNC_STATIC, proto_id);
+	if (!ASSERT_GT(sub_id, 0, "add_sub_func"))
+		goto cleanup;
+	enum_id = btf__add_enum(btf, "core_relo_subprog_poison_missing", 4);
+	if (!ASSERT_GT(enum_id, 0, "add_enum") ||
+	    !ASSERT_OK(btf__add_enum_value(btf, "value", 0), "add_enum_value"))
+		goto cleanup;
+	access_str_off = btf__add_str(btf, "0");
+	if (!ASSERT_GT(access_str_off, 0, "add_access_str"))
+		goto cleanup;
+
+	raw_btf = btf__raw_data(btf, &raw_btf_size);
+	if (!ASSERT_OK_PTR(raw_btf, "raw_btf"))
+		goto cleanup;
+	btf_fd = bpf_btf_load(raw_btf, raw_btf_size, NULL);
+	if (!ASSERT_GE(btf_fd, 0, "btf_load"))
+		goto cleanup;
+
+	/* The same two-subprogram program is valid before the relocation. */
+	prog_fd = load_core_relo_subprog(btf_fd, main_id, sub_id, enum_id,
+					 access_str_off, false);
+	if (!ASSERT_GE(prog_fd, 0, "control_load"))
+		goto cleanup;
+	close(prog_fd);
+	prog_fd = -1;
+
+	/*
+	 * Poison the first subprogram's terminal exit. The verifier must reject
+	 * the resulting control flow across the subprogram boundary in the CFG.
+	 */
+	prog_fd = load_core_relo_subprog(btf_fd, main_id, sub_id, enum_id,
+					 access_str_off, true);
+	if (!ASSERT_LT(prog_fd, 0, "poisoned_load"))
+		goto cleanup;
+	ASSERT_HAS_SUBSTR(log, "fall-through out of subprog from insn 2 to 3",
+			  "poisoned_load_log");
+
+cleanup:
+	if (env.verbosity > VERBOSE_NORMAL && log[0]) {
+		printf("-------- program load log start --------\n");
+		printf("%s", log);
+		printf("-------- program load log end ----------\n");
+	}
+	close(prog_fd);
+	close(btf_fd);
+	btf__free(btf);
+}
+
 /* Check that verifier rejects BPF program containing relocation
  * pointing to non-existent BTF type.
  */
@@ -120,6 +231,8 @@ static void test_bad_local_id(void)
 
 void test_core_reloc_raw(void)
 {
+	if (test__start_subtest("poisoned_subprog_terminator"))
+		test_poisoned_subprog_terminator();
 	if (test__start_subtest("bad_local_id"))
 		test_bad_local_id();
 }
-- 
2.53.0

Re: [PATCH bpf v1 6/8] selftests/bpf: Test poisoned subprogram terminator

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc_raw.c b/tools/testing/selftests/bpf/prog_tests/core_reloc_raw.c
> index a18d3680fb16..c350fbb95845 100644
> --- a/tools/testing/selftests/bpf/prog_tests/core_reloc_raw.c
> +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc_raw.c

[ ... ]

> @@ -14,6 +14,117 @@
>
>  static char log[16 * 1024];
>
> +static int load_core_relo_subprog(int btf_fd, int main_id, int sub_id,
> +				  int enum_id, int access_str_off, bool relocate)
> +{

[ ... ]

> +}
> +
> +static void test_poisoned_subprog_terminator(void)
> +{

[ ... ]

> +cleanup:
> +	if (env.verbosity > VERBOSE_NORMAL && log[0]) {
> +		printf("-------- program load log start --------\n");
> +		printf("%s", log);
> +		printf("-------- program load log end ----------\n");
> +	}

This isn't a bug, but would a small shared helper be worth it now that
there are three copies of this log-dump printf block in the file? The
same three-line sequence appears in test_bad_local_id() and also appears
for BTF load failures earlier in that function.

> +	close(prog_fd);
> +	close(btf_fd);
> +	btf__free(btf);
> +}

[ ... ]


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

[PATCH bpf v1 7/8] bpf: Assign lock identity to callback map values

[Kumar Kartikeya Dwivedi]

The verifier identifies the allocation containing a bpf_spin_lock by the
pair of the map pointer and register ID. This permits ID zero for direct
map-value loads because those maps have a single element.

Callback frame constructors also leave map-value arguments with ID zero,
but their maps can have multiple elements. Consequently, nested callbacks
can hold two distinct elements of the same map with an identical lock
identity. The verifier then permits a lock acquired through one element to
be released through another. The same confusion lets graph kfuncs operate
on one element while another element is locked, allowing concurrent list
corruption.

Give lockable callback map values a fresh ID in the for-each,
timer/workqueue, and task-work frame constructors. Preserve ID zero for
single-element array maps: their callback argument and a pseudo map-value
load are aliases of the same stable allocation. Maps without locks remain
unchanged, while copies of one callback value continue to share an ID and
support balanced locking.

Fixes: d0d78c1df9b1 ("bpf: Allow locking bpf_spin_lock global variables")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1c3039f3fc32..b08501734ddf 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -447,6 +447,11 @@ static bool reg_may_point_to_spin_lock(const struct bpf_reg_state *reg)
 	return btf_record_has_field(reg_btf_record(reg), BPF_SPIN_LOCK | BPF_RES_SPIN_LOCK);
 }
 
+static bool map_value_has_static_identity(const struct bpf_map *map)
+{
+	return map->map_type == BPF_MAP_TYPE_ARRAY && map->max_entries == 1;
+}
+
 static bool type_is_rdonly_mem(u32 type)
 {
 	return type & MEM_RDONLY;
@@ -10035,6 +10040,9 @@ int map_set_for_each_callback_args(struct bpf_verifier_env *env,
 	__mark_reg_known_zero(&callee->regs[BPF_REG_3]);
 	callee->regs[BPF_REG_3].map_ptr = caller->regs[BPF_REG_1].map_ptr;
 	callee->regs[BPF_REG_3].map_uid = caller->regs[BPF_REG_1].map_uid;
+	if (reg_may_point_to_spin_lock(&callee->regs[BPF_REG_3]) &&
+	    !map_value_has_static_identity(callee->regs[BPF_REG_3].map_ptr))
+		callee->regs[BPF_REG_3].id = ++env->id_gen;
 
 	/* pointer to stack or null */
 	callee->regs[BPF_REG_4] = caller->regs[BPF_REG_3];
@@ -10131,6 +10139,9 @@ static int set_timer_callback_state(struct bpf_verifier_env *env,
 	__mark_reg_known_zero(&callee->regs[BPF_REG_3]);
 	callee->regs[BPF_REG_3].map_ptr = map_ptr;
 	callee->regs[BPF_REG_3].map_uid = map_uid;
+	if (reg_may_point_to_spin_lock(&callee->regs[BPF_REG_3]) &&
+	    !map_value_has_static_identity(callee->regs[BPF_REG_3].map_ptr))
+		callee->regs[BPF_REG_3].id = ++env->id_gen;
 
 	/* unused */
 	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
@@ -10249,6 +10260,9 @@ static int set_task_work_schedule_callback_state(struct bpf_verifier_env *env,
 	__mark_reg_known_zero(&callee->regs[BPF_REG_3]);
 	callee->regs[BPF_REG_3].map_ptr = map_ptr;
 	callee->regs[BPF_REG_3].map_uid = map_uid;
+	if (reg_may_point_to_spin_lock(&callee->regs[BPF_REG_3]) &&
+	    !map_value_has_static_identity(callee->regs[BPF_REG_3].map_ptr))
+		callee->regs[BPF_REG_3].id = ++env->id_gen;
 
 	/* unused */
 	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
-- 
2.53.0

Re: [PATCH bpf v1 7/8] bpf: Assign lock identity to callback map values

[sashiko-bot]

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

New issues:
- [High] Lock identity aliasing across distinct inner map instances due to flawed static identity check.

Pre-existing issues:
- [High] A BPF program can cause an AA deadlock by acquiring `bpf_res_spin_lock` twice on the same single-element array map element, bypassing verifier lock tracking.
--

commit 996188899c474047a5a9bb68f80d0d3c3e5a5b6a
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Assign lock identity to callback map values

This commit assigns a fresh verifier ID to lockable map values in
callback frame constructors to prevent lock aliasing. It specifically
preserves ID zero for single-element array maps to support stable
allocation aliases.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1c3039f3fc323..b08501734ddfa 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -447,6 +447,11 @@ static bool reg_may_point_to_spin_lock(const struct bpf_reg_state *reg)
>  	return btf_record_has_field(reg_btf_record(reg), BPF_SPIN_LOCK | BPF_RES_SPIN_LOCK);
>  }
>  
> +static bool map_value_has_static_identity(const struct bpf_map *map)
> +{
> +	return map->map_type == BPF_MAP_TYPE_ARRAY && map->max_entries == 1;
> +}

[Severity: High]
Does this check fail to distinguish inner_map_meta from an actual
single-element array map?

When a BPF program retrieves two distinct inner maps from an outer map
(where the inner maps are single-element array maps), and calls
bpf_for_each_map_elem on both, the verifier tracks both using the same
inner_map_meta.

Because inner_map_meta has max_entries == 1 and map_type ==
BPF_MAP_TYPE_ARRAY, will map_value_has_static_identity() return true and
incorrectly assign id = 0 to both callbacks' element pointers?

Could this allow a program to acquire a lock on an element from the first
inner map and unlock it on the element from the second inner map, corrupting
the spinlock state?

[Severity: High]
This is a pre-existing issue, but does preserving ID 0 for single-element
array maps leave an AA deadlock vulnerability in bpf_res_spin_lock tracking?

If a program acquires a bpf_res_spin_lock using a direct map value pointer
via LD_IMM64, check_ld_imm() assigns id = 0. If the program then acquires
the same lock again using a pointer obtained via bpf_map_lookup_elem(),
check_helper_call() unconditionally assigns a fresh ID to the lookup result.

Will find_lock_state() inside process_spin_lock() see the different IDs and
fail to recognize the lock is already held, causing a system hang on the
second acquisition?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905070003.3193366-1-memxor@gmail.com?part=7

Re: [PATCH bpf v1 7/8] bpf: Assign lock identity to callback map values

[Kumar Kartikeya Dwivedi]

On Sat Sep 5, 2026 at 9:21 AM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [High] Lock identity aliasing across distinct inner map instances due to flawed static identity check.
>
> Pre-existing issues:
> - [High] A BPF program can cause an AA deadlock by acquiring `bpf_res_spin_lock` twice on the same single-element array map element, bypassing verifier lock tracking.
> --
>
> commit 996188899c474047a5a9bb68f80d0d3c3e5a5b6a
> Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>
> bpf: Assign lock identity to callback map values
>
> This commit assigns a fresh verifier ID to lockable map values in
> callback frame constructors to prevent lock aliasing. It specifically
> preserves ID zero for single-element array maps to support stable
> allocation aliases.
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 1c3039f3fc323..b08501734ddfa 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -447,6 +447,11 @@ static bool reg_may_point_to_spin_lock(const struct bpf_reg_state *reg)
>>  	return btf_record_has_field(reg_btf_record(reg), BPF_SPIN_LOCK | BPF_RES_SPIN_LOCK);
>>  }
>>
>> +static bool map_value_has_static_identity(const struct bpf_map *map)
>> +{
>> +	return map->map_type == BPF_MAP_TYPE_ARRAY && map->max_entries == 1;
>> +}
>
> [Severity: High]
> Does this check fail to distinguish inner_map_meta from an actual
> single-element array map?
>
> When a BPF program retrieves two distinct inner maps from an outer map
> (where the inner maps are single-element array maps), and calls
> bpf_for_each_map_elem on both, the verifier tracks both using the same
> inner_map_meta.
>
> Because inner_map_meta has max_entries == 1 and map_type ==
> BPF_MAP_TYPE_ARRAY, will map_value_has_static_identity() return true and
> incorrectly assign id = 0 to both callbacks' element pointers?
>
> Could this allow a program to acquire a lock on an element from the first
> inner map and unlock it on the element from the second inner map, corrupting
> the spinlock state?

This one is real, will fix.

>
> [Severity: High]
> This is a pre-existing issue, but does preserving ID 0 for single-element
> array maps leave an AA deadlock vulnerability in bpf_res_spin_lock tracking?
>
> If a program acquires a bpf_res_spin_lock using a direct map value pointer
> via LD_IMM64, check_ld_imm() assigns id = 0. If the program then acquires
> the same lock again using a pointer obtained via bpf_map_lookup_elem(),
> check_helper_call() unconditionally assigns a fresh ID to the lookup result.
>
> Will find_lock_state() inside process_spin_lock() see the different IDs and
> fail to recognize the lock is already held, causing a system hang on the
> second acquisition?
>

This one is not.

> [ ... ]

[PATCH bpf v1 8/8] selftests/bpf: Check callback map value lock identity

[Kumar Kartikeya Dwivedi]

Add a verifier test which retains a map value from an outer callback and
then acquires a lock through an inner callback value before attempting to
release the outer value's lock. Both values can denote different elements,
so the verifier must reject the mismatched unlock.

Keep a nested same-element lock/unlock program as a positive control. This
ensures assigning fresh identities to callback map values does not reject
balanced locking through one callback argument.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/prog_tests/verifier.c       |  2 +
 .../bpf/progs/verifier_callback_lock.c        | 73 +++++++++++++++++++
 2 files changed, 75 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_callback_lock.c

diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 64ac49ad67e6..5c572dd725e7 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -25,6 +25,7 @@
 #include "verifier_btf_ctx_access.skel.h"
 #include "verifier_btf_unreliable_prog.skel.h"
 #include "verifier_call_large_imm.skel.h"
+#include "verifier_callback_lock.skel.h"
 #include "verifier_cfg.skel.h"
 #include "verifier_cgroup_inv_retcode.skel.h"
 #include "verifier_cgroup_skb.skel.h"
@@ -188,6 +189,7 @@ void test_verifier_bswap(void)                { RUN(verifier_bswap); }
 void test_verifier_btf_ctx_access(void)       { RUN(verifier_btf_ctx_access); }
 void test_verifier_btf_unreliable_prog(void)  { RUN(verifier_btf_unreliable_prog); }
 void test_verifier_call_large_imm(void)       { RUN(verifier_call_large_imm); }
+void test_verifier_callback_lock(void)        { RUN(verifier_callback_lock); }
 void test_verifier_cfg(void)                  { RUN(verifier_cfg); }
 void test_verifier_cgroup_inv_retcode(void)   { RUN(verifier_cgroup_inv_retcode); }
 void test_verifier_cgroup_skb(void)           { RUN(verifier_cgroup_skb); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_callback_lock.c b/tools/testing/selftests/bpf/progs/verifier_callback_lock.c
new file mode 100644
index 000000000000..ee09dd66f467
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_callback_lock.c
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+struct bpf_map;
+
+struct lock_value {
+	struct bpf_spin_lock lock;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 2);
+	__type(key, int);
+	__type(value, struct lock_value);
+} lock_map SEC(".maps");
+
+struct callback_ctx {
+	struct lock_value *value;
+};
+
+static long lock_different_value(struct bpf_map *map, int *key,
+				 struct lock_value *value, struct callback_ctx *ctx)
+{
+	bpf_spin_lock(&value->lock);
+	bpf_spin_unlock(&ctx->value->lock);
+	return 0;
+}
+
+static long nest_lock_different_value(struct bpf_map *map, int *key,
+				      struct lock_value *value, void *data)
+{
+	struct callback_ctx ctx = { .value = value };
+
+	bpf_for_each_map_elem(&lock_map, lock_different_value, &ctx, 0);
+	return 0;
+}
+
+SEC("?tc")
+__description("callback map value has a distinct lock identity")
+__failure __msg("bpf_spin_unlock of different lock")
+int callback_value_lock_identity(void *ctx)
+{
+	bpf_for_each_map_elem(&lock_map, nest_lock_different_value, NULL, 0);
+	return 0;
+}
+
+static long lock_same_value(struct bpf_map *map, int *key,
+			    struct lock_value *value, void *data)
+{
+	bpf_spin_lock(&value->lock);
+	bpf_spin_unlock(&value->lock);
+	return 0;
+}
+
+static long nest_lock_same_value(struct bpf_map *map, int *key,
+				 struct lock_value *value, void *data)
+{
+	bpf_for_each_map_elem(&lock_map, lock_same_value, NULL, 0);
+	return 0;
+}
+
+SEC("?tc")
+__description("nested callback can lock its own map value")
+__success
+int callback_value_lock_identity_same(void *ctx)
+{
+	bpf_for_each_map_elem(&lock_map, nest_lock_same_value, NULL, 0);
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.53.0