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

[PATCH bpf v3 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:
----------
v2 -> v3
v2: https://lore.kernel.org/bpf/20260905083418.3723623-1-memxor@gmail.com

 * Propagate cancellation from constant blinding through both JIT fallback
   paths instead of rechecking fatal signals in bpf_check(). (Eduard)
 * Preserve packet-pointer displacement by comparing range bases, without
   extending the generic ID map. Veristat showed identical verdicts and
   successful-program instruction/state counts across 2773 loads. (Eduard,
   Alexei)
 * Reduce the packet pruning regression to 20 instructions and force state
   checkpoints. (Alexei, BPF CI)
 * Reject unsupported CO-RE poisoning targets in the shared relocation
   code instead of adding a CFG fall-through check. (Alexei)
 * Cover unsupported poison targets and supported relocations in dead code,
   including both halves of ldimm64.
 * Assign callback value IDs unconditionally and compare inner-map lookup
   IDs through check_ids(); explain the bug with a small program. (Eduard)
 * Move map_uid beside the other IDs and shrink frameno to preserve the
   register state size, keeping the existing memcmp() ranges.
 * Consolidate callback tests into the existing spinlock tests and reuse
   their map fixtures. Retain one-element and nested locking controls, and
   check nonzero IDs in timer, workqueue, and task-work callbacks. Clarify
   the inner-map lookup test description. (BPF CI)

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: Restrict CO-RE poisoning to relocatable instructions
  selftests/bpf: Test CO-RE instruction poisoning restrictions
  bpf: Assign lock identity to callback map values
  selftests/bpf: Check callback map value lock identity

 include/linux/bpf_verifier.h                  |  24 +--
 kernel/bpf/core.c                             |  19 +-
 kernel/bpf/fixups.c                           |  24 ++-
 kernel/bpf/states.c                           |   9 +-
 kernel/bpf/verifier.c                         |  15 +-
 tools/lib/bpf/relo_core.c                     |  28 +--
 .../selftests/bpf/prog_tests/cb_refs.c        |   2 +-
 .../selftests/bpf/prog_tests/core_reloc_raw.c | 168 ++++++++++++++++++
 .../selftests/bpf/prog_tests/spin_lock.c      |  33 +++-
 .../selftests/bpf/progs/test_spin_lock_fail.c | 167 ++++++++++++++++-
 .../progs/verifier_xdp_direct_packet_access.c |  35 ++++
 11 files changed, 477 insertions(+), 47 deletions(-)


base-commit: ee363e055895364039bf28348ff13c615afad4ba
-- 
2.53.0

[PATCH bpf v3 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.

Report interrupted constant blinding as -EINTR and propagate it through
both JIT paths, including kernels that permit interpreter fallback.
Other blinding failures retain the existing fallback behavior.

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/core.c   | 19 ++++++++++++++-----
 kernel/bpf/fixups.c | 24 ++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 8b294dfc1ad4..df1eb43d1933 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -19,6 +19,7 @@
 
 #include <uapi/linux/btf.h>
 #include <linux/filter.h>
+#include <linux/sched/signal.h>
 #include <linux/skbuff.h>
 #include <linux/static_call.h>
 #include <linux/vmalloc.h>
@@ -1619,6 +1620,8 @@ struct bpf_prog *bpf_jit_blind_constants(struct bpf_verifier_env *env, struct bp
 			 * fix it up here on error.
 			 */
 			bpf_jit_prog_release_other(prog, clone);
+			if (env && fatal_signal_pending(current))
+				return ERR_PTR(-EINTR);
 			return IS_ERR(tmp) ? tmp : ERR_PTR(-ENOMEM);
 		}
 
@@ -2625,7 +2628,8 @@ static bool bpf_prog_select_interpreter(struct bpf_prog *fp)
 	return select_interpreter;
 }
 
-static struct bpf_prog *bpf_prog_jit_compile(struct bpf_verifier_env *env, struct bpf_prog *prog)
+static struct bpf_prog *bpf_prog_jit_compile(struct bpf_verifier_env *env, struct bpf_prog *prog,
+					     int *err)
 {
 #ifdef CONFIG_BPF_JIT
 	struct bpf_prog *orig_prog;
@@ -2636,11 +2640,14 @@ static struct bpf_prog *bpf_prog_jit_compile(struct bpf_verifier_env *env, struc
 	orig_prog = prog;
 	prog = bpf_jit_blind_constants(env, prog);
 	/*
-	 * If blinding was requested and we failed during blinding, we must fall
-	 * back to the interpreter.
+	 * Fall back to the interpreter after blinding failures, except when
+	 * the loader was killed.
 	 */
-	if (IS_ERR(prog))
+	if (IS_ERR(prog)) {
+		if (PTR_ERR(prog) == -EINTR)
+			*err = -EINTR;
 		goto out_restore;
+	}
 
 	prog = bpf_int_jit_compile(env, prog);
 	if (prog->jited) {
@@ -2681,8 +2688,10 @@ struct bpf_prog *__bpf_prog_select_runtime(struct bpf_verifier_env *env, struct
 		if (*err)
 			return fp;
 
-		fp = bpf_prog_jit_compile(env, fp);
+		fp = bpf_prog_jit_compile(env, fp, err);
 		bpf_prog_jit_attempt_done(fp);
+		if (*err)
+			return fp;
 		if (!fp->jited && jit_needed) {
 			*err = -ENOTSUPP;
 			return fp;
diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
index 52d3cec33672..d6f83521fc78 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);
 
@@ -1356,7 +1376,7 @@ int bpf_jit_subprogs(struct bpf_verifier_env *env)
 		}
 		prog = bpf_jit_blind_constants(env, prog);
 		if (IS_ERR(prog)) {
-			err = -ENOMEM;
+			err = PTR_ERR(prog);
 			prog = orig_prog;
 			goto out_restore;
 		}
@@ -1433,7 +1453,7 @@ int bpf_fixup_call_args(struct bpf_verifier_env *env)
 		err = bpf_jit_subprogs(env);
 		if (err == 0)
 			return 0;
-		if (err == -EFAULT)
+		if (err == -EFAULT || err == -EINTR)
 			return err;
 	}
 #ifndef CONFIG_BPF_JIT_ALWAYS_ON
-- 
2.53.0

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

[bot+bpf-ci]

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

> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 8b294dfc1ad4d..df1eb43d19336 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c

[ ... ]

> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 52d3cec336727..d6f83521fc78e 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;
> +}

This isn't a bug, but would it read more clearly to keep the cond_resched()
visible at the two call sites, or to name this so the reschedule side effect
is apparent?

The function currently sounds like a pure query ("must abort?"), but it also
performs a cond_resched() as a side effect when returning false. Compare
with jit_subprogs() in this same file, which spells its two reschedule
points out as bare cond_resched() statements, making the preemption visible
where it happens.

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

[ ... ]

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

[ ... ]


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

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

[Eduard Zingerman]

On Mon, 2026-09-14 at 15:16 +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.
> 
> Report interrupted constant blinding as -EINTR and propagate it through
> both JIT paths, including kernels that permit interpreter fallback.
> Other blinding failures retain the existing fallback behavior.
> 
> 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>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

...

> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 8b294dfc1ad4..df1eb43d1933 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c

...

> @@ -2625,7 +2628,8 @@ static bool bpf_prog_select_interpreter(struct bpf_prog *fp)
>  	return select_interpreter;
>  }
>  
> -static struct bpf_prog *bpf_prog_jit_compile(struct bpf_verifier_env *env, struct bpf_prog *prog)
> +static struct bpf_prog *bpf_prog_jit_compile(struct bpf_verifier_env *env, struct bpf_prog *prog,
> +					     int *err)

Nit: in case if there would be a respin because of the CO-RE handling,
     I'd suggest to avoid adding the `err` parameter and using ERR_PTR
     mechanics instead.

>  {
>  #ifdef CONFIG_BPF_JIT
>  	struct bpf_prog *orig_prog;

...

[PATCH bpf v3 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.

Require matching range bases for packet pointers with an ID. Together
with the existing ID mapping, this preserves the displacement between
members of each packet-pointer class without adding per-ID state.
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>
---
 kernel/bpf/states.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
index 66fb11b6c6a7..6c88ad95b63b 100644
--- a/kernel/bpf/states.c
+++ b/kernel/bpf/states.c
@@ -635,6 +635,9 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
 		/* id relations must be preserved */
 		if (!check_ids(rold->id, rcur->id, idmap))
 			return false;
+		/* Preserve displacements between pointers sharing an ID. */
+		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);
-- 
2.53.0

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

[bot+bpf-ci]

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

> diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
> index 66fb11b6c6a76..6c88ad95b63b7 100644
> --- a/kernel/bpf/states.c
> +++ b/kernel/bpf/states.c
> @@ -635,6 +635,9 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
>  		/* id relations must be preserved */
>  		if (!check_ids(rold->id, rcur->id, idmap))
>  			return false;
> +		/* Preserve displacements between pointers sharing an ID. */
> +		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 this check stricter than what 022ac0750883 removed?  That commit
removed the .off equality check from regsafe() to make pointer
widening and loop convergence easier. The new rold->r64.base !=
rcur->r64.base comparison preserves displacement, which is what
safety requires, but it also pins the absolute base of the variable
part. Consider:

  explored:  r2 = {base 0,  size 15}, r3 = {base 8,  size 15}
  current:   r2 = {base 4,  size 3},  r3 = {base 12, size 3}

The displacement is 8 in both states, and range_within()/tnum_in()
pass for both registers, so pruning is sound. But rold->r64.base !=
rcur->r64.base (0 != 4 for r2, 8 != 12 for r3) rejects it.

Before 022ac0750883 this pair pruned because the removed check
compared only the constant part (rold->off vs rcur->off). Any path
that narrows a packet pointer's variable offset from below now fails
to prune, so those states are re-explored. For programs already near
the state limit this can turn a previously accepted program into a
load failure.

The minimal condition would compare the base difference within a class
rather than the absolute base, which the commit message rules out
("without adding per-ID state"). But it doesn't mention that the
resulting check is stronger than the removed .off check, or that
pruning of packet pointers with variable offsets is affected. Would it
be worth either relaxing the check (per-ID base delta) or documenting
the pruning impact in the changelog?


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

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

[Eduard Zingerman]

On Mon, 2026-09-14 at 15:16 +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.
> 
> Require matching range bases for packet pointers with an ID. Together
> with the existing ID mapping, this preserves the displacement between
> members of each packet-pointer class without adding per-ID state.
> 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>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

...

[PATCH bpf v3 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.

Read the unknown offset and branch selector directly from XDP context
fields, and force state checkpoints so the pruning attempt does not
depend on the verifier checkpoint heuristics.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../progs/verifier_xdp_direct_packet_access.c | 35 +++++++++++++++++++
 1 file changed, 35 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..9866bc154194 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
@@ -1719,4 +1719,39 @@ l0_%=:	r0 = 0;						\
 	: __clobber_all);
 }
 
+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) __flag(BPF_F_TEST_STATE_FREQ)
+__naked void pkt_regsafe_class_displacement(void)
+{
+	asm volatile ("					\
+	r8 = *(u32 *)(r1 + %[xdp_md_data_end]);		\
+	r9 = *(u32 *)(r1 + %[xdp_md_data]);		\
+	r4 = *(u32 *)(r1 + %[xdp_md_rx_queue_index]);	\
+	r4 &= 15;					\
+	r0 = *(u32 *)(r1 + %[xdp_md_ingress_ifindex]);	\
+	if r0 != 0 goto l0_%=;				\
+	r2 = r9;					\
+	r2 += r4;					\
+	r3 = r2;					\
+	r3 += 8;					\
+	goto l1_%=;					\
+l0_%=:	r4 &= 3;					\
+	r4 += 8;					\
+	r2 = r9;					\
+	r2 += r4;					\
+	r3 = r2;					\
+l1_%=:	if r3 > r8 goto l2_%=;				\
+	r0 = *(u64 *)(r2 + 0);				\
+l2_%=:	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)),
+	  __imm_const(xdp_md_rx_queue_index, offsetof(struct xdp_md, rx_queue_index)),
+	  __imm_const(xdp_md_ingress_ifindex, offsetof(struct xdp_md, ingress_ifindex))
+	: __clobber_all);
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.53.0

[PATCH bpf v3 4/7] bpf: Restrict CO-RE poisoning to relocatable instructions

[Kumar Kartikeya Dwivedi]

check_subprogs() verifies that each subprogram ends in an exit or an
unconditional jump before in-kernel CO-RE relocations are applied.
An unresolved relocation can then replace that terminal instruction with
an invalid helper call. The resulting fall-through into another subprogram
breaks the CFG invariant used by postorder and stack liveness analysis,
which can write past their per-subprogram arrays.

Handle instruction poisoning after the instruction has passed the same
class and operand-form checks used for a resolved relocation. This rejects
relocations targeting jumps, calls, exits, register-source arithmetic, or
non-immediate loads before they can alter the control flow. Keep poisoning
supported instructions, including both halves of a plain ldimm64, so an
unresolved relocation in dead code remains valid.

The shared relocation code applies this restriction to both libbpf and
in-kernel CO-RE.

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>
---
 tools/lib/bpf/relo_core.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
index 8ad2715721cf..a884bc2bff19 100644
--- a/tools/lib/bpf/relo_core.c
+++ b/tools/lib/bpf/relo_core.c
@@ -1047,17 +1047,6 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
 
 	class = BPF_CLASS(insn->code);
 
-	if (res->poison) {
-poison:
-		/* poison second part of ldimm64 to avoid confusing error from
-		 * verifier about "unknown opcode 00"
-		 */
-		if (is_ldimm64_insn(insn))
-			bpf_core_poison_insn(prog_name, relo_idx, insn_idx + 1, insn + 1);
-		bpf_core_poison_insn(prog_name, relo_idx, insn_idx, insn);
-		return 0;
-	}
-
 	orig_val = res->orig_val;
 	new_val = res->new_val;
 
@@ -1066,6 +1055,8 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
 	case BPF_ALU64:
 		if (BPF_SRC(insn->code) != BPF_K)
 			return -EINVAL;
+		if (res->poison)
+			goto poison;
 		if (res->validate && insn->imm != orig_val) {
 			pr_warn("prog '%s': relo #%d: unexpected insn #%d (ALU/ALU64) value: got %d, exp %llu -> %llu\n",
 				prog_name, relo_idx,
@@ -1082,6 +1073,8 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
 	case BPF_LDX:
 	case BPF_ST:
 	case BPF_STX:
+		if (res->poison)
+			goto poison;
 		if (res->validate && insn->off != orig_val) {
 			pr_warn("prog '%s': relo #%d: unexpected insn #%d (LDX/ST/STX) value: got %d, exp %llu -> %llu\n",
 				prog_name, relo_idx, insn_idx, insn->off, (unsigned long long)orig_val,
@@ -1140,6 +1133,9 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
 			return -EINVAL;
 		}
 
+		if (res->poison)
+			goto poison;
+
 		imm = (__u32)insn[0].imm | ((__u64)insn[1].imm << 32);
 		if (res->validate && imm != orig_val) {
 			pr_warn("prog '%s': relo #%d: unexpected insn #%d (LDIMM64) value: got %llu, exp %llu -> %llu\n",
@@ -1164,6 +1160,16 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
 	}
 
 	return 0;
+
+poison:
+	/*
+	 * Only relocatable instructions reach here. Poison both halves of
+	 * ldimm64 so the verifier reports the bad relocation, not opcode 00.
+	 */
+	if (is_ldimm64_insn(insn))
+		bpf_core_poison_insn(prog_name, relo_idx, insn_idx + 1, insn + 1);
+	bpf_core_poison_insn(prog_name, relo_idx, insn_idx, insn);
+	return 0;
 }
 
 /* Output spec definition in the format:
-- 
2.53.0

Re: [PATCH bpf v3 4/7] bpf: Restrict CO-RE poisoning to relocatable instructions

[sashiko-bot]

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

Pre-existing issues:
- [High] Heap out-of-bounds read and write in libbpf when processing CO-RE relocations targeting a truncated ldimm64 instruction at the end of an ELF section.
--

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

bpf: Restrict CO-RE poisoning to relocatable instructions

This patch restricts CO-RE poisoning to relocatable instructions. It handles
instruction poisoning after the instruction has passed the class and operand
checks, preventing unresolved relocations from altering the control flow by
poisoning instructions like exits or jumps.

> diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
> index 8ad2715721cfd..a884bc2bff198 100644
> --- a/tools/lib/bpf/relo_core.c
> +++ b/tools/lib/bpf/relo_core.c

[ ... ]

> @@ -1140,6 +1133,9 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
>  			return -EINVAL;
>  		}
>  
> +		if (res->poison)
> +			goto poison;
> +
>  		imm = (__u32)insn[0].imm | ((__u64)insn[1].imm << 32);

[Severity: High]
This isn't a bug introduced by this patch, but can this code cause an
out-of-bounds read?

Just before the new poison check in bpf_core_patch_insn(), the code validates
the BPF_LD instruction:

	case BPF_LD: {
		__u64 imm;

		if (!is_ldimm64_insn(insn) ||
		    insn[0].src_reg != 0 || insn[0].off != 0 ||
		    insn[1].code != 0 || insn[1].dst_reg != 0 ||
		    insn[1].src_reg != 0 || insn[1].off != 0) {

If an attacker crafts a malicious BPF ELF file where the final 8 bytes of an
ELF section form the first half of a BPF_LD instruction, prog->insns will be
allocated to end exactly at this half-instruction.

When processing a CO-RE relocation targeting this final instruction, would
evaluating insn[1] lead to a heap out-of-bounds read?

[ ... ]

> @@ -1164,6 +1160,16 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
>  	}
>  
>  	return 0;
> +
> +poison:
> +	/*
> +	 * Only relocatable instructions reach here. Poison both halves of
> +	 * ldimm64 so the verifier reports the bad relocation, not opcode 00.
> +	 */
> +	if (is_ldimm64_insn(insn))
> +		bpf_core_poison_insn(prog_name, relo_idx, insn_idx + 1, insn + 1);
> +	bpf_core_poison_insn(prog_name, relo_idx, insn_idx, insn);
> +	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but could this poison logic cause an
out-of-bounds write?

If a relocation targeting a truncated ldimm64 instruction is unresolvable and
marked for poisoning, execution reaches this poison block. Since
is_ldimm64_insn(insn) only checks the opcode of insn[0], it will evaluate to
true. 

Could bpf_core_poison_insn() then overwrite insn + 1 with a BPF_CALL
instruction past the end of the prog->insns array?

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

Re: [PATCH bpf v3 4/7] bpf: Restrict CO-RE poisoning to relocatable instructions

[bot+bpf-ci]

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

> diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
> index 8ad2715721cfd..a884bc2bff198 100644
> --- a/tools/lib/bpf/relo_core.c
> +++ b/tools/lib/bpf/relo_core.c
> @@ -1047,17 +1047,6 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
>
>  	class = BPF_CLASS(insn->code);
>
> -	if (res->poison) {
> -poison:
> -		/* poison second part of ldimm64 to avoid confusing error from
> -		 * verifier about "unknown opcode 00"
> -		 */
> -		if (is_ldimm64_insn(insn))
> -			bpf_core_poison_insn(prog_name, relo_idx, insn_idx + 1, insn + 1);
> -		bpf_core_poison_insn(prog_name, relo_idx, insn_idx, insn);
> -		return 0;
> -	}
> -
>  	orig_val = res->orig_val;
>  	new_val = res->new_val;
>
> @@ -1066,6 +1055,8 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
>  	case BPF_ALU64:
>  		if (BPF_SRC(insn->code) != BPF_K)
>  			return -EINVAL;
> +		if (res->poison)
> +			goto poison;

Moving the poison handling after the class/operand-form checks makes the
silent return -EINVAL reachable for unresolved relocations. Would adding a
pr_warn() on this path help with debugging?

Before this change, relocations with res->poison set jumped to the poison
label at the top of the function and always returned 0 without reaching
the BPF_SRC check. Now an unresolved relocation targeting a register-source
ALU or ALU64 instruction (BPF_MOV32_REG, BPF_MOV64_REG, or
BPF_ALU|BPF_END|BPF_TO_BE which encodes BPF_SRC == BPF_X) fails program
load with -EINVAL and nothing in the verifier or libbpf log explaining why.

The other rejection paths in this function do log diagnostics. The LDIMM64
form check at line 1129 does pr_warn("... (LDIMM64) has unexpected form"),
and the default: case at line 1155 does pr_warn("... trying to relocate
unrecognized insn #%d, code:0x%x ..."). This newly-reachable path is the
only one that returns -EINVAL silently.

The new selftest at f4f5a2c61d21 shows the gap. For poison_exit it can
assert ASSERT_HAS_SUBSTR(log, "trying to relocate unrecognized insn #2"),
but for poison_alu_reg and poison_alu64_reg it can only assert prog_fd < 0
and that "substituting insn" is absent, because no message is produced.

>  		if (res->validate && insn->imm != orig_val) {
>  			pr_warn("prog '%s': relo #%d: unexpected insn #%d (ALU/ALU64) value: got %d, exp %llu -> %llu\n",
>  				prog_name, relo_idx,

[ ... ]

> @@ -1164,6 +1160,16 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
>  	}
>
>  	return 0;
> +
> +poison:
> +	/*
> +	 * Only relocatable instructions reach here. Poison both halves of
> +	 * ldimm64 so the verifier reports the bad relocation, not opcode 00.
> +	 */
> +	if (is_ldimm64_insn(insn))
> +		bpf_core_poison_insn(prog_name, relo_idx, insn_idx + 1, insn + 1);
> +	bpf_core_poison_insn(prog_name, relo_idx, insn_idx, insn);
> +	return 0;
>  }
>
>  /* Output spec definition in the format:


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

Re: [PATCH bpf v3 4/7] bpf: Restrict CO-RE poisoning to relocatable instructions

[Eduard Zingerman]

On Mon, 2026-09-14 at 14:15 +0000, bot+bpf-ci@kernel.org wrote:

...

> > @@ -1066,6 +1055,8 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
> >  	case BPF_ALU64:
> >  		if (BPF_SRC(insn->code) != BPF_K)
> >  			return -EINVAL;
> > +		if (res->poison)
> > +			goto poison;
> 
> Moving the poison handling after the class/operand-form checks makes the
> silent return -EINVAL reachable for unresolved relocations. Would adding a
> pr_warn() on this path help with debugging?
> 
> Before this change, relocations with res->poison set jumped to the poison
> label at the top of the function and always returned 0 without reaching
> the BPF_SRC check. Now an unresolved relocation targeting a register-source
> ALU or ALU64 instruction (BPF_MOV32_REG, BPF_MOV64_REG, or
> BPF_ALU|BPF_END|BPF_TO_BE which encodes BPF_SRC == BPF_X) fails program
> load with -EINVAL and nothing in the verifier or libbpf log explaining why.

Ok, this might be a legit nit.
Add a goto default switch branch instead of `return -EINVAL;`
in the BPF_ALU64 case?

...

Re: [PATCH bpf v3 4/7] bpf: Restrict CO-RE poisoning to relocatable instructions

[Andrii Nakryiko]

On Mon, Sep 14, 2026 at 6:17 AM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> check_subprogs() verifies that each subprogram ends in an exit or an
> unconditional jump before in-kernel CO-RE relocations are applied.
> An unresolved relocation can then replace that terminal instruction with
> an invalid helper call. The resulting fall-through into another subprogram
> breaks the CFG invariant used by postorder and stack liveness analysis,
> which can write past their per-subprogram arrays.
>
> Handle instruction poisoning after the instruction has passed the same
> class and operand-form checks used for a resolved relocation. This rejects
> relocations targeting jumps, calls, exits, register-source arithmetic, or
> non-immediate loads before they can alter the control flow. Keep poisoning
> supported instructions, including both halves of a plain ldimm64, so an
> unresolved relocation in dead code remains valid.
>
> The shared relocation code applies this restriction to both libbpf and
> in-kernel CO-RE.
>
> 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>
> ---
>  tools/lib/bpf/relo_core.c | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
>

Kumar, can you please see if we can move check_core_relo() to right
after bpf_prepare_btf_info() instead of trying to guard against
malicious CO-RE relocation? I think what CO-RE logic is doing here
with poisoning is correct, it marks instruction as invalid because
CO-RE relocation couldn't be resolved/applied/whatever. That's sane
generic behavior, IMO.

The fact that this breaks subprog validation is more of a sign that we
do this CO-RE application too late, perhaps?

Also looking at current bpf_check_btf_info() logic, seems like we'll
skip CO-RE relocations altogether if we don't have
func_info/line_info, which is... fine, I guess, in modern world. But
it certainly is a bit orthogonal. So check_core_relo() seems like a
stand-alone step to me, which hopefully can be done very-very early,
so that no matter what patching we did will be properly validated by
verifier.

> diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
> index 8ad2715721cf..a884bc2bff19 100644
> --- a/tools/lib/bpf/relo_core.c
> +++ b/tools/lib/bpf/relo_core.c
> @@ -1047,17 +1047,6 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
>
>         class = BPF_CLASS(insn->code);
>
> -       if (res->poison) {
> -poison:
> -               /* poison second part of ldimm64 to avoid confusing error from
> -                * verifier about "unknown opcode 00"
> -                */
> -               if (is_ldimm64_insn(insn))
> -                       bpf_core_poison_insn(prog_name, relo_idx, insn_idx + 1, insn + 1);
> -               bpf_core_poison_insn(prog_name, relo_idx, insn_idx, insn);
> -               return 0;
> -       }
> -
>         orig_val = res->orig_val;
>         new_val = res->new_val;
>
> @@ -1066,6 +1055,8 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
>         case BPF_ALU64:
>                 if (BPF_SRC(insn->code) != BPF_K)
>                         return -EINVAL;
> +               if (res->poison)
> +                       goto poison;
>                 if (res->validate && insn->imm != orig_val) {
>                         pr_warn("prog '%s': relo #%d: unexpected insn #%d (ALU/ALU64) value: got %d, exp %llu -> %llu\n",
>                                 prog_name, relo_idx,
> @@ -1082,6 +1073,8 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
>         case BPF_LDX:
>         case BPF_ST:
>         case BPF_STX:
> +               if (res->poison)
> +                       goto poison;
>                 if (res->validate && insn->off != orig_val) {
>                         pr_warn("prog '%s': relo #%d: unexpected insn #%d (LDX/ST/STX) value: got %d, exp %llu -> %llu\n",
>                                 prog_name, relo_idx, insn_idx, insn->off, (unsigned long long)orig_val,
> @@ -1140,6 +1133,9 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
>                         return -EINVAL;
>                 }
>
> +               if (res->poison)
> +                       goto poison;
> +
>                 imm = (__u32)insn[0].imm | ((__u64)insn[1].imm << 32);
>                 if (res->validate && imm != orig_val) {
>                         pr_warn("prog '%s': relo #%d: unexpected insn #%d (LDIMM64) value: got %llu, exp %llu -> %llu\n",
> @@ -1164,6 +1160,16 @@ int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
>         }
>
>         return 0;
> +
> +poison:
> +       /*
> +        * Only relocatable instructions reach here. Poison both halves of
> +        * ldimm64 so the verifier reports the bad relocation, not opcode 00.
> +        */
> +       if (is_ldimm64_insn(insn))
> +               bpf_core_poison_insn(prog_name, relo_idx, insn_idx + 1, insn + 1);
> +       bpf_core_poison_insn(prog_name, relo_idx, insn_idx, insn);
> +       return 0;
>  }
>
>  /* Output spec definition in the format:
> --
> 2.53.0
>

Re: [PATCH bpf v3 4/7] bpf: Restrict CO-RE poisoning to relocatable instructions

[Alexei Starovoitov]

On Mon, Sep 14, 2026 at 12:54 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Sep 14, 2026 at 6:17 AM Kumar Kartikeya Dwivedi
> <memxor@gmail.com> wrote:
> >
> > check_subprogs() verifies that each subprogram ends in an exit or an
> > unconditional jump before in-kernel CO-RE relocations are applied.
> > An unresolved relocation can then replace that terminal instruction with
> > an invalid helper call. The resulting fall-through into another subprogram
> > breaks the CFG invariant used by postorder and stack liveness analysis,
> > which can write past their per-subprogram arrays.
> >
> > Handle instruction poisoning after the instruction has passed the same
> > class and operand-form checks used for a resolved relocation. This rejects
> > relocations targeting jumps, calls, exits, register-source arithmetic, or
> > non-immediate loads before they can alter the control flow. Keep poisoning
> > supported instructions, including both halves of a plain ldimm64, so an
> > unresolved relocation in dead code remains valid.
> >
> > The shared relocation code applies this restriction to both libbpf and
> > in-kernel CO-RE.
> >
> > 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>
> > ---
> >  tools/lib/bpf/relo_core.c | 28 +++++++++++++++++-----------
> >  1 file changed, 17 insertions(+), 11 deletions(-)
> >
>
> Kumar, can you please see if we can move check_core_relo() to right
> after bpf_prepare_btf_info() instead of trying to guard against
> malicious CO-RE relocation? I think what CO-RE logic is doing here
> with poisoning is correct, it marks instruction as invalid because
> CO-RE relocation couldn't be resolved/applied/whatever. That's sane
> generic behavior, IMO.
>
> The fact that this breaks subprog validation is more of a sign that we
> do this CO-RE application too late, perhaps?
>
> Also looking at current bpf_check_btf_info() logic, seems like we'll
> skip CO-RE relocations altogether if we don't have
> func_info/line_info, which is... fine, I guess, in modern world. But
> it certainly is a bit orthogonal. So check_core_relo() seems like a
> stand-alone step to me, which hopefully can be done very-very early,
> so that no matter what patching we did will be properly validated by
> verifier.

Let's do both:
- move bpf_check_btf_info() right after bpf_prepare_btf_info()
- check that relo applies to relocatable insns only
  and hard error if it's not.

Re: [PATCH bpf v3 4/7] bpf: Restrict CO-RE poisoning to relocatable instructions

[Kumar Kartikeya Dwivedi]

On Mon Sep 14, 2026 at 10:21 PM CEST, Alexei Starovoitov wrote:
> On Mon, Sep 14, 2026 at 12:54 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
>>
>> On Mon, Sep 14, 2026 at 6:17 AM Kumar Kartikeya Dwivedi
>> <memxor@gmail.com> wrote:
>> >
>> > check_subprogs() verifies that each subprogram ends in an exit or an
>> > unconditional jump before in-kernel CO-RE relocations are applied.
>> > An unresolved relocation can then replace that terminal instruction with
>> > an invalid helper call. The resulting fall-through into another subprogram
>> > breaks the CFG invariant used by postorder and stack liveness analysis,
>> > which can write past their per-subprogram arrays.
>> >
>> > Handle instruction poisoning after the instruction has passed the same
>> > class and operand-form checks used for a resolved relocation. This rejects
>> > relocations targeting jumps, calls, exits, register-source arithmetic, or
>> > non-immediate loads before they can alter the control flow. Keep poisoning
>> > supported instructions, including both halves of a plain ldimm64, so an
>> > unresolved relocation in dead code remains valid.
>> >
>> > The shared relocation code applies this restriction to both libbpf and
>> > in-kernel CO-RE.
>> >
>> > 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>
>> > ---
>> >  tools/lib/bpf/relo_core.c | 28 +++++++++++++++++-----------
>> >  1 file changed, 17 insertions(+), 11 deletions(-)
>> >
>>
>> Kumar, can you please see if we can move check_core_relo() to right
>> after bpf_prepare_btf_info() instead of trying to guard against
>> malicious CO-RE relocation? I think what CO-RE logic is doing here
>> with poisoning is correct, it marks instruction as invalid because
>> CO-RE relocation couldn't be resolved/applied/whatever. That's sane
>> generic behavior, IMO.
>>
>> The fact that this breaks subprog validation is more of a sign that we
>> do this CO-RE application too late, perhaps?
>>
>> Also looking at current bpf_check_btf_info() logic, seems like we'll
>> skip CO-RE relocations altogether if we don't have
>> func_info/line_info, which is... fine, I guess, in modern world. But
>> it certainly is a bit orthogonal. So check_core_relo() seems like a
>> stand-alone step to me, which hopefully can be done very-very early,
>> so that no matter what patching we did will be properly validated by
>> verifier.
>
> Let's do both:
> - move bpf_check_btf_info() right after bpf_prepare_btf_info()

This won't be a copy paste, since it depends on various checks on subprogs we
would end up skipping, but let me see how we can split some of the steps and
whether that ends up looking too bad.

> - check that relo applies to relocatable insns only
>   and hard error if it's not.

Ok.

Re: [PATCH bpf v3 4/7] bpf: Restrict CO-RE poisoning to relocatable instructions

[Andrii Nakryiko]

On Mon, Sep 14, 2026 at 1:42 PM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> On Mon Sep 14, 2026 at 10:21 PM CEST, Alexei Starovoitov wrote:
> > On Mon, Sep 14, 2026 at 12:54 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> >>
> >> On Mon, Sep 14, 2026 at 6:17 AM Kumar Kartikeya Dwivedi
> >> <memxor@gmail.com> wrote:
> >> >
> >> > check_subprogs() verifies that each subprogram ends in an exit or an
> >> > unconditional jump before in-kernel CO-RE relocations are applied.
> >> > An unresolved relocation can then replace that terminal instruction with
> >> > an invalid helper call. The resulting fall-through into another subprogram
> >> > breaks the CFG invariant used by postorder and stack liveness analysis,
> >> > which can write past their per-subprogram arrays.
> >> >
> >> > Handle instruction poisoning after the instruction has passed the same
> >> > class and operand-form checks used for a resolved relocation. This rejects
> >> > relocations targeting jumps, calls, exits, register-source arithmetic, or
> >> > non-immediate loads before they can alter the control flow. Keep poisoning
> >> > supported instructions, including both halves of a plain ldimm64, so an
> >> > unresolved relocation in dead code remains valid.
> >> >
> >> > The shared relocation code applies this restriction to both libbpf and
> >> > in-kernel CO-RE.
> >> >
> >> > 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>
> >> > ---
> >> >  tools/lib/bpf/relo_core.c | 28 +++++++++++++++++-----------
> >> >  1 file changed, 17 insertions(+), 11 deletions(-)
> >> >
> >>
> >> Kumar, can you please see if we can move check_core_relo() to right
> >> after bpf_prepare_btf_info() instead of trying to guard against
> >> malicious CO-RE relocation? I think what CO-RE logic is doing here
> >> with poisoning is correct, it marks instruction as invalid because
> >> CO-RE relocation couldn't be resolved/applied/whatever. That's sane
> >> generic behavior, IMO.
> >>
> >> The fact that this breaks subprog validation is more of a sign that we
> >> do this CO-RE application too late, perhaps?
> >>
> >> Also looking at current bpf_check_btf_info() logic, seems like we'll
> >> skip CO-RE relocations altogether if we don't have
> >> func_info/line_info, which is... fine, I guess, in modern world. But
> >> it certainly is a bit orthogonal. So check_core_relo() seems like a
> >> stand-alone step to me, which hopefully can be done very-very early,
> >> so that no matter what patching we did will be properly validated by
> >> verifier.
> >
> > Let's do both:
> > - move bpf_check_btf_info() right after bpf_prepare_btf_info()
>
> This won't be a copy paste, since it depends on various checks on subprogs we
> would end up skipping, but let me see how we can split some of the steps and
> whether that ends up looking too bad.
>
> > - check that relo applies to relocatable insns only
> >   and hard error if it's not.
>
> Ok.

let's also extract poisoning logic into separate function, so we have
less goto-ing unnecessarily

if (res->poison)
    return bpf_core_poison_insn(prog_name, relo_idx, insn, insn_idx);


and bpf_core_poison_insn() will be free to return error, if necessary

Re: [PATCH bpf v3 4/7] bpf: Restrict CO-RE poisoning to relocatable instructions

[Eduard Zingerman]

On Mon, 2026-09-14 at 15:16 +0200, Kumar Kartikeya Dwivedi wrote:
> check_subprogs() verifies that each subprogram ends in an exit or an
> unconditional jump before in-kernel CO-RE relocations are applied.
> An unresolved relocation can then replace that terminal instruction with
> an invalid helper call. The resulting fall-through into another subprogram
> breaks the CFG invariant used by postorder and stack liveness analysis,
> which can write past their per-subprogram arrays.
> 
> Handle instruction poisoning after the instruction has passed the same
> class and operand-form checks used for a resolved relocation. This rejects
> relocations targeting jumps, calls, exits, register-source arithmetic, or
> non-immediate loads before they can alter the control flow. Keep poisoning
> supported instructions, including both halves of a plain ldimm64, so an
> unresolved relocation in dead code remains valid.
> 
> The shared relocation code applies this restriction to both libbpf and
> in-kernel CO-RE.
> 
> 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>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

I think this commit is fine.
The error code is returned and the diagnostics is show if instruction
cannot be patched.

[PATCH bpf v3 5/7] selftests/bpf: Test CO-RE instruction poisoning restrictions

[Kumar Kartikeya Dwivedi]

Add raw CO-RE relocations that fail to resolve their target enum value.
Check that a relocation targeting a subprogram's terminal exit fails at
the relocation step, before poisoning can create cross-subprogram
fall-through.

Also place each supported and unsupported instruction class in dead code.
Unsupported targets must fail relocation even when unreachable. Supported
ALU immediates, memory accesses, and ldimm64 instructions must still be
poisoned and removed as dead code, allowing the program to load. Check
that both halves of ldimm64 are poisoned.

Load every program without relocations first to ensure that rejection is
caused by the relocation rather than the original instruction stream.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/prog_tests/core_reloc_raw.c | 168 ++++++++++++++++++
 1 file changed, 168 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..158b3e2e3f5f 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,173 @@
 
 static char log[16 * 1024];
 
+static int load_core_relo_insns(int btf_fd, struct bpf_insn *insns, int insn_cnt,
+				struct bpf_func_info *funcs, int func_cnt,
+				int enum_id, int access_str_off, int insn_idx,
+				bool relocate)
+{
+	struct bpf_core_relo relo = {
+		.insn_off = insn_idx * 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 = insn_cnt,
+		.insns = (__u64)insns,
+		.license = (__u64)"GPL",
+		.log_buf = (__u64)log,
+		.log_size = sizeof(log),
+		.log_level = 2,
+		.prog_btf_fd = btf_fd,
+		.func_info_rec_size = sizeof(struct bpf_func_info),
+		.func_info = (__u64)funcs,
+		.func_info_cnt = func_cnt,
+	};
+
+	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_insns(void)
+{
+	static const struct {
+		const char *name;
+		struct bpf_insn insns[2];
+		bool relocatable;
+	} tests[] = {
+		{ "poison_exit", { BPF_EXIT_INSN() } },
+		{ "poison_ja", { BPF_JMP_A(1) } },
+		{ "poison_jmp", { BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1) } },
+		{ "poison_jmp32", { BPF_JMP32_IMM(BPF_JEQ, BPF_REG_0, 0, 1) } },
+		{ "poison_call", { BPF_EMIT_CALL(BPF_FUNC_get_prandom_u32) } },
+		{ "poison_alu_reg", { BPF_MOV32_REG(BPF_REG_0, BPF_REG_1) } },
+		{ "poison_alu64_reg", { BPF_MOV64_REG(BPF_REG_0, BPF_REG_1) } },
+		{ "poison_ld_abs", { BPF_LD_ABS(BPF_W, 0) } },
+		{ "poison_alu_imm", { BPF_MOV32_IMM(BPF_REG_0, 0) }, true },
+		{ "poison_alu64_imm", { BPF_MOV64_IMM(BPF_REG_0, 0) }, true },
+		{ "poison_ldx", { BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1, 0) }, true },
+		{ "poison_st", { BPF_ST_MEM(BPF_W, BPF_REG_10, -4, 0) }, true },
+		{ "poison_stx", { BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -4) }, true },
+		{ "poison_ldimm64", { BPF_LD_IMM64(BPF_REG_0, 0) }, true },
+	};
+	struct bpf_func_info funcs[2];
+	struct bpf_insn subprog[] = {
+		BPF_CALL_REL(2),
+		BPF_MOV64_IMM(BPF_REG_0, 0),
+		BPF_EXIT_INSN(),
+		BPF_MOV64_IMM(BPF_REG_0, 0),
+		BPF_EXIT_INSN(),
+	};
+	const void *raw_btf;
+	struct btf *btf;
+	__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, i;
+
+	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_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;
+	funcs[0] = (struct bpf_func_info) { .insn_off = 0, .type_id = main_id };
+	funcs[1] = (struct bpf_func_info) { .insn_off = 3, .type_id = sub_id };
+
+	if (test__start_subtest("poisoned_subprog_terminator")) {
+		prog_fd = load_core_relo_insns(btf_fd, subprog, ARRAY_SIZE(subprog), funcs, 2,
+					       enum_id, access_str_off, 2, false);
+		if (!ASSERT_GE(prog_fd, 0, "control_load"))
+			goto cleanup;
+		close(prog_fd);
+		prog_fd = load_core_relo_insns(btf_fd, subprog, ARRAY_SIZE(subprog), funcs, 2,
+					       enum_id, access_str_off, 2, true);
+		if (!ASSERT_LT(prog_fd, 0, "poisoned_load"))
+			goto cleanup;
+		ASSERT_HAS_SUBSTR(log, "trying to relocate unrecognized insn #2",
+				  "poisoned_load_log");
+	}
+
+	for (i = 0; i < ARRAY_SIZE(tests); i++) {
+		struct bpf_insn insns[] = {
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
+			tests[i].insns[0],
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		};
+		int insn_cnt = ARRAY_SIZE(insns);
+
+		if (!test__start_subtest(tests[i].name))
+			continue;
+		if (insns[2].code == (BPF_LD | BPF_DW | BPF_IMM)) {
+			insns[3] = tests[i].insns[1];
+		} else {
+			insns[1].off = 1;
+			insns[4] = BPF_EXIT_INSN();
+			insn_cnt--;
+		}
+		prog_fd = load_core_relo_insns(btf_fd, insns, insn_cnt, funcs, 1,
+					       enum_id, access_str_off, 2, false);
+		if (!ASSERT_GE(prog_fd, 0, "control_load"))
+			goto cleanup;
+		close(prog_fd);
+		prog_fd = load_core_relo_insns(btf_fd, insns, insn_cnt, funcs, 1,
+					       enum_id, access_str_off, 2, true);
+		if (tests[i].relocatable) {
+			ASSERT_GE(prog_fd, 0, "dead_poison_load");
+			ASSERT_HAS_SUBSTR(log, "substituting insn #2", "poison_log");
+			if (insns[2].code == (BPF_LD | BPF_DW | BPF_IMM))
+				ASSERT_HAS_SUBSTR(log, "substituting insn #3", "poison_ldimm64_log");
+		} else {
+			ASSERT_LT(prog_fd, 0, "invalid_poison_load");
+			ASSERT_NULL(strstr(log, "substituting insn"), "invalid_poison_log");
+		}
+		close(prog_fd);
+		prog_fd = -1;
+	}
+
+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 +287,7 @@ static void test_bad_local_id(void)
 
 void test_core_reloc_raw(void)
 {
+	test_poisoned_insns();
 	if (test__start_subtest("bad_local_id"))
 		test_bad_local_id();
 }
-- 
2.53.0

Re: [PATCH bpf v3 5/7] selftests/bpf: Test CO-RE instruction poisoning restrictions

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2384 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 a18d3680fb169..158b3e2e3f5f7 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,173 @@
> +static void test_poisoned_insns(void)
> +{

[ ... ]

> +	for (i = 0; i < ARRAY_SIZE(tests); i++) {
> +		struct bpf_insn insns[] = {
> +			BPF_MOV64_IMM(BPF_REG_0, 0),
> +			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
> +			tests[i].insns[0],
> +			BPF_MOV64_IMM(BPF_REG_0, 0),
> +			BPF_MOV64_IMM(BPF_REG_0, 0),
> +			BPF_EXIT_INSN(),
> +		};
> +		int insn_cnt = ARRAY_SIZE(insns);
> +
> +		if (!test__start_subtest(tests[i].name))
> +			continue;
> +		if (insns[2].code == (BPF_LD | BPF_DW | BPF_IMM)) {
> +			insns[3] = tests[i].insns[1];
> +		} else {
> +			insns[1].off = 1;
> +			insns[4] = BPF_EXIT_INSN();
> +			insn_cnt--;
> +		}
> +		prog_fd = load_core_relo_insns(btf_fd, insns, insn_cnt, funcs, 1,
> +					       enum_id, access_str_off, 2, false);
> +		if (!ASSERT_GE(prog_fd, 0, "control_load"))
> +			goto cleanup;
> +		close(prog_fd);
> +		prog_fd = load_core_relo_insns(btf_fd, insns, insn_cnt, funcs, 1,
> +					       enum_id, access_str_off, 2, true);
> +		if (tests[i].relocatable) {
> +			ASSERT_GE(prog_fd, 0, "dead_poison_load");
> +			ASSERT_HAS_SUBSTR(log, "substituting insn #2", "poison_log");
> +			if (insns[2].code == (BPF_LD | BPF_DW | BPF_IMM))
> +				ASSERT_HAS_SUBSTR(log, "substituting insn #3", "poison_ldimm64_log");
                                    ^^^^

This isn't a bug, but the ldimm64 predicate (insns[2].code ==
(BPF_LD | BPF_DW | BPF_IMM)) appears in two places: once to set
up insns[3], and again to check for the "substituting insn #3"
message. Would a local variable or an extra flag in the tests[]
table make the two-slot instruction case easier to follow?

> +		} else {
> +			ASSERT_LT(prog_fd, 0, "invalid_poison_load");
> +			ASSERT_NULL(strstr(log, "substituting insn"), "invalid_poison_log");
> +		}
> +		close(prog_fd);
> +		prog_fd = -1;
> +	}

[ ... ]


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

Re: [PATCH bpf v3 5/7] selftests/bpf: Test CO-RE instruction poisoning restrictions

[Eduard Zingerman]

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

On Mon, 2026-09-14 at 15:16 +0200, Kumar Kartikeya Dwivedi wrote:

...

> +static void test_poisoned_insns(void)
> +{
> +	static const struct {
> +		const char *name;
> +		struct bpf_insn insns[2];
> +		bool relocatable;
> +	} tests[] = {
> +		{ "poison_exit", { BPF_EXIT_INSN() } },
> +		{ "poison_ja", { BPF_JMP_A(1) } },
> +		{ "poison_jmp", { BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1) } },
> +		{ "poison_jmp32", { BPF_JMP32_IMM(BPF_JEQ, BPF_REG_0, 0, 1) } },
> +		{ "poison_call", { BPF_EMIT_CALL(BPF_FUNC_get_prandom_u32) } },
> +		{ "poison_alu_reg", { BPF_MOV32_REG(BPF_REG_0, BPF_REG_1) } },
> +		{ "poison_alu64_reg", { BPF_MOV64_REG(BPF_REG_0, BPF_REG_1) } },
> +		{ "poison_ld_abs", { BPF_LD_ABS(BPF_W, 0) } },
> +		{ "poison_alu_imm", { BPF_MOV32_IMM(BPF_REG_0, 0) }, true },
> +		{ "poison_alu64_imm", { BPF_MOV64_IMM(BPF_REG_0, 0) }, true },
> +		{ "poison_ldx", { BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1, 0) }, true },
> +		{ "poison_st", { BPF_ST_MEM(BPF_W, BPF_REG_10, -4, 0) }, true },
> +		{ "poison_stx", { BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -4) }, true },
> +		{ "poison_ldimm64", { BPF_LD_IMM64(BPF_REG_0, 0) }, true },
> +	};
> +	struct bpf_func_info funcs[2];
> +	struct bpf_insn subprog[] = {
> +		BPF_CALL_REL(2),
> +		BPF_MOV64_IMM(BPF_REG_0, 0),
> +		BPF_EXIT_INSN(),
> +		BPF_MOV64_IMM(BPF_REG_0, 0),
> +		BPF_EXIT_INSN(),
> +	};
> +	const void *raw_btf;
> +	struct btf *btf;
> +	__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, i;
> +
> +	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_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;

Nit: We have a BTF_*_ENC series of macro to assist with this in selftests.
     See attached diff.

> +
> +	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;
> +	funcs[0] = (struct bpf_func_info) { .insn_off = 0, .type_id = main_id };
> +	funcs[1] = (struct bpf_func_info) { .insn_off = 3, .type_id = sub_id };
> +
> +	if (test__start_subtest("poisoned_subprog_terminator")) {
> +		prog_fd = load_core_relo_insns(btf_fd, subprog, ARRAY_SIZE(subprog), funcs, 2,
> +					       enum_id, access_str_off, 2, false);
> +		if (!ASSERT_GE(prog_fd, 0, "control_load"))
> +			goto cleanup;
> +		close(prog_fd);
> +		prog_fd = load_core_relo_insns(btf_fd, subprog, ARRAY_SIZE(subprog), funcs, 2,
> +					       enum_id, access_str_off, 2, true);
> +		if (!ASSERT_LT(prog_fd, 0, "poisoned_load"))
> +			goto cleanup;
> +		ASSERT_HAS_SUBSTR(log, "trying to relocate unrecognized insn #2",
> +				  "poisoned_load_log");
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(tests); i++) {
> +		struct bpf_insn insns[] = {
> +			BPF_MOV64_IMM(BPF_REG_0, 0),
> +			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
> +			tests[i].insns[0],
> +			BPF_MOV64_IMM(BPF_REG_0, 0),
> +			BPF_MOV64_IMM(BPF_REG_0, 0),
> +			BPF_EXIT_INSN(),
> +		};
> +		int insn_cnt = ARRAY_SIZE(insns);
> +
> +		if (!test__start_subtest(tests[i].name))
> +			continue;
> +		if (insns[2].code == (BPF_LD | BPF_DW | BPF_IMM)) {
> +			insns[3] = tests[i].insns[1];
> +		} else {
> +			insns[1].off = 1;
> +			insns[4] = BPF_EXIT_INSN();
> +			insn_cnt--;

Nit: this else block is not really necessary.

> +		}
> +		prog_fd = load_core_relo_insns(btf_fd, insns, insn_cnt, funcs, 1,
> +					       enum_id, access_str_off, 2, false);
> +		if (!ASSERT_GE(prog_fd, 0, "control_load"))
> +			goto cleanup;
> +		close(prog_fd);
> +		prog_fd = load_core_relo_insns(btf_fd, insns, insn_cnt, funcs, 1,
> +					       enum_id, access_str_off, 2, true);
> +		if (tests[i].relocatable) {
> +			ASSERT_GE(prog_fd, 0, "dead_poison_load");
> +			ASSERT_HAS_SUBSTR(log, "substituting insn #2", "poison_log");
> +			if (insns[2].code == (BPF_LD | BPF_DW | BPF_IMM))
> +				ASSERT_HAS_SUBSTR(log, "substituting insn #3", "poison_ldimm64_log");
> +		} else {
> +			ASSERT_LT(prog_fd, 0, "invalid_poison_load");
> +			ASSERT_NULL(strstr(log, "substituting insn"), "invalid_poison_log");
> +		}
> +		close(prog_fd);
> +		prog_fd = -1;
> +	}

...

[-- Attachment #2: btf-enc.diff --]
[-- Type: text/x-patch, Size: 3552 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 158b3e2e3f5f..aeb3dd2c7b72 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc_raw.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc_raw.c
@@ -70,7 +70,35 @@ static void test_poisoned_insns(void)
 		{ "poison_stx", { BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -4) }, true },
 		{ "poison_ldimm64", { BPF_LD_IMM64(BPF_REG_0, 0) }, true },
 	};
-	struct bpf_func_info funcs[2];
+	struct test_btf {
+		struct btf_header hdr;
+		__u32 types[18];
+		char strings[64];
+	} raw_btf = {
+		.hdr = {
+			.magic = BTF_MAGIC,
+			.version = BTF_VERSION,
+			.hdr_len = sizeof(struct btf_header),
+			.type_off = 0,
+			.type_len = sizeof(raw_btf.types),
+			.str_off = offsetof(struct test_btf, strings) -
+				   offsetof(struct test_btf, types),
+			.str_len = sizeof(raw_btf.strings),
+		},
+		.types = {
+			BTF_TYPE_INT_ENC(1, BTF_INT_SIGNED, 0, 32, 4), /* [1] int */
+			BTF_FUNC_PROTO_ENC(1, 0),	/* [2] int (*)(void) */
+			BTF_FUNC_ENC(5, 2),		/* [3] main_fn */
+			BTF_FUNC_ENC(13, 2),		/* [4] sub_fn */
+			BTF_TYPE_ENC(20, BTF_INFO_ENC(BTF_KIND_ENUM, 0, 1), 4), /* [5] enum */
+			BTF_ENUM_ENC(45, 0),		/* value = 0 */
+		},
+		.strings = "\0int\0main_fn\0sub_fn\0core_relo_poison_missing\0value\0" "0",
+	};
+	struct bpf_func_info funcs[] = {
+		{ .insn_off = 0, .type_id = 3 },
+		{ .insn_off = 3, .type_id = 4 },
+	};
 	struct bpf_insn subprog[] = {
 		BPF_CALL_REL(2),
 		BPF_MOV64_IMM(BPF_REG_0, 0),
@@ -78,43 +106,12 @@ static void test_poisoned_insns(void)
 		BPF_MOV64_IMM(BPF_REG_0, 0),
 		BPF_EXIT_INSN(),
 	};
-	const void *raw_btf;
-	struct btf *btf;
-	__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, i;
+	int access_str_off = 51, enum_id = 5;
+	int btf_fd, prog_fd = -1, i;
 
-	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_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);
+	btf_fd = bpf_btf_load(&raw_btf, sizeof(raw_btf), NULL);
 	if (!ASSERT_GE(btf_fd, 0, "btf_load"))
 		goto cleanup;
-	funcs[0] = (struct bpf_func_info) { .insn_off = 0, .type_id = main_id };
-	funcs[1] = (struct bpf_func_info) { .insn_off = 3, .type_id = sub_id };
 
 	if (test__start_subtest("poisoned_subprog_terminator")) {
 		prog_fd = load_core_relo_insns(btf_fd, subprog, ARRAY_SIZE(subprog), funcs, 2,
@@ -178,7 +175,6 @@ static void test_poisoned_insns(void)
 	}
 	close(prog_fd);
 	close(btf_fd);
-	btf__free(btf);
 }
 
 /* Check that verifier rejects BPF program containing relocation

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

[Kumar Kartikeya Dwivedi]

A nested bpf_for_each_map_elem() callback can unlock a different element
of the same map:

  static long inner(void *map, int *key, struct value *v,
                    struct value **outer_value)
  {
          bpf_spin_lock(&v->lock);
          bpf_spin_unlock(&(*outer_value)->lock);
          return 0;
  }

  static long outer(void *map, int *key, struct value *v, void *ctx)
  {
          bpf_for_each_map_elem(map, inner, &v, 0);
          return 0;
  }

Both callback values currently have ID zero and the same map_ptr.
process_spin_lock() compares those two fields, so it accepts the unlock
even though the two callbacks can receive different map elements.

Assign a fresh ID to every callback map value in the for-each,
timer/workqueue, and task-work constructors. Copies of one callback
argument retain its ID, so locking and unlocking through that argument
continues to work. Distinct callbacks also get distinct IDs for
single-element arrays, including inner arrays sharing inner_map_meta.

Preserve map_uid for every inner-map lookup and compare it through
check_ids() during state pruning. This preserves relationships between
maps, keys, and values while allowing equivalent states with different
lookup IDs to match. It avoids field-specific rules for when an inner map
needs an identity.

Move map_uid out of the metadata union and next to the other IDs, so
register comparisons can use the existing memcmp() ranges and remap the
IDs separately. Clear it when resetting a register or converting a map
lookup result to a socket pointer. Shrink frameno to u8, which is enough
for MAX_CALL_FRAMES, to make room without growing bpf_reg_state.

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>
---
 include/linux/bpf_verifier.h | 24 ++++++++++++------------
 kernel/bpf/states.c          |  6 ++++--
 kernel/bpf/verifier.c        | 15 ++++++++++-----
 3 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 36b65797877d..8f769ee14bd6 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -46,17 +46,10 @@ struct bpf_reg_state {
 		/* valid when type == PTR_TO_PACKET */
 		int range;
 
-		/* valid when type == CONST_PTR_TO_MAP | PTR_TO_MAP_VALUE |
-		 *   PTR_TO_MAP_VALUE_OR_NULL
+		/*
+		 * For CONST_PTR_TO_MAP, PTR_TO_MAP_KEY and PTR_TO_MAP_VALUE.
 		 */
-		struct {
-			struct bpf_map *map_ptr;
-			/* To distinguish map lookups from outer map
-			 * the map_uid is non-zero for registers
-			 * pointing to inner maps.
-			 */
-			u32 map_uid;
-		};
+		struct bpf_map *map_ptr;
 
 		/* for PTR_TO_BTF_ID */
 		struct {
@@ -155,13 +148,20 @@ struct bpf_reg_state {
 	 * gets parent_id set to the dynptr's id.
 	 */
 	u32 parent_id;
-	/* Inside the callee two registers can be both PTR_TO_STACK like
+	/*
+	 * Distinguishes inner-map lookups and their keys and values. Zero for
+	 * other registers. Kept outside the metadata union for ID remapping
+	 * during state comparisons.
+	 */
+	u32 map_uid;
+	/*
+	 * Inside the callee two registers can be both PTR_TO_STACK like
 	 * R1=fp-8 and R2=fp-8, but one of them points to this function stack
 	 * while another to the caller's stack. To differentiate them 'frameno'
 	 * is used which is an index in bpf_verifier_state->frame[] array
 	 * pointing to bpf_func_state.
 	 */
-	u32 frameno;
+	u8 frameno;
 	/* if (!precise && SCALAR_VALUE) min/max/tnum don't affect safety */
 	bool precise;
 };
diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
index 6c88ad95b63b..e4ec007f7fa6 100644
--- a/kernel/bpf/states.c
+++ b/kernel/bpf/states.c
@@ -491,7 +491,8 @@ static bool regs_exact(const struct bpf_reg_state *rold,
 {
 	return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0 &&
 	       check_ids(rold->id, rcur->id, idmap) &&
-	       check_ids(rold->parent_id, rcur->parent_id, idmap);
+	       check_ids(rold->parent_id, rcur->parent_id, idmap) &&
+	       check_ids(rold->map_uid, rcur->map_uid, idmap);
 }
 
 enum exact_level {
@@ -616,7 +617,8 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
 		       range_within(rold, rcur) &&
 		       tnum_in(rold->var_off, rcur->var_off) &&
 		       check_ids(rold->id, rcur->id, idmap) &&
-		       check_ids(rold->parent_id, rcur->parent_id, idmap);
+		       check_ids(rold->parent_id, rcur->parent_id, idmap) &&
+		       check_ids(rold->map_uid, rcur->map_uid, idmap);
 	case PTR_TO_PACKET_META:
 	case PTR_TO_PACKET:
 		/* We must have at least as much range as the old ptr
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ddba53eaa333..ab4f0ea2647b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1864,6 +1864,7 @@ static void __mark_reg_known(struct bpf_reg_state *reg, u64 imm)
 	       offsetof(struct bpf_reg_state, var_off) - sizeof(reg->type));
 	reg->id = 0;
 	reg->parent_id = 0;
+	reg->map_uid = 0;
 	___mark_reg_known(reg, imm);
 }
 
@@ -1925,17 +1926,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.
+		/*
+		 * Each lookup gets a unique ID. Preserve it as the inner map's
+		 * identity, since different inner maps share inner_map_meta.
 		 */
-		if (btf_record_has_field(map->inner_map_meta->record,
-					 BPF_TIMER | BPF_WORKQUEUE | BPF_TASK_WORK))
-			reg->map_uid = reg->id;
+		reg->map_uid = reg->id;
 	} else if (map->map_type == BPF_MAP_TYPE_XSKMAP) {
 		reg->type = PTR_TO_XDP_SOCK | maybe_null;
+		reg->map_uid = 0;
 	} else if (map->map_type == BPF_MAP_TYPE_SOCKMAP ||
 		   map->map_type == BPF_MAP_TYPE_SOCKHASH) {
 		reg->type = PTR_TO_SOCKET | maybe_null;
+		reg->map_uid = 0;
 	}
 }
 
@@ -10048,6 +10050,7 @@ 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;
+	callee->regs[BPF_REG_3].id = ++env->id_gen;
 
 	/* pointer to stack or null */
 	callee->regs[BPF_REG_4] = caller->regs[BPF_REG_3];
@@ -10144,6 +10147,7 @@ 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;
+	callee->regs[BPF_REG_3].id = ++env->id_gen;
 
 	/* unused */
 	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
@@ -10262,6 +10266,7 @@ 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;
+	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 v3 6/7] bpf: Assign lock identity to callback map values

[bot+bpf-ci]

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

> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 36b65797877d0..8f769ee14bd65 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -46,17 +46,10 @@ struct bpf_reg_state {
>  		/* valid when type == PTR_TO_PACKET */
>  		int range;
>
> -		/* valid when type == CONST_PTR_TO_MAP | PTR_TO_MAP_VALUE |
> -		 *   PTR_TO_MAP_VALUE_OR_NULL
> +		/*
> +		 * For CONST_PTR_TO_MAP, PTR_TO_MAP_KEY and PTR_TO_MAP_VALUE.
>  		 */
> -		struct {
> -			struct bpf_map *map_ptr;
> -			/* To distinguish map lookups from outer map
> -			 * the map_uid is non-zero for registers
> -			 * pointing to inner maps.
> -			 */
> -			u32 map_uid;
> -		};
> +		struct bpf_map *map_ptr;
>
>  		/* for PTR_TO_BTF_ID */
>  		struct {
> @@ -155,13 +148,20 @@ struct bpf_reg_state {
>  	 * gets parent_id set to the dynptr's id.
>  	 */
>  	u32 parent_id;
> -	/* Inside the callee two registers can be both PTR_TO_STACK like
> +	/*
> +	 * Distinguishes inner-map lookups and their keys and values. Zero for
> +	 * other registers. Kept outside the metadata union for ID remapping
> +	 * during state comparisons.
> +	 */
> +	u32 map_uid;
> +	/*
> +	 * Inside the callee two registers can be both PTR_TO_STACK like
>  	 * R1=fp-8 and R2=fp-8, but one of them points to this function stack
>  	 * while another to the caller's stack. To differentiate them 'frameno'
>  	 * is used which is an index in bpf_verifier_state->frame[] array
>  	 * pointing to bpf_func_state.
>  	 */
> -	u32 frameno;
> +	u8 frameno;

This isn't a bug, but would it be worth leaving the frameno comment in its
original wrapping so the hunk shows only the new map_uid field and the
frameno narrowing? The comment body is unchanged by this patch but is
reflowed from the '/* first line of text' opening used by the rest of
struct bpf_reg_state to a bare '/*' opening. The same reflow is applied
to the map_ptr union comment. The behavioral part of the hunk is the new
u32 map_uid field and the u32->u8 frameno narrowing; the comment
reformatting is orthogonal to that and enlarges the diff.

[ ... ]


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

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

[Eduard Zingerman]

On Mon, 2026-09-14 at 15:16 +0200, Kumar Kartikeya Dwivedi wrote:
> A nested bpf_for_each_map_elem() callback can unlock a different element
> of the same map:
> 
>   static long inner(void *map, int *key, struct value *v,
>                     struct value **outer_value)
>   {
>           bpf_spin_lock(&v->lock);
>           bpf_spin_unlock(&(*outer_value)->lock);
>           return 0;
>   }
> 
>   static long outer(void *map, int *key, struct value *v, void *ctx)
>   {
>           bpf_for_each_map_elem(map, inner, &v, 0);
>           return 0;
>   }
> 
> Both callback values currently have ID zero and the same map_ptr.
> process_spin_lock() compares those two fields, so it accepts the unlock
> even though the two callbacks can receive different map elements.
> 
> Assign a fresh ID to every callback map value in the for-each,
> timer/workqueue, and task-work constructors. Copies of one callback
> argument retain its ID, so locking and unlocking through that argument
> continues to work. Distinct callbacks also get distinct IDs for
> single-element arrays, including inner arrays sharing inner_map_meta.
> 
> Preserve map_uid for every inner-map lookup and compare it through
> check_ids() during state pruning. This preserves relationships between
> maps, keys, and values while allowing equivalent states with different
> lookup IDs to match. It avoids field-specific rules for when an inner map
> needs an identity.
> 
> Move map_uid out of the metadata union and next to the other IDs, so
> register comparisons can use the existing memcmp() ranges and remap the
> IDs separately. Clear it when resetting a register or converting a map
> lookup result to a socket pointer. Shrink frameno to u8, which is enough
> for MAX_CALL_FRAMES, to make room without growing bpf_reg_state.
> 
> 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>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

It appears all map_uid has to be reset are handled.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index ddba53eaa333..ab4f0ea2647b 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

...

> @@ -1925,17 +1926,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.
> +		/*
> +		 * Each lookup gets a unique ID. Preserve it as the inner map's
> +		 * identity, since different inner maps share inner_map_meta.
>  		 */

Nit: old comment is better, imo.

> -		if (btf_record_has_field(map->inner_map_meta->record,
> -					 BPF_TIMER | BPF_WORKQUEUE | BPF_TASK_WORK))
> -			reg->map_uid = reg->id;
> +		reg->map_uid = reg->id;
>  	} else if (map->map_type == BPF_MAP_TYPE_XSKMAP) {
>  		reg->type = PTR_TO_XDP_SOCK | maybe_null;
> +		reg->map_uid = 0;
>  	} else if (map->map_type == BPF_MAP_TYPE_SOCKMAP ||
>  		   map->map_type == BPF_MAP_TYPE_SOCKHASH) {
>  		reg->type = PTR_TO_SOCKET | maybe_null;
> +		reg->map_uid = 0;
>  	}
>  }
>  

...

[PATCH bpf v3 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 callbacks reached through two inner-map lookups. The lookup
results share inner_map_meta but may refer to different one-element
arrays, so their callback values must retain distinct lock identities.

Keep nested and one-element array callbacks as positive controls for
balanced locking through one callback argument.

For timer, workqueue, and task-work callbacks, lock and unlock the value,
then pass it to bpf_this_cpu_ptr() to expose its ID in the verifier error
log. Require a nonzero ID, following the existing lock identity tests.
Balanced locking alone would still succeed if a constructor stopped
assigning an ID.

Extend the existing spin_lock test table and reuse its array and inner-map
fixtures to keep the callback cases alongside the other lock identity tests.

Update the nested callback reference-leak expectation for the extra
callback value ID.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/prog_tests/cb_refs.c        |   2 +-
 .../selftests/bpf/prog_tests/spin_lock.c      |  33 +++-
 .../selftests/bpf/progs/test_spin_lock_fail.c | 167 +++++++++++++++++-
 3 files changed, 192 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/cb_refs.c b/tools/testing/selftests/bpf/prog_tests/cb_refs.c
index 78566b817fd7..490e15e7126d 100644
--- a/tools/testing/selftests/bpf/prog_tests/cb_refs.c
+++ b/tools/testing/selftests/bpf/prog_tests/cb_refs.c
@@ -13,7 +13,7 @@ struct {
 } cb_refs_tests[] = {
 	{ "underflow_prog", "release kfunc bpf_kfunc_call_test_release expects referenced PTR_TO_BTF_ID passed to R1" },
 	{ "leak_prog", "Possibly NULL pointer passed to helper R2" },
-	{ "nested_cb", "Unreleased reference id=4 alloc_insn=2" }, /* alloc_insn=2{4,5} */
+	{ "nested_cb", "Unreleased reference id=5 alloc_insn=2" }, /* alloc_insn=2{4,5} */
 	{ "non_cb_transfer_ref", "Unreleased reference id=4 alloc_insn=1" }, /* alloc_insn=1{1,2} */
 };
 
diff --git a/tools/testing/selftests/bpf/prog_tests/spin_lock.c b/tools/testing/selftests/bpf/prog_tests/spin_lock.c
index 5c3579438427..d49f52a8f822 100644
--- a/tools/testing/selftests/bpf/prog_tests/spin_lock.c
+++ b/tools/testing/selftests/bpf/prog_tests/spin_lock.c
@@ -10,8 +10,8 @@ static char log_buf[1024 * 1024];
 
 static struct {
 	const char *prog_name;
-	const char *err_msg;
-} spin_lock_fail_tests[] = {
+	const char *err_msg; /* NULL if loading should succeed. */
+} spin_lock_tests[] = {
 	{ "lock_id_kptr_preserve",
 	  "[0-9]\\+: (bf) r1 = r0                       ; R0=ptr_foo(id=2)"
 	  " R1=ptr_foo(id=2) refs=2\n"
@@ -54,6 +54,22 @@ static struct {
 	{ "lock_global_sleepable_helper_subprog", "global function calls are not allowed while holding a lock" },
 	{ "lock_global_sleepable_kfunc_subprog", "global function calls are not allowed while holding a lock" },
 	{ "lock_global_sleepable_subprog_indirect", "global function calls are not allowed while holding a lock" },
+	{ "callback_value_lock_identity", "bpf_spin_unlock of different lock" },
+	{ "callback_inner_map_value_lock_identity", "bpf_spin_unlock of different lock" },
+	{ "callback_value_lock_identity_same", NULL },
+	{ "callback_single_value_lock_identity_same", NULL },
+	{ "lock_id_timer_preserve",
+	  "R1=map_value(id=[1-9][0-9]*,map=async_lock_map,[^\n]*\n"
+	  "[0-9]\\+: (85) call bpf_this_cpu_ptr#154\n"
+	  "R1 type=map_value expected=percpu_ptr_" },
+	{ "lock_id_wq_preserve",
+	  "R1=map_value(id=[1-9][0-9]*,map=async_lock_map,[^\n]*\n"
+	  "[0-9]\\+: (85) call bpf_this_cpu_ptr#154\n"
+	  "R1 type=map_value expected=percpu_ptr_" },
+	{ "lock_id_task_work_preserve",
+	  "R1=map_value(id=[1-9][0-9]*,map=async_lock_map,[^\n]*\n"
+	  "[0-9]\\+: (85) call bpf_this_cpu_ptr#154\n"
+	  "R1 type=map_value expected=percpu_ptr_" },
 };
 
 static int match_regex(const char *pattern, const char *string)
@@ -74,7 +90,7 @@ static int match_regex(const char *pattern, const char *string)
 	return rc == 0 ? 1 : 0;
 }
 
-static void test_spin_lock_fail_prog(const char *prog_name, const char *err_msg)
+static void test_spin_lock_prog(const char *prog_name, const char *err_msg)
 {
 	LIBBPF_OPTS(bpf_object_open_opts, opts, .kernel_log_buf = log_buf,
 						.kernel_log_size = sizeof(log_buf),
@@ -94,6 +110,10 @@ static void test_spin_lock_fail_prog(const char *prog_name, const char *err_msg)
 	bpf_program__set_autoload(prog, true);
 
 	ret = test_spin_lock_fail__load(skel);
+	if (!err_msg) {
+		ASSERT_OK(ret, "test_spin_lock_fail__load");
+		goto end;
+	}
 	if (!ASSERT_ERR(ret, "test_spin_lock_fail__load must fail"))
 		goto end;
 
@@ -166,10 +186,9 @@ void test_spin_lock(void)
 
 	test_spin_lock_success();
 
-	for (i = 0; i < ARRAY_SIZE(spin_lock_fail_tests); i++) {
-		if (!test__start_subtest(spin_lock_fail_tests[i].prog_name))
+	for (i = 0; i < ARRAY_SIZE(spin_lock_tests); i++) {
+		if (!test__start_subtest(spin_lock_tests[i].prog_name))
 			continue;
-		test_spin_lock_fail_prog(spin_lock_fail_tests[i].prog_name,
-					 spin_lock_fail_tests[i].err_msg);
+		test_spin_lock_prog(spin_lock_tests[i].prog_name, spin_lock_tests[i].err_msg);
 	}
 }
diff --git a/tools/testing/selftests/bpf/progs/test_spin_lock_fail.c b/tools/testing/selftests/bpf/progs/test_spin_lock_fail.c
index f678ee6bd7ea..a27bbbe3523f 100644
--- a/tools/testing/selftests/bpf/progs/test_spin_lock_fail.c
+++ b/tools/testing/selftests/bpf/progs/test_spin_lock_fail.c
@@ -14,17 +14,18 @@ struct array_map {
 	__type(key, int);
 	__type(value, struct foo);
 	__uint(max_entries, 1);
-} array_map SEC(".maps");
+} array_map SEC(".maps"), array_map_b SEC(".maps");
 
 struct {
 	__uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
-	__uint(max_entries, 1);
+	__uint(max_entries, 2);
 	__type(key, int);
 	__type(value, int);
 	__array(values, struct array_map);
 } map_of_maps SEC(".maps") = {
 	.values = {
 		[0] = &array_map,
+		[1] = &array_map_b,
 	},
 };
 
@@ -314,4 +315,166 @@ int lock_global_sleepable_subprog_indirect(struct __sk_buff *ctx)
 	return ret;
 }
 
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 2);
+	__type(key, int);
+	__type(value, struct foo);
+} callback_array_map SEC(".maps");
+
+struct callback_ctx {
+	struct foo *value;
+};
+
+static long lock_different_value(struct bpf_map *map, int *key,
+				 struct foo *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 foo *value, void *data)
+{
+	struct callback_ctx ctx = { .value = value };
+
+	bpf_for_each_map_elem(&callback_array_map, lock_different_value, &ctx, 0);
+	return 0;
+}
+
+SEC("?tc")
+int callback_value_lock_identity(void *ctx)
+{
+	bpf_for_each_map_elem(&callback_array_map, nest_lock_different_value, NULL, 0);
+	return 0;
+}
+
+static long nest_lock_different_inner_value(struct bpf_map *map, int *key,
+					    struct foo *value, void *data)
+{
+	struct callback_ctx ctx = { .value = value };
+	int inner_key = 1;
+	void *inner_map;
+
+	inner_map = bpf_map_lookup_elem(&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")
+int callback_inner_map_value_lock_identity(void *ctx)
+{
+	int inner_key = 0;
+	void *inner_map;
+
+	inner_map = bpf_map_lookup_elem(&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 foo *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 foo *value, void *data)
+{
+	bpf_for_each_map_elem(&callback_array_map, lock_same_value, NULL, 0);
+	return 0;
+}
+
+SEC("?tc")
+int callback_value_lock_identity_same(void *ctx)
+{
+	bpf_for_each_map_elem(&callback_array_map, nest_lock_same_value, NULL, 0);
+	return 0;
+}
+
+SEC("?tc")
+int callback_single_value_lock_identity_same(void *ctx)
+{
+	bpf_for_each_map_elem(&array_map, lock_same_value, NULL, 0);
+	return 0;
+}
+
+struct async_lock_value {
+	struct bpf_spin_lock lock;
+	struct bpf_timer timer;
+	struct bpf_wq work;
+	struct bpf_task_work task_work;
+	int counter;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 2);
+	__type(key, int);
+	__type(value, struct async_lock_value);
+} async_lock_map SEC(".maps");
+
+/*
+ * Expose the callback value ID in the verifier log, as in lock_id_mapval_preserve().
+ */
+static int async_lock_id_preserve(void *map, int *key, void *data)
+{
+	struct async_lock_value *value = data;
+
+	bpf_spin_lock(&value->lock);
+	value->counter++;
+	bpf_spin_unlock(&value->lock);
+	bpf_this_cpu_ptr(value);
+	return 0;
+}
+
+SEC("?tc")
+int lock_id_timer_preserve(void *ctx)
+{
+	struct async_lock_value *value;
+	int key = 0;
+
+	value = bpf_map_lookup_elem(&async_lock_map, &key);
+	if (value && !bpf_timer_init(&value->timer, &async_lock_map, 1))
+		bpf_timer_set_callback(&value->timer, async_lock_id_preserve);
+	return 0;
+}
+
+SEC("?tc")
+int lock_id_wq_preserve(void *ctx)
+{
+	struct async_lock_value *value;
+	int key = 0;
+
+	value = bpf_map_lookup_elem(&async_lock_map, &key);
+	if (value && !bpf_wq_init(&value->work, &async_lock_map, 0))
+		bpf_wq_set_callback(&value->work, async_lock_id_preserve, 0);
+	return 0;
+}
+
+static int task_work_lock_id_preserve(struct bpf_map *map, void *key, void *data)
+{
+	return async_lock_id_preserve(map, key, data);
+}
+
+SEC("?tc")
+int lock_id_task_work_preserve(void *ctx)
+{
+	struct async_lock_value *value;
+	int key = 0;
+
+	value = bpf_map_lookup_elem(&async_lock_map, &key);
+	if (value)
+		bpf_task_work_schedule_resume(bpf_get_current_task_btf(), &value->task_work,
+					      &async_lock_map, task_work_lock_id_preserve);
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.53.0

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

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/cb_refs.c b/tools/testing/selftests/bpf/prog_tests/cb_refs.c
> index 78566b817fd70..490e15e7126d9 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cb_refs.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cb_refs.c

[ ... ]

> diff --git a/tools/testing/selftests/bpf/prog_tests/spin_lock.c b/tools/testing/selftests/bpf/prog_tests/spin_lock.c
> index 5c3579438427d..d49f52a8f8223 100644
> --- a/tools/testing/selftests/bpf/prog_tests/spin_lock.c
> +++ b/tools/testing/selftests/bpf/prog_tests/spin_lock.c
> @@ -10,8 +10,8 @@ static char log_buf[1024 * 1024];
>
>  static struct {
>  	const char *prog_name;
> -	const char *err_msg;
> -} spin_lock_fail_tests[] = {
> +	const char *err_msg; /* NULL if loading should succeed. */
> +} spin_lock_tests[] = {

[ ... ]

> @@ -54,6 +54,22 @@ static struct {
>  	{ "lock_global_sleepable_helper_subprog", "global function calls are not allowed while holding a lock" },
>  	{ "lock_global_sleepable_kfunc_subprog", "global function calls are not allowed while holding a lock" },
>  	{ "lock_global_sleepable_subprog_indirect", "global function calls are not allowed while holding a lock" },
> +	{ "callback_value_lock_identity", "bpf_spin_unlock of different lock" },
> +	{ "callback_inner_map_value_lock_identity", "bpf_spin_unlock of different lock" },
> +	{ "callback_value_lock_identity_same", NULL },
> +	{ "callback_single_value_lock_identity_same", NULL },
> +	{ "lock_id_timer_preserve",
> +	  "R1=map_value(id=[1-9][0-9]*,map=async_lock_map,[^\n]*\n"
> +	  "[0-9]\\+: (85) call bpf_this_cpu_ptr#154\n"
> +	  "R1 type=map_value expected=percpu_ptr_" },
> +	{ "lock_id_wq_preserve",
> +	  "R1=map_value(id=[1-9][0-9]*,map=async_lock_map,[^\n]*\n"
> +	  "[0-9]\\+: (85) call bpf_this_cpu_ptr#154\n"
> +	  "R1 type=map_value expected=percpu_ptr_" },
> +	{ "lock_id_task_work_preserve",
> +	  "R1=map_value(id=[1-9][0-9]*,map=async_lock_map,[^\n]*\n"
> +	  "[0-9]\\+: (85) call bpf_this_cpu_ptr#154\n"
> +	  "R1 type=map_value expected=percpu_ptr_" },
>  };

This isn't a bug, but could the three-line regex pattern for the async
lock tests be extracted into a shared define? The three entries repeat
the same literal character-for-character.

> @@ -74,7 +90,7 @@ static int match_regex(const char *pattern, const char *string)
>  	return rc == 0 ? 1 : 0;
>  }
>
> -static void test_spin_lock_fail_prog(const char *prog_name, const char *err_msg)
> +static void test_spin_lock_prog(const char *prog_name, const char *err_msg)
>  {

[ ... ]

> @@ -94,6 +110,10 @@ static void test_spin_lock_fail_prog(const char *prog_name, const char *err_msg)
>  	bpf_program__set_autoload(prog, true);
>
>  	ret = test_spin_lock_fail__load(skel);
> +	if (!err_msg) {
> +		ASSERT_OK(ret, "test_spin_lock_fail__load");
> +		goto end;
> +	}
>  	if (!ASSERT_ERR(ret, "test_spin_lock_fail__load must fail"))
>  		goto end;

This isn't a bug, but the table and helper were renamed from _fail to
accommodate success cases, while the skeleton calls, assertion tags,
and BPF object filename all keep the _fail naming. Is the partial
migration intentional?

> @@ -166,10 +186,9 @@ void test_spin_lock(void)
>
>  	test_spin_lock_success();
>
> -	for (i = 0; i < ARRAY_SIZE(spin_lock_fail_tests); i++) {
> -		if (!test__start_subtest(spin_lock_fail_tests[i].prog_name))
> +	for (i = 0; i < ARRAY_SIZE(spin_lock_tests); i++) {
> +		if (!test__start_subtest(spin_lock_tests[i].prog_name))
>  			continue;
> -		test_spin_lock_fail_prog(spin_lock_fail_tests[i].prog_name,
> -					 spin_lock_fail_tests[i].err_msg);
> +		test_spin_lock_prog(spin_lock_tests[i].prog_name, spin_lock_tests[i].err_msg);
>  	}
>  }

[ ... ]


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