BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state
@ 2026-08-05 18:44 Daniel Borkmann
  2026-08-05 18:44 ` [PATCH bpf-next 2/4] bpf: Mark pending zero extension of arena ptrs " Daniel Borkmann
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Daniel Borkmann @ 2026-08-05 18:44 UTC (permalink / raw)
  To: memxor; +Cc: eddyz87, puranjay, info, bpf

A 32-bit write records the writing instruction in reg->subreg_def, and a
later 64-bit read of that register calls mark_insn_zext() to record that
the definition has to be zero extended. Architectures whose JIT sets
bpf_jit_needs_zext() rely on that mark to emit the extension.

The mark is produced by walking the path from the definition to the read.
If the walk stops at a state equivalent to an already explored one, the
reads the explored path performs from there on are not repeated for this
path's registers, so a definition whose only 64-bit read lies beyond the
pruning point never gets marked and keeps a garbage upper half.

Example with BPF_F_TEST_STATE_FREQ making every instruction a checkpoint:

      r7 = *(u32 *)(r1 + offsetof(struct __sk_buff, len))
      r6 = 0        /* 64-bit define */
      if r7 != 0 goto l1
      goto l0                              path A, explored first
  l1: w6 = 0        /* 32-bit define */    path B, explored second
  l0: r0 = r6       /* 64-bit read   */
      r0 >>= 32
      exit

Now, path A is the fall-through of the conditional and is explored first.
It reaches l0 with r6 defined by the 64-bit r6 = 0, so reg->subreg_def is
DEF_NOT_SUBREG and the read marks nothing. The walk runs on to exit and
leaves a checkpoint at every instruction along the way. Path B is explored
second. w6 = 0 sets r6->subreg_def to that instruction, so a zero extension
is pending and only the 64-bit read at l0 can resolve it. B then arrives
at l0, where bpf_is_state_visited() finds the checkpoint A left behind:
r6 is the scalar 0 in both states, so they are equivalent and B is pruned
before r0 = r6 is verified.

Without the fix nothing happens at that point, so the one read that would
have called mark_insn_zext() for w6 is never walked and the definition
stays unmarked:

  l1: w6 = 0        /* subreg_def = w6, pending */
  l0: r0 = r6     <--- B pruned, read never walked, w6 stays unmarked

The JIT of an architecture that needs explicit zero extension then emits
none, and the upper half of w6's definition is left undefined (under
BPF_F_TEST_RND_HI32 it holds the randomized half, which r0 >>= 32 returns).
This used to be handled by the registers chain based liveness: the
propagate_liveness() called mark_insn_zext() for every parent register
whose read mark was REG_LIVE_READ64, which carried the requirement across
a pruned state. Commit 107e16979905 ("bpf: disable and remove registers
chain based liveness") removed that machinery and with it the propagation,
leaving the mark dependent on the path actually being walked.

Mark the pending definitions where the walk stops instead, i.e. on the way
into the prune rather than at the read that is no longer reached:

  l1: w6 = 0        /* subreg_def = w6, pending */
  l0: r0 = r6     <--- mark w6, r6 is live here

The set of registers to mark is the one the pruning decision was made on:
func_states_equal() compares the registers live at the instruction, per
insn_aux_data[].live_regs_before, and those are exactly the registers that
can still be read. A register that is not live there is never read again
and needs nothing. This is conservative in one direction: a live register
whose remaining reads are all 32-bit also gets its definition marked,
which costs a zero extension that is not needed. However, it never marks
too little, and it does not weaken pruning. On x86-64 the effect is only
observable with BPF_F_TEST_RND_HI32.

The one live scalar whose subreg_def can point at a call insn is r0 of a
kfunc returning a 32-bit value. Marking that one is harmless, the fixup pass
skips kfunc calls since their zero extension is done by the caller.

Fixes: 107e16979905 ("bpf: disable and remove registers chain based liveness")
Reported-by: STAR Labs SG <info@starlabs.sg>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 include/linux/bpf_verifier.h |  2 ++
 kernel/bpf/states.c          |  2 ++
 kernel/bpf/verifier.c        | 38 ++++++++++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index a2a40caca0a0..0952fa5649db 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1212,6 +1212,8 @@ void bpf_clear_singular_ids(struct bpf_verifier_env *env, struct bpf_verifier_st
 int bpf_mark_chain_precision(struct bpf_verifier_env *env,
 			     struct bpf_verifier_state *starting_state,
 			     int regno, bool *changed);
+void bpf_mark_live_subregs_zext(struct bpf_verifier_env *env,
+				struct bpf_verifier_state *vstate);
 
 static inline int bpf_get_spi(s32 off)
 {
diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
index ea2153cf28d0..24009a606249 100644
--- a/kernel/bpf/states.c
+++ b/kernel/bpf/states.c
@@ -1405,6 +1405,8 @@ int bpf_is_state_visited(struct bpf_verifier_env *env, int insn_idx)
 hit:
 			sl->hit_cnt++;
 
+			bpf_mark_live_subregs_zext(env, cur);
+
 			/* if previous state reached the exit with precision and
 			 * current state is equivalent to it (except precision marks)
 			 * the precision needs to be propagated back in
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 09588b7b08b0..e62b350b37af 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3161,6 +3161,44 @@ static void mark_insn_zext(struct bpf_verifier_env *env,
 	reg->subreg_def = DEF_NOT_SUBREG;
 }
 
+/*
+ * Reaching a state equivalent to an already explored one stops the walk, so a
+ * register that still carries a subreg_def may have its only 64-bit read on
+ * the path that is no longer walked, and without the mark the JIT of an
+ * architecture that needs explicit zero extension leaves the upper half of the
+ * definition undefined. Mark the definitions of the registers live at this
+ * instruction, i.e. the ones the equivalence was decided on and thus the only
+ * ones that can still be read. This is conservative in that a live register
+ * which is only ever read as a sub-register also gets its definition marked,
+ * at the cost of a zero extension that is not needed.
+ *
+ * Only scalars are considered since a live_regs_before bit does not imply that
+ * the register holds a readable value: the caller saved regs of a frame below
+ * the current one are clobbered to NOT_INIT at the call while keeping the
+ * subreg_def of the call insn. Such a definition must not be marked, the call
+ * insn has no destination register to zero extend.
+ */
+void bpf_mark_live_subregs_zext(struct bpf_verifier_env *env,
+				struct bpf_verifier_state *vstate)
+{
+	struct bpf_insn_aux_data *aux = env->insn_aux_data;
+	struct bpf_func_state *func;
+	u16 live_regs;
+	int i, j;
+
+	for (i = vstate->curframe; i >= 0; i--) {
+		live_regs = aux[bpf_frame_insn_idx(vstate, i)].live_regs_before;
+		func = vstate->frame[i];
+		for (j = 0; j < BPF_REG_FP; j++) {
+			if (!(live_regs & BIT(j)))
+				continue;
+			if (func->regs[j].type != SCALAR_VALUE)
+				continue;
+			mark_insn_zext(env, &func->regs[j]);
+		}
+	}
+}
+
 static int __check_reg_arg(struct bpf_verifier_env *env, struct bpf_reg_state *regs, u32 regno,
 			   enum bpf_reg_arg_type t)
 {
-- 
2.43.0


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

* [PATCH bpf-next 2/4] bpf: Mark pending zero extension of arena ptrs before pruning a state
  2026-08-05 18:44 [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state Daniel Borkmann
@ 2026-08-05 18:44 ` Daniel Borkmann
  2026-08-05 18:44 ` [PATCH bpf-next 3/4] selftests/bpf: Add tests for sub-register zext across state pruning Daniel Borkmann
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Daniel Borkmann @ 2026-08-05 18:44 UTC (permalink / raw)
  To: memxor; +Cc: eddyz87, puranjay, info, bpf

bpf_mark_live_subregs_zext() marks the definitions of the registers live
at a pruning point so that a subreg_def whose only 64-bit read lies beyond
the prune is not left unmarked, but only SCALAR_VALUE currently.

PTR_TO_ARENA is the one other type that carries a real subreg_def and can
be read as a full 64-bit value: the addr_space_cast to arena (cast_kern)
records the cast insn in subreg_def, and the later 64-bit use (a load,
store or ALU64 with the pointer as source) is what marks it. The arena
access computes its address as pointer + arena_vm_start and trusts the
pointer's upper half to be zero, e.g. an indexed 'llgc %dst,off(%src,%arena)'
on s390 or a 'src + arena_vm_start' add on riscv64 and x86-64.

On x86-64 and riscv64 that upper half is cleared regardless of the mark:
x86-64 zero extends natively, and the riscv64 JIT emits its own zextw for
the cast. On a pure bpf_jit_needs_zext() architecture such as s390 the
cast_kern emits nothing and relies solely on the mark driven BPF_ZEXT_REG.
Thus, also mark the definitions of live PTR_TO_ARENA registers.

Fixes: 107e16979905 ("bpf: disable and remove registers chain based liveness")
Reported-by: STAR Labs SG <info@starlabs.sg>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 kernel/bpf/verifier.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index e62b350b37af..4de9b464a9fc 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3172,11 +3172,14 @@ static void mark_insn_zext(struct bpf_verifier_env *env,
  * which is only ever read as a sub-register also gets its definition marked,
  * at the cost of a zero extension that is not needed.
  *
- * Only scalars are considered since a live_regs_before bit does not imply that
- * the register holds a readable value: the caller saved regs of a frame below
+ * Scalars and arena pointers are considered, the two types that carry a
+ * subreg_def and can still be read as a full 64-bit value past this point.
+ * The arena case matters on a pure bpf_jit_needs_zext() architecture. Other
+ * types are skipped since a live_regs_before bit does not imply that the
+ * register holds a readable value: the caller saved regs of a frame below
  * the current one are clobbered to NOT_INIT at the call while keeping the
- * subreg_def of the call insn. Such a definition must not be marked, the call
- * insn has no destination register to zero extend.
+ * subreg_def of the call insn. Such a definition must not be marked, the
+ * call insn has no destination register to zero extend.
  */
 void bpf_mark_live_subregs_zext(struct bpf_verifier_env *env,
 				struct bpf_verifier_state *vstate)
@@ -3192,7 +3195,8 @@ void bpf_mark_live_subregs_zext(struct bpf_verifier_env *env,
 		for (j = 0; j < BPF_REG_FP; j++) {
 			if (!(live_regs & BIT(j)))
 				continue;
-			if (func->regs[j].type != SCALAR_VALUE)
+			if (func->regs[j].type != SCALAR_VALUE &&
+			    func->regs[j].type != PTR_TO_ARENA)
 				continue;
 			mark_insn_zext(env, &func->regs[j]);
 		}
-- 
2.43.0


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

* [PATCH bpf-next 3/4] selftests/bpf: Add tests for sub-register zext across state pruning
  2026-08-05 18:44 [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state Daniel Borkmann
  2026-08-05 18:44 ` [PATCH bpf-next 2/4] bpf: Mark pending zero extension of arena ptrs " Daniel Borkmann
@ 2026-08-05 18:44 ` Daniel Borkmann
  2026-08-05 18:44 ` [PATCH bpf-next 4/4] selftests/bpf: Add test for arena pointer " Daniel Borkmann
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Daniel Borkmann @ 2026-08-05 18:44 UTC (permalink / raw)
  To: memxor; +Cc: eddyz87, puranjay, info, bpf

Add the example walked through in the previous patch as a test case: define
r6 twice, by a 64-bit write on the path the verifier explores first and by
a 32-bit write on the path explored second, and read it 64-bit after the
two paths meet. The second path is pruned at the merge, so the w6 definition
never reaches the 64-bit read, and its zero extension must be marked at the
pruning point instead.

The second test moves the pruning point into a callee, so that the marking
walks the caller frames as well. Their caller saved registers are NOT_INIT
while the callee runs, and marking those would set zext_dst on the call
insn, which has no destination register to zero extend.

The third test uses a BPF_CMPXCHG fetching into r0 as the 32-bit define.
Unlike the other definitions this one is patched by the fixup pass even
where the JIT does not ask for zero extension, so it also pins down that
the marks added at a pruning point reach x86-64.

  # LDLIBS=-static PKG_CONFIG='pkg-config --static' ./vmtest.sh -- ./test_progs -t subreg
  [...]
  #668/1   verifier_subreg/add32 reg zero extend check:OK
  #668/2   verifier_subreg/add32 reg zero extend check @unpriv:OK
  [...]
  #668/80  verifier_subreg/lsh32_imm31_value:OK
  #668/81  verifier_subreg/rsh32_imm31_value:OK
  #668/82  verifier_subreg/arsh32_imm31_value:OK
  #668/83  verifier_subreg/lsh32_unknown_precise_bounds:OK
  #668/84  verifier_subreg/rsh32_unknown_bounds:OK
  #668/85  verifier_subreg/subreg zero extend check across state pruning:OK
  #668/86  verifier_subreg/subreg zero extend check across state pruning in a callee:OK
  #668/87  verifier_subreg/subreg zero extend check across state pruning with cmpxchg:OK
  #668     verifier_subreg:OK
  Summary: 1/87 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 .../selftests/bpf/progs/verifier_subreg.c     | 88 +++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_subreg.c b/tools/testing/selftests/bpf/progs/verifier_subreg.c
index 73b5b0cf6706..101f2a8bff7f 100644
--- a/tools/testing/selftests/bpf/progs/verifier_subreg.c
+++ b/tools/testing/selftests/bpf/progs/verifier_subreg.c
@@ -3,6 +3,7 @@
 
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
+#include "../../../include/linux/filter.h"
 #include "bpf_misc.h"
 
 /* This file contains sub-register zero extension checks for insns defining
@@ -990,4 +991,91 @@ l0_%=:	r0 = r6;					\
 	: __clobber_all);
 }
 
+SEC("socket")
+__description("subreg zero extend check across state pruning")
+__flag(BPF_F_TEST_RND_HI32)
+__flag(BPF_F_TEST_STATE_FREQ)
+__success __retval(0)
+__naked void subreg_zero_extend_check_pruning(void)
+{
+	asm volatile ("					\
+	r7 = *(u32 *)(r1 + %[__sk_buff_len]);		\
+	r6 = 0;			/* 64-bit define */	\
+	if r7 != 0 goto l1_%=;				\
+	goto l0_%=;					\
+l1_%=:	w6 = 0;			/* 32-bit define */	\
+l0_%=:	r0 = r6;		/* 64-bit read */	\
+	r0 >>= 32;					\
+	exit;						\
+"	:
+	: __imm_const(__sk_buff_len, offsetof(struct __sk_buff, len))
+	: __clobber_all);
+}
+
+/*
+ * Same as the previous test, but with the pruning point inside a callee. The
+ * marking then also walks the caller frames, whose caller saved registers are
+ * NOT_INIT while the callee runs, and must not mark the call insn.
+ */
+SEC("socket")
+__description("subreg zero extend check across state pruning in a callee")
+__flag(BPF_F_TEST_RND_HI32)
+__flag(BPF_F_TEST_STATE_FREQ)
+__success __retval(0)
+__naked void subreg_zero_extend_check_pruning_callee(void)
+{
+	asm volatile ("					\
+	r1 = *(u32 *)(r1 + %[__sk_buff_len]);		\
+	call subreg_zero_extend_check_pruning_subprog;	\
+	r0 >>= 32;					\
+	exit;						\
+"	:
+	: __imm_const(__sk_buff_len, offsetof(struct __sk_buff, len))
+	: __clobber_all);
+}
+
+static __used __naked void subreg_zero_extend_check_pruning_subprog(void)
+{
+	asm volatile ("					\
+	r0 = 0;			/* 64-bit define */	\
+	if r1 != 0 goto l1_%=;				\
+	goto l0_%=;					\
+l1_%=:	w0 = 0;			/* 32-bit define */	\
+l0_%=:	exit;			/* 64-bit read */	\
+"	::: __clobber_all);
+}
+
+/*
+ * Same as the first test, but with the 32-bit define coming from a BPF_CMPXCHG
+ * fetching into r0. Unlike the other definitions this one is patched even where
+ * the JIT does not ask for zero extension, see bpf_opt_subreg_zext_lo32_rnd_hi32().
+ * The stack slot is left as STACK_MISC by the initial 32-bit store so that the
+ * cmpxchg does not alter it, otherwise the two paths would not converge.
+ */
+SEC("socket")
+__description("subreg zero extend check across state pruning with cmpxchg")
+__flag(BPF_F_TEST_RND_HI32)
+__flag(BPF_F_TEST_STATE_FREQ)
+__success __retval(0)
+__naked void subreg_zero_extend_check_pruning_cmpxchg(void)
+{
+	asm volatile ("					\
+	r7 = *(u32 *)(r1 + %[__sk_buff_len]);		\
+	r1 = 1;						\
+	*(u32 *)(r10 - 4) = r1;				\
+	call %[bpf_get_prandom_u32];	/* 64-bit define */\
+	if r7 != 0 goto l1_%=;				\
+	goto l0_%=;					\
+l1_%=:	r2 = 2;						\
+	.8byte %[cmpxchg32];		/* 32-bit define */\
+l0_%=:	r0 >>= 32;			/* 64-bit read */\
+	exit;						\
+"	:
+	: __imm(bpf_get_prandom_u32),
+	  __imm_const(__sk_buff_len, offsetof(struct __sk_buff, len)),
+	  __imm_insn(cmpxchg32,
+		     BPF_ATOMIC_OP(BPF_W, BPF_CMPXCHG, BPF_REG_10, BPF_REG_2, -4))
+	: __clobber_all);
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.43.0


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

* [PATCH bpf-next 4/4] selftests/bpf: Add test for arena pointer zext across state pruning
  2026-08-05 18:44 [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state Daniel Borkmann
  2026-08-05 18:44 ` [PATCH bpf-next 2/4] bpf: Mark pending zero extension of arena ptrs " Daniel Borkmann
  2026-08-05 18:44 ` [PATCH bpf-next 3/4] selftests/bpf: Add tests for sub-register zext across state pruning Daniel Borkmann
@ 2026-08-05 18:44 ` Daniel Borkmann
  2026-08-05 20:14   ` sashiko-bot
  2026-08-05 19:11 ` [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state Eduard Zingerman
  2026-08-05 20:22 ` sashiko-bot
  4 siblings, 1 reply; 9+ messages in thread
From: Daniel Borkmann @ 2026-08-05 18:44 UTC (permalink / raw)
  To: memxor; +Cc: eddyz87, puranjay, info, bpf

Add the arena counterpart to the sub-register zero extension pruning tests:
r6 is defined as an arena pointer twice, by a 64-bit copy on the path the
verifier explores first and by a 32-bit addr_space_cast on the path explored
second, and it is dereferenced only after the two paths meet. The second
path is pruned at the merge, so the cast never reaches the 64-bit read and
its zero extension has to be marked at the pruning point. The test is only
relevant for bpf_jit_needs_zext() architecture such as s390x.

  # ./vmtest.sh -- ./test_progs -t verifier_arena
  [...]
  #564/1   verifier_arena/basic_alloc1_nosleep:OK
  #564/2   verifier_arena/basic_alloc2_nosleep:OK
  #564/3   verifier_arena/basic_alloc3_nosleep:OK
  #564/4   verifier_arena/basic_reserve1_nosleep:OK
  #564/5   verifier_arena/basic_reserve2_nosleep:OK
  #564/6   verifier_arena/reserve_twice_nosleep:OK
  #564/7   verifier_arena/reserve_invalid_region_nosleep:OK
  #564/8   verifier_arena/subreg zero extend check across state pruning with arena pointer:OK
  #564/9   verifier_arena/basic_alloc1:OK
  [...]
  #564/26  verifier_arena/iter_maps3:OK
  #564     verifier_arena:OK
  Summary: 4/35 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 .../selftests/bpf/progs/verifier_arena.c      | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_arena.c b/tools/testing/selftests/bpf/progs/verifier_arena.c
index b241bbcf54a8..b5f5b81a27e7 100644
--- a/tools/testing/selftests/bpf/progs/verifier_arena.c
+++ b/tools/testing/selftests/bpf/progs/verifier_arena.c
@@ -635,6 +635,41 @@ int non_arena_ptr_add_to_arena_ptr(void *ctx)
 	return 0;
 }
 
+/*
+ * The verifier walks from a sub-register definition to its 64-bit read to mark
+ * the definition for zero extension. When the walk stops at a pruned state, the
+ * definitions live at that point must be marked there instead, and that set
+ * includes PTR_TO_ARENA: on a pure bpf_jit_needs_zext() architecture such as
+ * s390 the addr_space_cast defining an arena pointer emits no zero extension
+ * of its own and relies solely on the mark driven zero extension, so a missing
+ * mark otherwise leaves the pointer's upper half undefined.
+ */
+SEC("socket")
+__description("subreg zero extend check across state pruning with arena pointer")
+__flag(BPF_F_TEST_RND_HI32)
+__flag(BPF_F_TEST_STATE_FREQ)
+__success __retval(0)
+__naked void subreg_zero_extend_check_pruning_arena(void)
+{
+	asm volatile ("					\
+	r7 = *(u32 *)(r1 + %[__sk_buff_len]);		\
+	r9 = %[arena] ll;				\
+	r2 = 0;						\
+	r2 = addr_space_cast(r2, 0x0, 0x1);		\
+	r6 = r2;		/* 64-bit define */	\
+	if r7 != 0 goto l1_%=;				\
+	goto l0_%=;					\
+l1_%=:	r6 = 0;			/* 32-bit define */	\
+	r6 = addr_space_cast(r6, 0x0, 0x1);		\
+l0_%=:	r0 = *(u32 *)(r6 + 0);	/* 64-bit read */	\
+	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm_addr(arena),
+	  __imm_const(__sk_buff_len, offsetof(struct __sk_buff, len))
+	: __clobber_all);
+}
+
 #endif
 
 static __noinline
-- 
2.43.0


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

* Re: [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state
  2026-08-05 18:44 [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state Daniel Borkmann
                   ` (2 preceding siblings ...)
  2026-08-05 18:44 ` [PATCH bpf-next 4/4] selftests/bpf: Add test for arena pointer " Daniel Borkmann
@ 2026-08-05 19:11 ` Eduard Zingerman
  2026-08-05 19:54   ` Daniel Borkmann
  2026-08-05 20:22 ` sashiko-bot
  4 siblings, 1 reply; 9+ messages in thread
From: Eduard Zingerman @ 2026-08-05 19:11 UTC (permalink / raw)
  To: Daniel Borkmann, memxor; +Cc: puranjay, info, bpf

On Wed, 2026-08-05 at 20:44 +0200, Daniel Borkmann wrote:
> A 32-bit write records the writing instruction in reg->subreg_def, and a
> later 64-bit read of that register calls mark_insn_zext() to record that
> the definition has to be zero extended. Architectures whose JIT sets
> bpf_jit_needs_zext() rely on that mark to emit the extension.
> 
> The mark is produced by walking the path from the definition to the read.
> If the walk stops at a state equivalent to an already explored one, the
> reads the explored path performs from there on are not repeated for this
> path's registers, so a definition whose only 64-bit read lies beyond the
> pruning point never gets marked and keeps a garbage upper half.
> 
> Example with BPF_F_TEST_STATE_FREQ making every instruction a checkpoint:
> 
>       r7 = *(u32 *)(r1 + offsetof(struct __sk_buff, len))
>       r6 = 0        /* 64-bit define */
>       if r7 != 0 goto l1
>       goto l0                              path A, explored first
>   l1: w6 = 0        /* 32-bit define */    path B, explored second
>   l0: r0 = r6       /* 64-bit read   */
>       r0 >>= 32
>       exit
> 
> Now, path A is the fall-through of the conditional and is explored first.
> It reaches l0 with r6 defined by the 64-bit r6 = 0, so reg->subreg_def is
> DEF_NOT_SUBREG and the read marks nothing. The walk runs on to exit and
> leaves a checkpoint at every instruction along the way. Path B is explored
> second. w6 = 0 sets r6->subreg_def to that instruction, so a zero extension
> is pending and only the 64-bit read at l0 can resolve it. B then arrives
> at l0, where bpf_is_state_visited() finds the checkpoint A left behind:
> r6 is the scalar 0 in both states, so they are equivalent and B is pruned
> before r0 = r6 is verified.
> 
> Without the fix nothing happens at that point, so the one read that would
> have called mark_insn_zext() for w6 is never walked and the definition
> stays unmarked:
> 
>   l1: w6 = 0        /* subreg_def = w6, pending */
>   l0: r0 = r6     <--- B pruned, read never walked, w6 stays unmarked
> 
> The JIT of an architecture that needs explicit zero extension then emits
> none, and the upper half of w6's definition is left undefined (under
> BPF_F_TEST_RND_HI32 it holds the randomized half, which r0 >>= 32 returns).
> This used to be handled by the registers chain based liveness: the
> propagate_liveness() called mark_insn_zext() for every parent register
> whose read mark was REG_LIVE_READ64, which carried the requirement across
> a pruned state. Commit 107e16979905 ("bpf: disable and remove registers
> chain based liveness") removed that machinery and with it the propagation,
> leaving the mark dependent on the path actually being walked.
> 
> Mark the pending definitions where the walk stops instead, i.e. on the way
> into the prune rather than at the read that is no longer reached:
> 
>   l1: w6 = 0        /* subreg_def = w6, pending */
>   l0: r0 = r6     <--- mark w6, r6 is live here
> 
> The set of registers to mark is the one the pruning decision was made on:
> func_states_equal() compares the registers live at the instruction, per
> insn_aux_data[].live_regs_before, and those are exactly the registers that
> can still be read. A register that is not live there is never read again
> and needs nothing. This is conservative in one direction: a live register
> whose remaining reads are all 32-bit also gets its definition marked,
> which costs a zero extension that is not needed. However, it never marks
> too little, and it does not weaken pruning. On x86-64 the effect is only
> observable with BPF_F_TEST_RND_HI32.
> 
> The one live scalar whose subreg_def can point at a call insn is r0 of a
> kfunc returning a 32-bit value. Marking that one is harmless, the fixup pass
> skips kfunc calls since their zero extension is done by the caller.
> 
> Fixes: 107e16979905 ("bpf: disable and remove registers chain based liveness")
> Reported-by: STAR Labs SG <info@starlabs.sg>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---

Hi Daniel,

I have this series in progress:
https://lore.kernel.org/bpf/20260802-static-zext-v3-0-3456b2604574@gmail.com/
Fixing that exact issue. It needs one more iteration.

...

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

* Re: [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state
  2026-08-05 19:11 ` [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state Eduard Zingerman
@ 2026-08-05 19:54   ` Daniel Borkmann
  2026-08-05 20:36     ` Eduard Zingerman
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Borkmann @ 2026-08-05 19:54 UTC (permalink / raw)
  To: Eduard Zingerman, memxor; +Cc: puranjay, info, bpf

On 8/5/26 9:11 PM, Eduard Zingerman wrote:
> On Wed, 2026-08-05 at 20:44 +0200, Daniel Borkmann wrote:
[...]
> 
> Hi Daniel,
> 
> I have this series in progress:
> https://lore.kernel.org/bpf/20260802-static-zext-v3-0-3456b2604574@gmail.com/
> Fixing that exact issue. It needs one more iteration.
Dang, missed it, integrating into liveness looks better, lets go with
that. If the selftests from here are of any use, please feel free to
pick and integrate into your respin.

Thanks,
Daniel

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

* Re: [PATCH bpf-next 4/4] selftests/bpf: Add test for arena pointer zext across state pruning
  2026-08-05 18:44 ` [PATCH bpf-next 4/4] selftests/bpf: Add test for arena pointer " Daniel Borkmann
@ 2026-08-05 20:14   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-08-05 20:14 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The selftest uses a 64-bit instruction (`r6 = 0`) instead of a 32-bit instruction (`w6 = 0`), which zeroes the upper 32 bits and bypasses `BPF_F_TEST_RND_HI32` randomization, masking the bug it intends to catch.
- [Medium] The test's exit logic masks wild pointer faults via BPF exception tables, causing it to always return 0 (success) even if it successfully reproduces the bug.
--

--- Patch [4]: [PATCH bpf-next 4/4] selftests/bpf: Add test for arena pointer zext across state pruning ---
commit 230d6f79c5ec9b66b6dcd9f9c3cb999533024b62
Author: Daniel Borkmann <daniel@iogearbox.net>

selftests/bpf: Add test for arena pointer zext across state pruning

This patch adds a selftest to verify that the BPF verifier correctly marks
zero extension for arena pointers across state pruning. The test uses an
addr_space_cast and relies on BPF_F_TEST_RND_HI32 to ensure proper zero
extension on architectures like s390x.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_arena.c b/tools/testing/selftests/bpf/progs/verifier_arena.c
> index b241bbcf54a8a..b5f5b81a27e71 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_arena.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_arena.c
[ ... ]
> @@ -635,6 +635,41 @@ int non_arena_ptr_add_to_arena_ptr(void *ctx)
[ ... ]
> +	if r7 != 0 goto l1_%=;				\
> +	goto l0_%=;					\
> +l1_%=:	r6 = 0;			/* 32-bit define */	\

[Severity: Medium]
Does using a 64-bit assignment here bypass the BPF_F_TEST_RND_HI32
randomization framework?

Since r6 = 0 translates to a BPF_ALU64 instruction, bpf_is_reg64() returns
true and the verifier's bpf_opt_subreg_zext_lo32_rnd_hi32() pass skips
instrumenting it. This prevents garbage from being injected into the upper 32
bits, masking the missing zero-extension bug the test intends to catch.

Would using a 32-bit assignment like w6 = 0 correctly trigger the
randomization?

> +	r6 = addr_space_cast(r6, 0x0, 0x1);		\
> +l0_%=:	r0 = *(u32 *)(r6 + 0);	/* 64-bit read */	\
> +	r0 = 0;						\
> +	exit;						\

[Severity: Medium]
Will this exit logic mask wild pointer faults?

Because r6 is a PTR_TO_ARENA, the verifier converts the out-of-bounds read
into a BPF_PROBE_MEM32 load. If r6 contains a wild garbage pointer and triggers
a page fault, the kernel's exception tables will silently catch it, zero r0,
and resume execution.

Since the test then explicitly executes r0 = 0 and exits, and the framework
expects __retval(0), it appears the test will unconditionally pass even if it
successfully reproduces the bug.

> +"	:
> +	: __imm_addr(arena),
> +	  __imm_const(__sk_buff_len, offsetof(struct __sk_buff, len))
> +	: __clobber_all);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805184444.292828-1-daniel@iogearbox.net?part=4

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

* Re: [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state
  2026-08-05 18:44 [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state Daniel Borkmann
                   ` (3 preceding siblings ...)
  2026-08-05 19:11 ` [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state Eduard Zingerman
@ 2026-08-05 20:22 ` sashiko-bot
  4 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-08-05 20:22 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] bpf_mark_live_subregs_zext() fails to mark pending zero-extensions for registers of type PTR_TO_ARENA on pruned paths.
--

--- Patch [1]: [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state ---
commit 59834505da01b398ada03dbde42eaf69952c910a
Author: Daniel Borkmann <daniel@iogearbox.net>

bpf: Mark pending sub-register zero extension before pruning a state

This commit ensures that 32-bit sub-register definitions maintain their
zero-extension marks even on pruned verification paths. It achieves this
by marking live sub-registers for zero extension when the verifier stops
its walk at an equivalent state.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index d925197c2e5f7..addcfd1319c5c 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -3161,6 +3161,44 @@ static void mark_insn_zext(struct bpf_verifier_env *env,
[ ... ]
> +void bpf_mark_live_subregs_zext(struct bpf_verifier_env *env,
> +				struct bpf_verifier_state *vstate)
> +{
> +	struct bpf_insn_aux_data *aux = env->insn_aux_data;
> +	struct bpf_func_state *func;
> +	u16 live_regs;
> +	int i, j;
> +
> +	for (i = vstate->curframe; i >= 0; i--) {
> +		live_regs = aux[bpf_frame_insn_idx(vstate, i)].live_regs_before;
> +		func = vstate->frame[i];
> +		for (j = 0; j < BPF_REG_FP; j++) {
> +			if (!(live_regs & BIT(j)))
> +				continue;
> +			if (func->regs[j].type != SCALAR_VALUE)
> +				continue;

[Severity: Critical]
Does this check inadvertently skip PTR_TO_ARENA registers?

PTR_TO_ARENA is also a 32-bit register type that relies on subreg_def for
zero extension. If a BPF program casts a 64-bit value to a 32-bit
PTR_TO_ARENA, branches, and gets pruned, skipping PTR_TO_ARENA here
would leave its subreg_def unmarked.

Could this lead to missing zero-extensions in the JIT, leaving garbage in
the upper 32 bits and potentially causing out-of-bounds memory accesses
outside the arena boundaries?

> +			mark_insn_zext(env, &func->regs[j]);
> +		}
> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805184444.292828-1-daniel@iogearbox.net?part=1

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

* Re: [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state
  2026-08-05 19:54   ` Daniel Borkmann
@ 2026-08-05 20:36     ` Eduard Zingerman
  0 siblings, 0 replies; 9+ messages in thread
From: Eduard Zingerman @ 2026-08-05 20:36 UTC (permalink / raw)
  To: Daniel Borkmann, memxor; +Cc: puranjay, info, bpf

On Wed, 2026-08-05 at 21:54 +0200, Daniel Borkmann wrote:
> On 8/5/26 9:11 PM, Eduard Zingerman wrote:
> > On Wed, 2026-08-05 at 20:44 +0200, Daniel Borkmann wrote:
> [...]
> > 
> > Hi Daniel,
> > 
> > I have this series in progress:
> > https://lore.kernel.org/bpf/20260802-static-zext-v3-0-3456b2604574@gmail.com/
> > Fixing that exact issue. It needs one more iteration.
> Dang, missed it, integrating into liveness looks better, lets go with
> that. If the selftests from here are of any use, please feel free to
> pick and integrate into your respin.

Ack, will do!

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

end of thread, other threads:[~2026-08-05 20:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05 18:44 [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state Daniel Borkmann
2026-08-05 18:44 ` [PATCH bpf-next 2/4] bpf: Mark pending zero extension of arena ptrs " Daniel Borkmann
2026-08-05 18:44 ` [PATCH bpf-next 3/4] selftests/bpf: Add tests for sub-register zext across state pruning Daniel Borkmann
2026-08-05 18:44 ` [PATCH bpf-next 4/4] selftests/bpf: Add test for arena pointer " Daniel Borkmann
2026-08-05 20:14   ` sashiko-bot
2026-08-05 19:11 ` [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state Eduard Zingerman
2026-08-05 19:54   ` Daniel Borkmann
2026-08-05 20:36     ` Eduard Zingerman
2026-08-05 20:22 ` sashiko-bot

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