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

[PATCH bpf-next 0/2] bpf: Preserve stack frame number for commuted stack 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 frameno when the source pointer is PTR_TO_STACK.

Preserve the frame number 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.

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                         |  7 +++-
 .../selftests/bpf/prog_tests/verifier.c       |  2 -
 .../bpf/prog_tests/verifier_basic_stack.c     | 28 +++++++++++++
 .../bpf/progs/verifier_basic_stack.c          | 41 +++++++++++++++++++
 4 files changed, 74 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/verifier_basic_stack.c


base-commit: cfce77b63375dac81d53f2f85593c548415206b7
-- 
2.34.1

[PATCH bpf-next 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 inherits the pointer type and id from the source
pointer. For PTR_TO_STACK, the inherited pointer state also has to carry
the stack frame number.

Without the frame number copy, a stack pointer derived inside a callee
through scalar += fp can be recorded as pointing to frame 0. Stack reads
and writes through that register can then update or consult the caller
frame while the actual instruction uses the callee frame.

Copy the frame number when PTR_TO_STACK state is inherited by the
commuted arithmetic form.

Signed-off-by: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
---
 kernel/bpf/verifier.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 52be0a118cce0..58017141d52b7 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13796,11 +13796,14 @@ 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.
+	/* In case of 'scalar += pointer', dst_reg inherits pointer type, id,
+	 * and for stack pointers also the frame number. The id may be overwritten
+	 * later if we create a new variable offset.
 	 */
 	dst_reg->type = ptr_reg->type;
 	dst_reg->id = ptr_reg->id;
+	if (base_type(ptr_reg->type) == PTR_TO_STACK)
+		dst_reg->frameno = ptr_reg->frameno;
 
 	if (!check_reg_sane_offset_scalar(env, off_reg, ptr_reg->type) ||
 	    !check_reg_sane_offset_ptr(env, ptr_reg, ptr_reg->type))
-- 
2.34.1

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing inheritance of `parent_id` in commuted pointer arithmetic (scalar += pointer) allows derived pointers like dynptr slices to escape reference invalidation, leading to a Use-After-Free.
- [High] An early return for untrusted pointers during commuted arithmetic leaves the verifier state desynchronized from the runtime state, allowing arbitrary kernel memory read/write.
--

--- Patch [1]: [PATCH bpf-next 1/2] bpf: Preserve stack frame number for commuted arithmetic ---
commit e1439595e89ba75141d297320a4a5530c2e2c8ce
Author: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>

bpf: Preserve stack frame number for commuted arithmetic

When scalar += pointer is handled in adjust_ptr_min_max_vals(), the
destination register inherits the pointer type and id from the source
pointer. For PTR_TO_STACK, the inherited pointer state also has to carry
the stack frame number.

Without the frame number copy, a stack pointer derived inside a callee
through scalar += fp can be recorded as pointing to frame 0. Stack reads
and writes through that register can then update or consult the caller
frame while the actual instruction uses the callee frame.

Copy the frame number when PTR_TO_STACK state is inherited by the
commuted arithmetic form.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 52be0a118cce0..58017141d52b7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -13796,11 +13796,14 @@ 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.
> +	/* In case of 'scalar += pointer', dst_reg inherits pointer type, id,
> +	 * and for stack pointers also the frame number. The id may be overwritten
> +	 * later if we create a new variable offset.
>  	 */
>  	dst_reg->type = ptr_reg->type;
>  	dst_reg->id = ptr_reg->id;
> +	if (base_type(ptr_reg->type) == PTR_TO_STACK)
> +		dst_reg->frameno = ptr_reg->frameno;

[Severity: High]
This isn't a bug introduced by this patch, but should we also copy parent_id
here?

During commuted arithmetic (scalar += pointer), the verifier transforms
the scalar destination register into a pointer but fails to copy the
parent_id from the source pointer, leaving it as 0.

If a BPF program obtains a dynptr slice (PTR_TO_MEM with parent_id tied
to the dynptr), performs commuted arithmetic (scalar += slice_ptr), and
then releases the dynptr (e.g., bpf_ringbuf_discard_dynptr), will
release_reference() skip invalidating the commuted pointer?

release_reference() looks for matching parent_id:

kernel/bpf/verifier.c:release_reference() {
    ...
		bpf_for_each_reg_in_vstate_mask(vstate, state, reg, stack, mask, ({
			if (reg->id != id && reg->parent_id != id)
				continue;
    ...
}

Because the commuted pointer has a parent_id of 0, could it escape
invalidation and be dereferenced later to read or write freed memory?

[Severity: High]
This is a pre-existing issue, but does the early return for untrusted
pointers in adjust_ptr_min_max_vals() handle scalar += untrusted_ptr
correctly?

Earlier in adjust_ptr_min_max_vals(), we have:

kernel/bpf/verifier.c:adjust_ptr_min_max_vals() {
    ...
	/*
	 * Accesses to untrusted PTR_TO_MEM are done through probe
	 * instructions, hence no need to track offsets.
	 */
	if (base_type(ptr_reg->type) == PTR_TO_MEM && (ptr_reg->type & PTR_UNTRUSTED))
		return 0;
    ...
}

If a BPF program executes scalar += untrusted_ptr, check_alu_op() invokes
adjust_ptr_min_max_vals() with the scalar as dst_reg and the untrusted
pointer as src_reg.

If the early return succeeds, it returns 0 without modifying the
destination register's type. Does this leave the verifier tracking dst_reg
as a scalar, while at runtime it becomes a pointer?

Could a program then use this register as a forged index into an array or
map, bypassing bounds checks?

>  
>  	if (!check_reg_sane_offset_scalar(env, off_reg, ptr_reg->type) ||
>  	    !check_reg_sane_offset_ptr(env, ptr_reg, ptr_reg->type))

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784563939.git.chenyy23@mails.tsinghua.edu.cn?part=1

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

[Eduard Zingerman]

On Mon, 2026-07-20 at 16:37 +0000, Yiyang Chen wrote:
> When scalar += pointer is handled in adjust_ptr_min_max_vals(), the
> destination register inherits the pointer type and id from the source
> pointer. For PTR_TO_STACK, the inherited pointer state also has to carry
> the stack frame number.
> 
> Without the frame number copy, a stack pointer derived inside a callee
> through scalar += fp can be recorded as pointing to frame 0. Stack reads
> and writes through that register can then update or consult the caller
> frame while the actual instruction uses the callee frame.
> 
> Copy the frame number when PTR_TO_STACK state is inherited by the
> commuted arithmetic form.

Could you please infer a "Fixes" tag?

> Signed-off-by: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
> ---
>  kernel/bpf/verifier.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 52be0a118cce0..58017141d52b7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -13796,11 +13796,14 @@ 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.
> +	/* In case of 'scalar += pointer', dst_reg inherits pointer type, id,
> +	 * and for stack pointers also the frame number. The id may be overwritten
> +	 * later if we create a new variable offset.
>  	 */
>  	dst_reg->type = ptr_reg->type;
>  	dst_reg->id = ptr_reg->id;
> +	if (base_type(ptr_reg->type) == PTR_TO_STACK)
> +		dst_reg->frameno = ptr_reg->frameno;

This patch fixes a real issue and this a surprisingly buggy piece of
code. Looking at other fields defined in bpf_reg_state, it appears
that several additional modifications are necessary:
- dst_reg->delta = 0
- dst_reg->parent_id = ptr_reg->parent_id

Given such an error prone nature, I think it would be better to:
- stash the value of the off_reg in a temporary variable and adjust
  off_reg pointer accordingly (see bpf_verifier_env for a collection
  of similar temporaries).
- do *dst_reg = *ptr_reg instead of the fixup.

Wdyt?

>  
>  	if (!check_reg_sane_offset_scalar(env, off_reg, ptr_reg->type) ||
>  	    !check_reg_sane_offset_ptr(env, ptr_reg, ptr_reg->type))

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

[Yiyang Chen]

Hi Eduard,

Totally agree.

I'll respin v2 to preserve the full pointer state instead of copying only selected fields, and use a temporary scalar offset register for the commuted `scalar += pointer` case as you suggested.

I also re-checked the history before adding the `Fixes:` tag. The tightest candidate I found is:

`Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")`

That commit introduced the unified `scalar += pointer` handling and the pointer-state inheritance path. `f4d7e40a5b15` added the multi-frame stack model later, so it is related but less precise as a `Fixes:` target.

Will send v2 soon.

Thanks!

[PATCH bpf-next 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     | 28 +++++++++++++
 .../bpf/progs/verifier_basic_stack.c          | 41 +++++++++++++++++++
 3 files changed, 69 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..279f389729b8e
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/verifier_basic_stack.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <test_progs.h>
+
+#include "cap_helpers.h"
+#include "verifier_basic_stack.skel.h"
+
+void test_verifier_basic_stack(void)
+{
+	struct test_loader tester = {};
+	__u64 old_caps;
+	int err;
+
+	/* test_verifier tests are executed w/o CAP_SYS_ADMIN, do the same here */
+	err = cap_disable_effective(1ULL << CAP_SYS_ADMIN, &old_caps);
+	if (err) {
+		PRINT_FAIL("failed to drop CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
+		return;
+	}
+
+	test_loader__run_subtests(&tester, "verifier_basic_stack",
+				  verifier_basic_stack__elf_bytes);
+	test_loader_fini(&tester);
+
+	err = cap_enable_effective(old_caps, NULL);
+	if (err)
+		PRINT_FAIL("failed to restore CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
+}
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 bpf-next 2/2] selftests/bpf: Cover stack frame number after scalar plus fp

[Eduard Zingerman]

On Mon, 2026-07-20 at 16:37 +0000, Yiyang Chen wrote:

...

> +++ b/tools/testing/selftests/bpf/prog_tests/verifier_basic_stack.c
> @@ -0,0 +1,28 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <test_progs.h>
> +
> +#include "cap_helpers.h"
> +#include "verifier_basic_stack.skel.h"
> +
> +void test_verifier_basic_stack(void)
> +{
> +	struct test_loader tester = {};
> +	__u64 old_caps;
> +	int err;
> +
> +	/* test_verifier tests are executed w/o CAP_SYS_ADMIN, do the same here */
> +	err = cap_disable_effective(1ULL << CAP_SYS_ADMIN, &old_caps);
> +	if (err) {
> +		PRINT_FAIL("failed to drop CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));

Why is this necessary? It should be possible to prepare a test case
demonstrating the bug w/o this change.

> +		return;
> +	}
> +
> +	test_loader__run_subtests(&tester, "verifier_basic_stack",
> +				  verifier_basic_stack__elf_bytes);
> +	test_loader_fini(&tester);
> +
> +	err = cap_enable_effective(old_caps, NULL);
> +	if (err)
> +		PRINT_FAIL("failed to restore CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
> +}

...