[PATCH bpf v2 0/7] Misc bug fixes - part 5

[PATCH bpf v2 0/7] Misc bug fixes - part 5

[Kumar Kartikeya Dwivedi]

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

Changelog:
----------
v1 -> v2
v1: https://lore.kernel.org/bpf/20260905070003.3193366-1-memxor@gmail.com

 * Address inner map corner case for callback map value patch.
 * Drop patch 2 since the test can be flaky.

Kumar Kartikeya Dwivedi (7):
  bpf: Make post-verification instruction rewrites killable
  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                         |  39 +++++-
 .../selftests/bpf/prog_tests/core_reloc_raw.c | 113 ++++++++++++++++
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../bpf/progs/verifier_callback_lock.c        | 121 ++++++++++++++++++
 .../progs/verifier_xdp_direct_packet_access.c |  44 +++++++
 9 files changed, 399 insertions(+), 6 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_callback_lock.c


base-commit: b75a000f2ac15f4778ddd6d9298d60b24ad776fa
-- 
2.53.0

[PATCH bpf v2 1/7] 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. Most callers propagate patching failures directly. JIT
constant blinding can instead fall back to the interpreter, so recheck for
a pending fatal signal after bpf_fixup_call_args() and after runtime
selection to keep cancellation from being consumed by that fallback.

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 ++++++++++++++++++++-
 kernel/bpf/verifier.c | 10 ++++++++++
 2 files changed, 30 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;
 }
-
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1c3039f3fc32..9c797cc3df40 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -19,6 +19,7 @@
 #include <linux/bsearch.h>
 #include <linux/sort.h>
 #include <linux/perf_event.h>
+#include <linux/sched/signal.h>
 #include <linux/ctype.h>
 #include <linux/error-injection.h>
 #include <linux/bpf_lsm.h>
@@ -21367,6 +21368,13 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
 
 	if (ret == 0)
 		ret = bpf_fixup_call_args(env);
+	/*
+	 * JIT constant blinding treats instruction patching failures as a
+	 * request to fall back to the interpreter. Do not let such fallback
+	 * consume a fatal-signal cancellation from bpf_patch_insn_data().
+	 */
+	if (ret == 0 && fatal_signal_pending(current))
+		ret = -EINTR;
 
 	env->verification_time = ktime_get_ns() - start_time;
 	print_verification_stats(env);
@@ -21425,6 +21433,8 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
 		env->prog->expected_attach_type = 0;
 
 	env->prog = __bpf_prog_select_runtime(env, env->prog, &ret);
+	if (ret == 0 && fatal_signal_pending(current))
+		ret = -EINTR;
 
 err_release_maps:
 	if (ret)
-- 
2.53.0

Re: [PATCH bpf v2 1/7] bpf: Make post-verification instruction rewrites killable

[Eduard Zingerman]

On Sat, 2026-09-05 at 10:34 +0200, Kumar Kartikeya Dwivedi wrote:
> 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. Most callers propagate patching failures directly. JIT
> constant blinding can instead fall back to the interpreter, so recheck for
> a pending fatal signal after bpf_fixup_call_args() and after runtime
> selection to keep cancellation from being consumed by that fallback.
> 
> 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>
> ---

In general, I think we should stop hiding our heads and finally
address this properly, by changed bpf_patch_insn_data()
implementation. Multiple solutions were discussed:
- converting instructions to a linked list before rewrites
- accumulation of patches in a loop and application in a single pass
  [I'll give this one a try as it seem most self-contained]

...

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1c3039f3fc32..9c797cc3df40 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -19,6 +19,7 @@
>  #include <linux/bsearch.h>
>  #include <linux/sort.h>
>  #include <linux/perf_event.h>
> +#include <linux/sched/signal.h>
>  #include <linux/ctype.h>
>  #include <linux/error-injection.h>
>  #include <linux/bpf_lsm.h>
> @@ -21367,6 +21368,13 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
>  
>  	if (ret == 0)
>  		ret = bpf_fixup_call_args(env);
> +	/*
> +	 * JIT constant blinding treats instruction patching failures as a
> +	 * request to fall back to the interpreter. Do not let such fallback
> +	 * consume a fatal-signal cancellation from bpf_patch_insn_data().
> +	 */
> +	if (ret == 0 && fatal_signal_pending(current))
> +		ret = -EINTR;
>  
>  	env->verification_time = ktime_get_ns() - start_time;
>  	print_verification_stats(env);
> @@ -21425,6 +21433,8 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
>  		env->prog->expected_attach_type = 0;
>  
>  	env->prog = __bpf_prog_select_runtime(env, env->prog, &ret);
> +	if (ret == 0 && fatal_signal_pending(current))
> +		ret = -EINTR;
>  
>  err_release_maps:
>  	if (ret)

Would it be possible to move these checks to constants blinding path?

[PATCH bpf v2 2/7] 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 v2 2/7] bpf: Preserve packet pointer class displacement in regsafe()

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 5083 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;
>  };

Adding both s32 pkt_ptr_delta and bool pkt_ptr_delta_set doubles
sizeof(struct bpf_id_pair) from 8 to 16 bytes (4 + 4 + 4 + 1 = 13,
rounded up to 16 by alignment). With BPF_ID_MAP_SIZE = 1312, this grows
idmap_scratch from ~10 KiB to ~21 KiB on every bpf_verifier_env.

Could a sentinel value (e.g., S32_MIN) encode the 'unset' state without
needing the separate flag? The code already bounds useful deltas to
(-2^30, 2^30) via BPF_MAX_VAR_OFF and check_reg_sane_offset_ptr(), and
rejects anything outside [S32_MIN, S32_MAX]. This would keep the struct
at 12 bytes and save approximately 5 KiB per verifier environment.

As a design note: struct bpf_id_pair is the generic old->cur id map
shared by scalar linking, reference/lock/irq ids, dynptrs and iterators
(check_ids() is called from check_scalar_ids(), regs_exact(), refsafe(),
stacksafe() and regsafe()). Carrying packet-pointer-specific state in it
means every one of those users pays the memory cost.

> 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;
> +}

Is the trailing return false reachable? check_pkt_ptr_ids() only reaches
the loop after check_ids(old->id, cur->id, idmap) returned true AND
old->id != 0. Looking at check_ids() in states.c, it has exactly two
true-returning paths for a non-zero old_id:

  for (i = 0; i < idmap->cnt; i++) {
          if (map[i].old == old_id)
                  return map[i].cur == cur_id;
          if (map[i].cur == cur_id)
                  return false;
  }
  if (idmap->cnt < BPF_ID_MAP_SIZE) {
          map[idmap->cnt].old = old_id;
          ...
          return true;
  }
  return false;

So on every path that reaches check_pkt_ptr_ids()'s loop there is
guaranteed to be an i < idmap->cnt with map[i].old == old->id, and
check_ids() never creates two entries with the same .old (it only
appends after failing to find a match).

The loop therefore always returns from inside its body and control never
falls through to the trailing return false. This is cosmetic (the
fallback returns the safe/conservative value), but it is unreachable
code in a new function.

The BPF verifier convention for a provably impossible state is
verifier_bug()/WARN_ONCE() rather than a silent return, so either
dropping the statement or making it a verifier_bug() would document the
invariant instead of hiding it. As written, a future change that lets
duplicate or foreign entries into idmap->map would silently degrade to
'never prune' with no diagnostic.

> @@ -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) &&


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

Re: [PATCH bpf v2 2/7] bpf: Preserve packet pointer class displacement in regsafe()

[Alexei Starovoitov]

On Sat Sep 5, 2026 at 1:34 AM PDT, Kumar Kartikeya Dwivedi wrote:
> 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;

AI's complain is somewhat valid.
Using extra memory for this narrow case isn't great.

>  };
>  
>  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);

Poking into cnum internals isn't great either.
If we go with this apporach it has be some helper in cnum.c

Let's figure out an alternative fix.

> +	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) &&

Re: [PATCH bpf v2 2/7] bpf: Preserve packet pointer class displacement in regsafe()

[Eduard Zingerman]

On Sat, 2026-09-05 at 10:34 +0200, Kumar Kartikeya Dwivedi wrote:
> 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>
> ---

So, essentially this is the same problem as `id + ADD_CONST` tracking
for scalar values, where the actual difference is stored in `delta` field.
In other words, that would be an undo of 022ac0750883 for packet pointers.

What about the original suggested fix:

            /* id relations must be preserved */
            if (!check_ids(rold->id, rcur->id, idmap))
                    return false;
    +       if (rold->id && rold->r64.base != rcur->r64.base)
    +               return false;
            /* new val must satisfy old val knowledge */
            return range_within(rold, rcur) &&
                   tnum_in(rold->var_off, rcur->var_off);

Is it that bad?
Given this logic in adjust_ptr_min_max_vals:

    case BPF_ADD:
        ...
		if (reg_is_pkt_pointer(ptr_reg))
			if (!known)
				dst_reg->id = ++env->id_gen;

I'd expect it to be okay.

...

Re: [PATCH bpf v2 2/7] bpf: Preserve packet pointer class displacement in regsafe()

[Eduard Zingerman]

On Sat, 2026-09-05 at 23:40 -0700, Eduard Zingerman wrote:
> On Sat, 2026-09-05 at 10:34 +0200, Kumar Kartikeya Dwivedi wrote:
> > 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>
> > ---
> 
> So, essentially this is the same problem as `id + ADD_CONST` tracking
> for scalar values, where the actual difference is stored in `delta` field.
> In other words, that would be an undo of 022ac0750883 for packet pointers.
> 
> What about the original suggested fix:
> 
>             /* id relations must be preserved */
>             if (!check_ids(rold->id, rcur->id, idmap))
>                     return false;
>     +       if (rold->id && rold->r64.base != rcur->r64.base)
>     +               return false;
>             /* new val must satisfy old val knowledge */
>             return range_within(rold, rcur) &&
>                    tnum_in(rold->var_off, rcur->var_off);
> 
> Is it that bad?
> Given this logic in adjust_ptr_min_max_vals:
> 
>     case BPF_ADD:
>         ...
> 		if (reg_is_pkt_pointer(ptr_reg))
> 			if (!known)
> 				dst_reg->id = ++env->id_gen;
> 
> I'd expect it to be okay.
> 
> ...

In case there are veristat regressions, it seems the following should
be on-par with pre 022ac0750883 state (untested, llm):

  --- a/kernel/bpf/verifier.c
  +++ b/kernel/bpf/verifier.c
  @@ -14685,16 +14685,20 @@
   		 * dst_reg gets the pointer type and since some positive
   		 * integer value was added to the pointer, give it a new 'id'
   		 * if it's a PTR_TO_PACKET.
  -		 * this creates a new 'base' pointer, off_reg (variable) gets
  -		 * added into the variable offset, and we copy the fixed offset
  -		 * from ptr_reg.
  +		 * This creates a new shared base with delta zero. Constant
  +		 * arithmetic preserves the ID and accumulates delta instead;
  +		 * if delta overflows, start a new ID as well.
   		 */
   		dst_reg->r64 = cnum64_add(ptr_reg->r64, off_reg->r64);
   		dst_reg->var_off = tnum_add(ptr_reg->var_off, off_reg->var_off);
   		dst_reg->raw = ptr_reg->raw;
   		if (reg_is_pkt_pointer(ptr_reg)) {
  -			if (!known)
  +			if (!known ||
  +			    check_add_overflow(ptr_reg->delta, smin_val,
  +					       &dst_reg->delta)) {
   				dst_reg->id = ++env->id_gen;
  +				dst_reg->delta = 0;
  +			}
   			/*
   			 * Clear range for unknown addends since we can't know
   			 * where the pkt pointer ended up. Also clear AT_PKT_END /
  @@ -14738,8 +14742,12 @@
   		dst_reg->var_off = tnum_sub(ptr_reg->var_off, off_reg->var_off);
   		dst_reg->raw = ptr_reg->raw;
   		if (reg_is_pkt_pointer(ptr_reg)) {
  -			if (!known)
  +			if (!known ||
  +			    check_sub_overflow(ptr_reg->delta, smin_val,
  +					       &dst_reg->delta)) {
   				dst_reg->id = ++env->id_gen;
  +				dst_reg->delta = 0;
  +			}
   			/*
   			 * Clear range if the subtrahend may be negative since
   			 * pkt pointer could move past its bounds. A positive
  --- a/kernel/bpf/states.c
  +++ b/kernel/bpf/states.c
  @@ -634,6 +634,9 @@
   		}
   		/* id relations must be preserved */
   		if (!check_ids(rold->id, rcur->id, idmap))
  +			return false;
  +		/* Preserve displacements from the shared packet base. */
  +		if (rold->id && rold->delta != rcur->delta)
   			return false;
   		/* new val must satisfy old val knowledge */
   		return range_within(rold, rcur) &&

Re: [PATCH bpf v2 2/7] bpf: Preserve packet pointer class displacement in regsafe()

[Alexei Starovoitov]

On Sun, Sep 6, 2026 at 12:04 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Sat, 2026-09-05 at 23:40 -0700, Eduard Zingerman wrote:
> > On Sat, 2026-09-05 at 10:34 +0200, Kumar Kartikeya Dwivedi wrote:
> > > 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>
> > > ---
> >
> > So, essentially this is the same problem as `id + ADD_CONST` tracking
> > for scalar values, where the actual difference is stored in `delta` field.
> > In other words, that would be an undo of 022ac0750883 for packet pointers.
> >
> > What about the original suggested fix:
> >
> >             /* id relations must be preserved */
> >             if (!check_ids(rold->id, rcur->id, idmap))
> >                     return false;
> >     +       if (rold->id && rold->r64.base != rcur->r64.base)
> >     +               return false;
> >             /* new val must satisfy old val knowledge */
> >             return range_within(rold, rcur) &&
> >                    tnum_in(rold->var_off, rcur->var_off);
> >
> > Is it that bad?
> > Given this logic in adjust_ptr_min_max_vals:
> >
> >     case BPF_ADD:
> >         ...
> >               if (reg_is_pkt_pointer(ptr_reg))
> >                       if (!known)
> >                               dst_reg->id = ++env->id_gen;
> >
> > I'd expect it to be okay.
> >
> > ...
>
> In case there are veristat regressions, it seems the following should
> be on-par with pre 022ac0750883 state (untested, llm):

Yeah. let's check veristat.
If it's in the noise I'd go with the simplest
rold->r64.base != rcur->r64.base

[PATCH bpf v2 3/7] 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 v2 3/7] selftests/bpf: Test packet pointer class displacement pruning

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2298 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"
>

This isn't a bug, but any reason to place this ahead of the converted
tests rather than appending it at the end of the file, so the
`/* Converted from ... */` block stays contiguous?

> +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;\

This isn't a bug, but would it be worth matching the surrounding file's
asm layout here (aligned continuation backslashes, `lN_%=:` labels with
the first insn on the same line), or is the more descriptive label naming
deliberate for the multi-path control flow?

> +	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")


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

Re: [PATCH bpf v2 3/7] selftests/bpf: Test packet pointer class displacement pruning

[Alexei Starovoitov]

On Sat Sep 5, 2026 at 1:34 AM PDT, Kumar Kartikeya Dwivedi wrote:
> 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_%=;\

why all the loads and math? Are they meaningful for the test?
I suspect the test can be reduced in half.

> +	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")

[PATCH bpf v2 4/7] 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

Re: [PATCH bpf v2 4/7] bpf: Reject fall-through across subprogram boundaries

[Alexei Starovoitov]

On Sat Sep 5, 2026 at 1:34 AM PDT, Kumar Kartikeya Dwivedi wrote:
> 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;
> +		}
> +	}

I don't think it's the right fix.
Instead we should teach bpf_core_poison_insn() to poison only relocatable insns.
Those in bpf_core_patch_insn()'s switch() statement.

Otherwise we will have all sort of other issues with random insns being
converted into call insns by in-kernel CORE.

pw-bot: cr

[PATCH bpf v2 5/7] 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

[PATCH bpf v2 6/7] 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. It permits ID zero for direct
map-value loads into single-element array maps because they have one stable
value.

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. Keep ID zero for
top-level one-element array maps, whose callback argument and pseudo
map-value load alias the same stable allocation. Maps without locks remain
unchanged, while copies of one callback value continue to share an ID and
support balanced locking.

Map-in-map lookups need additional care. Distinct concrete inner maps share
the verifier-visible inner_map_meta, so a one-element inner array otherwise
looks like the same static allocation. Preserve lookup identity as map_uid
for lock-bearing inner maps and consult it before applying the ID-zero
exception. Restricting UID propagation to maps with identity-sensitive
fields avoids making state pruning conservative for every inner map.

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 | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 9c797cc3df40..ad14a7fbed72 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -448,6 +448,13 @@ 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_reg_state *reg)
+{
+	const struct bpf_map *map = reg->map_ptr;
+
+	return !reg->map_uid && map->map_type == BPF_MAP_TYPE_ARRAY && map->max_entries == 1;
+}
+
 static bool type_is_rdonly_mem(u32 type)
 {
 	return type & MEM_RDONLY;
@@ -1926,11 +1933,18 @@ static void refine_map_lookup_value(struct bpf_reg_state *reg)
 	if (map->inner_map_meta) {
 		reg->type = CONST_PTR_TO_MAP | maybe_null;
 		reg->map_ptr = map->inner_map_meta;
-		/* transfer reg's id which is unique for every map_lookup_elem
-		 * as UID of the inner map.
+		/*
+		 * Concrete inner maps share the verifier-visible inner_map_meta.
+		 * Preserve the lookup identity only for embedded objects whose
+		 * verification needs to distinguish concrete map instances. Doing
+		 * this for every inner map makes state pruning too conservative.
+		 *
+		 * Each map lookup has a unique register ID, so use it as the UID of
+		 * the inner map.
 		 */
 		if (btf_record_has_field(map->inner_map_meta->record,
-					 BPF_TIMER | BPF_WORKQUEUE | BPF_TASK_WORK))
+					 BPF_TIMER | BPF_WORKQUEUE | BPF_TASK_WORK |
+					 BPF_SPIN_LOCK | BPF_RES_SPIN_LOCK))
 			reg->map_uid = reg->id;
 	} else if (map->map_type == BPF_MAP_TYPE_XSKMAP) {
 		reg->type = PTR_TO_XDP_SOCK | maybe_null;
@@ -10036,6 +10050,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]))
+		callee->regs[BPF_REG_3].id = ++env->id_gen;
 
 	/* pointer to stack or null */
 	callee->regs[BPF_REG_4] = caller->regs[BPF_REG_3];
@@ -10132,6 +10149,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]))
+		callee->regs[BPF_REG_3].id = ++env->id_gen;
 
 	/* unused */
 	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
@@ -10250,6 +10270,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]))
+		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 v2 6/7] bpf: Assign lock identity to callback map values

[bot+bpf-ci]

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

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 9c797cc3df40..ad14a7fbed72 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -10036,6 +10050,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]))
> +		callee->regs[BPF_REG_3].id = ++env->id_gen;
>
>  	/* pointer to stack or null */
>  	callee->regs[BPF_REG_4] = caller->regs[BPF_REG_3];
> @@ -10132,6 +10149,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]))
> +		callee->regs[BPF_REG_3].id = ++env->id_gen;
>
>  	/* unused */
>  	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
> @@ -10250,6 +10270,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]))
> +		callee->regs[BPF_REG_3].id = ++env->id_gen;
>
>  	/* unused */
>  	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);

This isn't a bug, but the identical three-line block appears in all
three callback-frame constructors (map_set_for_each_callback_args,
set_timer_callback_state, and set_task_work_schedule_callback_state).

Since the security property depends on every PTR_TO_MAP_VALUE callback
argument getting an identity, would it make sense to fold this logic
into a small helper function? Something like mark_callback_map_value(env,
reg, map_ptr, map_uid) that handles the type/map_ptr/map_uid assignment
together with the conditional ID generation would make the invariant
enforced by construction, so a future callback constructor couldn't
accidentally omit the check.


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

Re: [PATCH bpf v2 6/7] bpf: Assign lock identity to callback map values

[Eduard Zingerman]

On Sat, 2026-09-05 at 10:34 +0200, Kumar Kartikeya Dwivedi wrote:
> The verifier identifies the allocation containing a bpf_spin_lock by the
> pair of the map pointer and register ID. It permits ID zero for direct
> map-value loads into single-element array maps because they have one stable
> value.
> 
> 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. Keep ID zero for
> top-level one-element array maps, whose callback argument and pseudo
> map-value load alias the same stable allocation. Maps without locks remain
> unchanged, while copies of one callback value continue to share an ID and
> support balanced locking.
> 
> Map-in-map lookups need additional care. Distinct concrete inner maps share
> the verifier-visible inner_map_meta, so a one-element inner array otherwise
> looks like the same static allocation. Preserve lookup identity as map_uid
> for lock-bearing inner maps and consult it before applying the ID-zero
> exception. Restricting UID propagation to maps with identity-sensitive
> fields avoids making state pruning conservative for every inner map.

I find the above commit message extremely hard to parse.
Please replace it with a small repro program and an explanation of
what fails in code.

> 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 | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 9c797cc3df40..ad14a7fbed72 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -448,6 +448,13 @@ 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_reg_state *reg)
> +{
> +	const struct bpf_map *map = reg->map_ptr;
> +
> +	return !reg->map_uid && map->map_type == BPF_MAP_TYPE_ARRAY && map->max_entries == 1;
> +}
> +
>  static bool type_is_rdonly_mem(u32 type)
>  {
>  	return type & MEM_RDONLY;
> @@ -1926,11 +1933,18 @@ static void refine_map_lookup_value(struct bpf_reg_state *reg)
>  	if (map->inner_map_meta) {
>  		reg->type = CONST_PTR_TO_MAP | maybe_null;
>  		reg->map_ptr = map->inner_map_meta;
> -		/* transfer reg's id which is unique for every map_lookup_elem
> -		 * as UID of the inner map.
> +		/*
> +		 * Concrete inner maps share the verifier-visible inner_map_meta.
> +		 * Preserve the lookup identity only for embedded objects whose
> +		 * verification needs to distinguish concrete map instances. Doing
> +		 * this for every inner map makes state pruning too conservative.
> +		 *
> +		 * Each map lookup has a unique register ID, so use it as the UID of
> +		 * the inner map.
>  		 */
>  		if (btf_record_has_field(map->inner_map_meta->record,
> -					 BPF_TIMER | BPF_WORKQUEUE | BPF_TASK_WORK))
> +					 BPF_TIMER | BPF_WORKQUEUE | BPF_TASK_WORK |
> +					 BPF_SPIN_LOCK | BPF_RES_SPIN_LOCK))

I'd drop the condition above altogether and route map_uid through
check_ids().

>  			reg->map_uid = reg->id;
>  	} else if (map->map_type == BPF_MAP_TYPE_XSKMAP) {
>  		reg->type = PTR_TO_XDP_SOCK | maybe_null;
> @@ -10036,6 +10050,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]))

reg_may_point_to_spin_lock() check here and below is redundant. Let's
drop it and save ourselves from necessity to analyze in which cases
PTR_TO_MAP_VALUE requires an .id.

map_value_has_static_identity() -- I don't think this predicate is
necessary either. Can you construct a realistic program requiring such
special case?

> +		callee->regs[BPF_REG_3].id = ++env->id_gen;
>  
>  	/* pointer to stack or null */
>  	callee->regs[BPF_REG_4] = caller->regs[BPF_REG_3];
> @@ -10132,6 +10149,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]))
> +		callee->regs[BPF_REG_3].id = ++env->id_gen;
>  
>  	/* unused */
>  	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
> @@ -10250,6 +10270,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]))
> +		callee->regs[BPF_REG_3].id = ++env->id_gen;
>  
>  	/* unused */
>  	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);

[PATCH bpf v2 7/7] 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 callback value. Both values can denote different elements,
so the verifier must reject the mismatched unlock.

Also exercise two distinct one-element inner arrays. Their concrete map
instances share inner-map metadata, but their callback values must retain
distinct lock identities.

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        | 121 ++++++++++++++++++
 2 files changed, 123 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..d23e13908ceb
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_callback_lock.c
@@ -0,0 +1,121 @@
+// 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 inner_lock_map {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, int);
+	__type(value, struct lock_value);
+} inner_lock_map_a SEC(".maps"), inner_lock_map_b SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
+	__uint(max_entries, 2);
+	__type(key, int);
+	__array(values, struct inner_lock_map);
+} lock_map_of_maps SEC(".maps") = {
+	.values = {
+		[0] = &inner_lock_map_a,
+		[1] = &inner_lock_map_b,
+	},
+};
+
+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 nest_lock_different_inner_value(struct bpf_map *map, int *key,
+					    struct lock_value *value, void *data)
+{
+	struct callback_ctx ctx = { .value = value };
+	int inner_key = 1;
+	void *inner_map;
+
+	inner_map = bpf_map_lookup_elem(&lock_map_of_maps, &inner_key);
+	if (!inner_map)
+		return 0;
+	bpf_for_each_map_elem(inner_map, lock_different_value, &ctx, 0);
+	return 0;
+}
+
+SEC("?tc")
+__description("distinct one-element inner maps have distinct lock identities")
+__failure __msg("bpf_spin_unlock of different lock")
+int callback_inner_map_value_lock_identity(void *ctx)
+{
+	int inner_key = 0;
+	void *inner_map;
+
+	inner_map = bpf_map_lookup_elem(&lock_map_of_maps, &inner_key);
+	if (!inner_map)
+		return 0;
+	bpf_for_each_map_elem(inner_map, nest_lock_different_inner_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

Re: [PATCH bpf v2 7/7] selftests/bpf: Check callback map value lock identity

[bot+bpf-ci]

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

> commit bc45ac4e826b8cd6d25406fcd3cef1fbb13c58c1
> Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> Date:   Mon Apr 28 20:52:43 2025 +0000
>
>     selftests/bpf: Check callback map value lock identity
>
>     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 callback value. Both values can denote different elements,
>     so the verifier must reject the mismatched unlock.
>     Also exercise two distinct one-element inner arrays. Their concrete map
>     instances share inner-map metadata, but their callback values must retain
>     distinct lock identities.

The commit message states that lock identities are distinct because "their
concrete map instances share inner-map metadata, but their callback values
must retain distinct lock identities." Is this the correct explanation?

Looking at refine_map_lookup_value() in the verifier, it assigns
reg->map_uid = reg->id where reg->id is a fresh ++env->id_gen per lookup.
This means map_uid encodes the lookup operation, not the concrete inner map.

In callback_inner_map_value_lock_identity, the outer lookup uses
inner_key=0 and nest_lock_different_inner_value uses inner_key=1. These
two separate bpf_map_lookup_elem call sites would get different map_uid
values (and hence different callback-value IDs) even if both used
inner_key=0 against a single shared concrete inner map.

Shouldn't the explanation be about distinct lookups preserving distinct
identities, rather than about the two concrete inner maps?

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

> 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"

[ ... ]

> 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..d23e13908ceb
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_callback_lock.c
> @@ -0,0 +1,121 @@
> +// 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 inner_lock_map {
> +	__uint(type, BPF_MAP_TYPE_ARRAY);
> +	__uint(max_entries, 1);
> +	__type(key, int);
> +	__type(value, struct lock_value);
> +} inner_lock_map_a SEC(".maps"), inner_lock_map_b SEC(".maps");
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
> +	__uint(max_entries, 2);
> +	__type(key, int);
> +	__array(values, struct inner_lock_map);
> +} lock_map_of_maps SEC(".maps") = {
> +	.values = {
> +		[0] = &inner_lock_map_a,
> +		[1] = &inner_lock_map_b,
> +	},
> +};
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_ARRAY);
> +	__uint(max_entries, 2);
> +	__type(key, int);
> +	__type(value, struct lock_value);
> +} lock_map SEC(".maps");

A subsystem pattern flags this as potentially concerning:

A new prog file is being added for spin-lock identity mismatch tests,
but tools/testing/selftests/bpf/progs/test_spin_lock_fail.c already
exists in the same directory and is dedicated to exactly this behavior.

That file already has lock_id_mismatch_* programs (kptr/global/mapval/
innermapval variants plus lock_id_mismatch_mapval_mapval and
lock_id_mismatch_innermapval_innermapval1/2), and every one of those
expects "bpf_spin_unlock of different lock" - the same string these
tests expect.

The three new programs here are callback-frame variants of the same
mismatch (one more source of lock identity), and the map fixtures
(lock-bearing ARRAY plus ARRAY_OF_MAPS of lock-bearing one-element
inner arrays) closely resemble fixtures already in test_spin_lock_fail.c.

Could the three programs be added as new cases in test_spin_lock_fail.c
to avoid duplicating the fixtures and keep all lock-identity cases
discoverable in one place?

Note: There may be a defensible reason to keep them separate -
test_spin_lock_fail.c uses a hand-maintained table in
prog_tests/spin_lock.c and pulls in vmlinux.h + bpf_experimental.h,
while this file uses modern __failure/__msg annotations and needs no
table entry. Just checking whether the split is intended.

[ ... ]

> +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";

The preceding kernel commit 06ece9a5c36d ("bpf: Assign lock identity to
callback map values") adds the same three-line ID assignment to three
frame constructors - map_set_for_each_callback_args(),
set_timer_callback_state(), and set_task_work_schedule_callback_state() -
and adds a deliberate exception, map_value_has_static_identity(), that
keeps ID 0 for top-level one-element ARRAY maps.

Does this test file exercise all those paths?

Looking at the programs, they all use bpf_for_each_map_elem, so they
only exercise the for-each constructor. Nothing here loads a
bpf_timer/bpf_wq/bpf_task_work callback whose map value carries a
bpf_spin_lock.

Also, all three programs iterate maps with max_entries==2, or inner maps
with a non-zero map_uid, so map_value_has_static_identity() returns
false on every path the tests reach. The ID-zero exception is untested.

Should additional test cases be added to cover:
- timer/task-work callback constructors with spin locks?
- the map_value_has_static_identity() exception (single-element top-level
  ARRAY)?

A regression that removed the !map_value_has_static_identity() guard, or
that dropped the assignment from the timer/task-work constructors, would
still pass this test file.


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