[bpf-next v2 0/2] bpf: Preserve stack frame number for commuted arithmetic

[bpf-next v2 0/2] bpf: Preserve stack frame number for commuted arithmetic

[Yiyang Chen]

The verifier uses reg->frameno to distinguish stack pointers that have
the same offset but refer to different call frames. The scalar += pointer
path copies pointer type and id to the destination register, but it also
needs to preserve pointer provenance fields such as the stack frame
number.

Preserve the full pointer register state for commuted stack pointer
arithmetic and add a verifier regression test where a callee derives its
frame pointer through scalar += fp before overwriting and reloading the
same stack slot.

Changes in v2:
- Preserve the full pointer register state instead of copying only
  selected fields, as suggested by Eduard.
- Use a temporary scalar offset register for the commuted scalar += fp
  case, as suggested by Eduard.
- Add a Fixes tag to the verifier change.
- Drop CAP_SYS_ADMIN handling from the new verifier_basic_stack harness,
  as requested by Eduard.

Yiyang Chen (2):
  bpf: Preserve stack frame number for commuted arithmetic
  selftests/bpf: Cover stack frame number after scalar plus fp

 kernel/bpf/verifier.c                         | 14 +++++++-----
 .../selftests/bpf/prog_tests/verifier.c       |  2 -
 .../bpf/prog_tests/verifier_basic_stack.c     | 14 ++++++++
 .../bpf/progs/verifier_basic_stack.c          | 41 +++++++++++++++++++
 4 files changed, 64 insertions(+), 10 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/verifier_basic_stack.c


base-commit: 0bcca2a42cc50b7d64a95c08dffc6b93661a7ea2
--
2.34.1

[PATCH 1/2] bpf: Preserve stack frame number for commuted arithmetic

[Yiyang Chen]

When scalar += pointer is handled in adjust_ptr_min_max_vals(), the
destination register has to inherit the pointer register state from the
source pointer. Copying only selected fields is fragile because pointer
provenance is tracked by several bpf_reg_state fields.

For the commuted form, pass a temporary scalar offset register to the
common pointer arithmetic helper and copy the full pointer state before
applying pointer arithmetic. This preserves the frame number for
PTR_TO_STACK registers and keeps parent identity fields consistent.

Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
Signed-off-by: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
---
 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 52be0a118cce0..3c0af83db4672 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13796,11 +13796,12 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
 		return -EACCES;
 	}
 
-	/* In case of 'scalar += pointer', dst_reg inherits pointer type and id.
-	 * The id may be overwritten later if we create a new variable offset.
+	/* For 'scalar += pointer', dst_reg inherits the complete pointer
+	 * register state. Individual fields may be adjusted later by pointer
+	 * arithmetic.
 	 */
-	dst_reg->type = ptr_reg->type;
-	dst_reg->id = ptr_reg->id;
+	if (ptr_reg != dst_reg)
+		*dst_reg = *ptr_reg;
 
 	if (!check_reg_sane_offset_scalar(env, off_reg, ptr_reg->type) ||
 	    !check_reg_sane_offset_ptr(env, ptr_reg, ptr_reg->type))
@@ -14854,15 +14855,18 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env,
 					bpf_alu_string[opcode >> 4]);
 				return -EACCES;
 			} else {
+				struct bpf_reg_state off_reg;
+
 				/* scalar += pointer
 				 * This is legal, but we have to reverse our
 				 * src/dest handling in computing the range
 				 */
+				off_reg = *dst_reg;
 				err = mark_chain_precision(env, insn->dst_reg);
 				if (err)
 					return err;
 				return adjust_ptr_min_max_vals(env, insn,
-							       src_reg, dst_reg);
+							       src_reg, &off_reg);
 			}
 		} else if (ptr_reg) {
 			/* pointer += scalar */

base-commit: 0bcca2a42cc50b7d64a95c08dffc6b93661a7ea2
-- 
2.34.1

[PATCH 2/2] selftests/bpf: Cover stack frame number after scalar plus fp

[Yiyang Chen]

Add a verifier test where a callee spills a map value pointer. It then
derives its frame pointer through scalar += fp. It overwrites the same
stack slot through the derived pointer.

The final reload must be treated as a scalar after the overwrite. The
verifier should reject the program when the reloaded value is dereferenced.

Signed-off-by: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
---
 .../selftests/bpf/prog_tests/verifier.c       |  2 -
 .../bpf/prog_tests/verifier_basic_stack.c     | 14 +++++++
 .../bpf/progs/verifier_basic_stack.c          | 41 +++++++++++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/verifier_basic_stack.c

diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index be97f6887f0e7..0483ae340726c 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -11,7 +11,6 @@
 #include "verifier_arena_globals2.skel.h"
 #include "verifier_array_access.skel.h"
 #include "verifier_async_cb_context.skel.h"
-#include "verifier_basic_stack.skel.h"
 #include "verifier_bitfield_write.skel.h"
 #include "verifier_bounds.skel.h"
 #include "verifier_bounds_deduction.skel.h"
@@ -165,7 +164,6 @@ void test_verifier_arena(void)                { RUN(verifier_arena); }
 void test_verifier_arena_large(void)          { RUN(verifier_arena_large); }
 void test_verifier_arena_globals1(void)       { RUN(verifier_arena_globals1); }
 void test_verifier_arena_globals2(void)       { RUN(verifier_arena_globals2); }
-void test_verifier_basic_stack(void)          { RUN(verifier_basic_stack); }
 void test_verifier_bitfield_write(void)       { RUN(verifier_bitfield_write); }
 void test_verifier_bounds(void)               { RUN(verifier_bounds); }
 void test_verifier_bounds_deduction(void)     { RUN(verifier_bounds_deduction); }
diff --git a/tools/testing/selftests/bpf/prog_tests/verifier_basic_stack.c b/tools/testing/selftests/bpf/prog_tests/verifier_basic_stack.c
new file mode 100644
index 0000000000000..ff9382b47920c
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/verifier_basic_stack.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <test_progs.h>
+
+#include "verifier_basic_stack.skel.h"
+
+void test_verifier_basic_stack(void)
+{
+	struct test_loader tester = {};
+
+	test_loader__run_subtests(&tester, "verifier_basic_stack",
+				  verifier_basic_stack__elf_bytes);
+	test_loader_fini(&tester);
+}
diff --git a/tools/testing/selftests/bpf/progs/verifier_basic_stack.c b/tools/testing/selftests/bpf/progs/verifier_basic_stack.c
index fb62e09f21146..634183fc86883 100644
--- a/tools/testing/selftests/bpf/progs/verifier_basic_stack.c
+++ b/tools/testing/selftests/bpf/progs/verifier_basic_stack.c
@@ -97,4 +97,45 @@ __naked void misaligned_read_from_stack(void)
 "	::: __clobber_all);
 }
 
+SEC("socket")
+__description("stack pointer arithmetic preserves frame number")
+__failure __msg("R7 invalid mem access 'scalar'")
+__naked void stack_ptr_arith_preserves_frameno(void)
+{
+	asm volatile ("					\
+	r3 = 0;						\
+	*(u64 *)(r10 - 8) = r3;			\
+	r1 = %[map_hash_8b] ll;			\
+	r2 = r10;					\
+	r2 += -8;					\
+	call %[bpf_map_lookup_elem];			\
+	if r0 != 0 goto +2;				\
+	r0 = 0;						\
+	exit;						\
+	r1 = r0;					\
+	r2 = 0;						\
+	r3 = 0;						\
+	call stack_ptr_arith_preserves_frameno_subprog;	\
+	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_map_lookup_elem),
+	  __imm_addr(map_hash_8b)
+	: __clobber_all);
+}
+
+static __used __naked void stack_ptr_arith_preserves_frameno_subprog(void)
+{
+	asm volatile ("					\
+	*(u64 *)(r10 - 8) = r1;			\
+	r6 = -8;					\
+	r6 += r10;					\
+	*(u64 *)(r6 + 0) = r2;			\
+	r7 = *(u64 *)(r10 - 8);			\
+	*(u64 *)(r7 + 0) = r3;			\
+	r0 = 0;						\
+	exit;						\
+"	::: __clobber_all);
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.34.1

Re: [PATCH 2/2] selftests/bpf: Cover stack frame number after scalar plus fp

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> index be97f6887f0e..0483ae340726 100644
> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> @@ -11,7 +11,6 @@
>  #include "verifier_arena_globals2.skel.h"
>  #include "verifier_array_access.skel.h"
>  #include "verifier_async_cb_context.skel.h"
> -#include "verifier_basic_stack.skel.h"
>  #include "verifier_bitfield_write.skel.h"
>  #include "verifier_bounds.skel.h"
>  #include "verifier_bounds_deduction.skel.h"
> @@ -165,7 +164,6 @@ void test_verifier_arena(void)                { RUN(verifier_arena); }
>  void test_verifier_arena_large(void)          { RUN(verifier_arena_large); }
>  void test_verifier_arena_globals1(void)       { RUN(verifier_arena_globals1); }
>  void test_verifier_arena_globals2(void)       { RUN(verifier_arena_globals2); }
> -void test_verifier_basic_stack(void)          { RUN(verifier_basic_stack); }
>  void test_verifier_bitfield_write(void)       { RUN(verifier_bitfield_write); }
>  void test_verifier_bounds(void)               { RUN(verifier_bounds); }
>  void test_verifier_bounds_deduction(void)     { RUN(verifier_bounds_deduction); }

The verifier_basic_stack skeleton was already dispatched here through
RUN(verifier_basic_stack).  Adding the new subtest to
progs/verifier_basic_stack.c on its own would have exercised it under the
existing harness with no change to prog_tests.

This isn't a bug, but is the move to a standalone harness needed?  The new
subtest could be added to progs/verifier_basic_stack.c while leaving this
RUN(verifier_basic_stack) line in place.

There is also a behavioral difference worth noting.  run_tests_aux() in
verifier.c drops CAP_SYS_ADMIN for the whole run:

    /* test_verifier tests are executed w/o CAP_SYS_ADMIN, do the same here */
    err = cap_disable_effective(1ULL << CAP_SYS_ADMIN, &old_caps);

> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier_basic_stack.c b/tools/testing/selftests/bpf/prog_tests/verifier_basic_stack.c
> new file mode 100644
> index 000000000000..ff9382b47920
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/verifier_basic_stack.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <test_progs.h>
> +
> +#include "verifier_basic_stack.skel.h"
> +
> +void test_verifier_basic_stack(void)
> +{
> +	struct test_loader tester = {};
> +
> +	test_loader__run_subtests(&tester, "verifier_basic_stack",
> +				  verifier_basic_stack__elf_bytes);
> +	test_loader_fini(&tester);
> +}

The new harness does no capability manipulation, so the six pre-existing
basic_stack subtests now run their privileged pass with CAP_SYS_ADMIN
retained, where before they ran without it.

The verifier verdicts should be unchanged, since env->bpf_capable keys on
CAP_BPF and allow_ptr_leaks / allow_uninit_stack / bypass_spec key on
CAP_PERFMON, all of which the old harness kept and CAP_SYS_ADMIN implies.

Was the different privileged environment for the existing subtests
intentional, and is it worth a note in the changelog?

> diff --git a/tools/testing/selftests/bpf/progs/verifier_basic_stack.c b/tools/testing/selftests/bpf/progs/verifier_basic_stack.c
> index fb62e09f2114..634183fc8688 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_basic_stack.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_basic_stack.c
> @@ -97,4 +97,45 @@ __naked void misaligned_read_from_stack(void)
>  "	::: __clobber_all);
>  }
>
> +SEC("socket")
> +__description("stack pointer arithmetic preserves frame number")
> +__failure __msg("R7 invalid mem access 'scalar'")
> +__naked void stack_ptr_arith_preserves_frameno(void)
> +{
> +	asm volatile ("					\
> +	r3 = 0;						\
> +	*(u64 *)(r10 - 8) = r3;			\
> +	r1 = %[map_hash_8b] ll;			\
> +	r2 = r10;					\
> +	r2 += -8;					\
> +	call %[bpf_map_lookup_elem];			\
> +	if r0 != 0 goto +2;				\
> +	r0 = 0;						\
> +	exit;						\
> +	r1 = r0;					\
> +	r2 = 0;						\
> +	r3 = 0;						\
> +	call stack_ptr_arith_preserves_frameno_subprog;	\
> +	r0 = 0;						\
> +	exit;						\
> +"	:
> +	: __imm(bpf_map_lookup_elem),
> +	  __imm_addr(map_hash_8b)
> +	: __clobber_all);
> +}

The BPF program and expected message ("R7 invalid mem access 'scalar'")
match the reject path in do_check for a scalar-typed pointer dereference, so
the test itself looks correct.


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

Re: [PATCH 2/2] selftests/bpf: Cover stack frame number after scalar plus fp

[Eduard Zingerman]

On Tue, 2026-07-21 at 10:29 +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> > index be97f6887f0e..0483ae340726 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> > @@ -11,7 +11,6 @@
> >  #include "verifier_arena_globals2.skel.h"
> >  #include "verifier_array_access.skel.h"
> >  #include "verifier_async_cb_context.skel.h"
> > -#include "verifier_basic_stack.skel.h"
> >  #include "verifier_bitfield_write.skel.h"
> >  #include "verifier_bounds.skel.h"
> >  #include "verifier_bounds_deduction.skel.h"
> > @@ -165,7 +164,6 @@ void test_verifier_arena(void)                { RUN(verifier_arena); }
> >  void test_verifier_arena_large(void)          { RUN(verifier_arena_large); }
> >  void test_verifier_arena_globals1(void)       { RUN(verifier_arena_globals1); }
> >  void test_verifier_arena_globals2(void)       { RUN(verifier_arena_globals2); }
> > -void test_verifier_basic_stack(void)          { RUN(verifier_basic_stack); }
> >  void test_verifier_bitfield_write(void)       { RUN(verifier_bitfield_write); }
> >  void test_verifier_bounds(void)               { RUN(verifier_bounds); }
> >  void test_verifier_bounds_deduction(void)     { RUN(verifier_bounds_deduction); }
> 
> The verifier_basic_stack skeleton was already dispatched here through
> RUN(verifier_basic_stack).  Adding the new subtest to
> progs/verifier_basic_stack.c on its own would have exercised it under the
> existing harness with no change to prog_tests.

Bot is spot on here.

pw-bot: cr.

Re: [PATCH 1/2] bpf: Preserve stack frame number for commuted arithmetic

[Eduard Zingerman]

On Tue, 2026-07-21 at 09:39 +0000, Yiyang Chen wrote:
> When scalar += pointer is handled in adjust_ptr_min_max_vals(), the
> destination register has to inherit the pointer register state from the
> source pointer. Copying only selected fields is fragile because pointer
> provenance is tracked by several bpf_reg_state fields.
> 
> For the commuted form, pass a temporary scalar offset register to the
> common pointer arithmetic helper and copy the full pointer state before
> applying pointer arithmetic. This preserves the frame number for
> PTR_TO_STACK registers and keeps parent identity fields consistent.
> 
> Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
> Signed-off-by: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
> ---
>  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 52be0a118cce0..3c0af83db4672 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -13796,11 +13796,12 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
>  		return -EACCES;
>  	}
>  
> -	/* In case of 'scalar += pointer', dst_reg inherits pointer type and id.
> -	 * The id may be overwritten later if we create a new variable offset.
> +	/* For 'scalar += pointer', dst_reg inherits the complete pointer
> +	 * register state. Individual fields may be adjusted later by pointer
> +	 * arithmetic.
>  	 */
> -	dst_reg->type = ptr_reg->type;
> -	dst_reg->id = ptr_reg->id;
> +	if (ptr_reg != dst_reg)
> +		*dst_reg = *ptr_reg;


Lot's of tests are failing on the CI, please investigate.

>  
>  	if (!check_reg_sane_offset_scalar(env, off_reg, ptr_reg->type) ||
>  	    !check_reg_sane_offset_ptr(env, ptr_reg, ptr_reg->type))
> @@ -14854,15 +14855,18 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env,
>  					bpf_alu_string[opcode >> 4]);
>  				return -EACCES;
>  			} else {
> +				struct bpf_reg_state off_reg;
> +

We store such temporaries in the bpf_verifier_env to avoid excessive
stack consumption.

>  				/* scalar += pointer
>  				 * This is legal, but we have to reverse our
>  				 * src/dest handling in computing the range
>  				 */
> +				off_reg = *dst_reg;
>  				err = mark_chain_precision(env, insn->dst_reg);
>  				if (err)
>  					return err;
>  				return adjust_ptr_min_max_vals(env, insn,
> -							       src_reg, dst_reg);
> +							       src_reg, &off_reg);
>  			}
>  		} else if (ptr_reg) {
>  			/* pointer += scalar */
> 
> base-commit: 0bcca2a42cc50b7d64a95c08dffc6b93661a7ea2

Re: [PATCH 1/2] bpf: Preserve stack frame number for commuted arithmetic

[Shung-Hsi Yu]

On Tue, Jul 21, 2026 at 05:12:14PM -0700, Eduard Zingerman wrote:
> On Tue, 2026-07-21 at 09:39 +0000, Yiyang Chen wrote:
> > When scalar += pointer is handled in adjust_ptr_min_max_vals(), the
> > destination register has to inherit the pointer register state from the
> > source pointer. Copying only selected fields is fragile because pointer
> > provenance is tracked by several bpf_reg_state fields.
> > 
> > For the commuted form, pass a temporary scalar offset register to the
> > common pointer arithmetic helper and copy the full pointer state before
> > applying pointer arithmetic. This preserves the frame number for
> > PTR_TO_STACK registers and keeps parent identity fields consistent.
> > 
> > Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")

This doesn't look right. Without bpf2bpf there is just one frame, hence
there isn't a bug at this point. I think the commit f4d7e40a5b71 ("bpf:
introduce function calls (verification)") the you mentioned in v1 should
be used here instead.

Ideally there should be a second tag for the missed parent_id
preservation issue for invalidating dynptr mentioned by Sashiko, too.
But it wasn't that obvious where that issue was introduced to me, and
f4d7e40a5b71 pre-dates dynptr anyway.

> > Signed-off-by: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
> > ---
> >  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 52be0a118cce0..3c0af83db4672 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -13796,11 +13796,12 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
> >  		return -EACCES;
> >  	}
> >  
> > -	/* In case of 'scalar += pointer', dst_reg inherits pointer type and id.
> > -	 * The id may be overwritten later if we create a new variable offset.
> > +	/* For 'scalar += pointer', dst_reg inherits the complete pointer
> > +	 * register state. Individual fields may be adjusted later by pointer
> > +	 * arithmetic.
> >  	 */
> > -	dst_reg->type = ptr_reg->type;
> > -	dst_reg->id = ptr_reg->id;
> > +	if (ptr_reg != dst_reg)
> > +		*dst_reg = *ptr_reg;
> 
> Lot's of tests are failing on the CI, please investigate.

I think one reason is that this overwrites r64 and var_off, hence in the
scalar += pointer case, we're adding the existing offset in ptr_reg to
itself, equivalent to:

    dst_reg->r64 = cnum64_add(ptr_reg->r64, ptr_reg->r64);
    dst_reg->var_off = tnum_add(ptr_reg->var_off, ptr_reg->var_off);

[...]