* [PATCH bpf-next 0/2] bpf: Preserve stack frame number for commuted stack arithmetic
@ 2026-07-20 16:37 Yiyang Chen
2026-07-20 16:37 ` [PATCH bpf-next 1/2] bpf: Preserve stack frame number for commuted arithmetic Yiyang Chen
2026-07-20 16:37 ` [PATCH bpf-next 2/2] selftests/bpf: Cover stack frame number after scalar plus fp Yiyang Chen
0 siblings, 2 replies; 6+ messages in thread
From: Yiyang Chen @ 2026-07-20 16:37 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi
Cc: Yiyang Chen, John Fastabend, Martin KaFai Lau, Song Liu,
Yonghong Song, Jiri Olsa, Shuah Khan, Emil Tsalapatis, bpf,
linux-kselftest, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf-next 1/2] bpf: Preserve stack frame number for commuted arithmetic
2026-07-20 16:37 [PATCH bpf-next 0/2] bpf: Preserve stack frame number for commuted stack arithmetic Yiyang Chen
@ 2026-07-20 16:37 ` Yiyang Chen
2026-07-20 18:47 ` Eduard Zingerman
2026-07-20 16:37 ` [PATCH bpf-next 2/2] selftests/bpf: Cover stack frame number after scalar plus fp Yiyang Chen
1 sibling, 1 reply; 6+ messages in thread
From: Yiyang Chen @ 2026-07-20 16:37 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi
Cc: Yiyang Chen, John Fastabend, Martin KaFai Lau, Song Liu,
Yonghong Song, Jiri Olsa, Shuah Khan, Emil Tsalapatis, bpf,
linux-kselftest, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf-next 2/2] selftests/bpf: Cover stack frame number after scalar plus fp
2026-07-20 16:37 [PATCH bpf-next 0/2] bpf: Preserve stack frame number for commuted stack arithmetic Yiyang Chen
2026-07-20 16:37 ` [PATCH bpf-next 1/2] bpf: Preserve stack frame number for commuted arithmetic Yiyang Chen
@ 2026-07-20 16:37 ` Yiyang Chen
2026-07-20 17:18 ` Eduard Zingerman
1 sibling, 1 reply; 6+ messages in thread
From: Yiyang Chen @ 2026-07-20 16:37 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi
Cc: Yiyang Chen, John Fastabend, Martin KaFai Lau, Song Liu,
Yonghong Song, Jiri Olsa, Shuah Khan, Emil Tsalapatis, bpf,
linux-kselftest, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] selftests/bpf: Cover stack frame number after scalar plus fp
2026-07-20 16:37 ` [PATCH bpf-next 2/2] selftests/bpf: Cover stack frame number after scalar plus fp Yiyang Chen
@ 2026-07-20 17:18 ` Eduard Zingerman
0 siblings, 0 replies; 6+ messages in thread
From: Eduard Zingerman @ 2026-07-20 17:18 UTC (permalink / raw)
To: Yiyang Chen, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Kumar Kartikeya Dwivedi
Cc: John Fastabend, Martin KaFai Lau, Song Liu, Yonghong Song,
Jiri Olsa, Shuah Khan, Emil Tsalapatis, bpf, linux-kselftest,
linux-kernel
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));
> +}
...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: Preserve stack frame number for commuted arithmetic
2026-07-20 16:37 ` [PATCH bpf-next 1/2] bpf: Preserve stack frame number for commuted arithmetic Yiyang Chen
@ 2026-07-20 18:47 ` Eduard Zingerman
2026-07-21 7:51 ` Yiyang Chen
0 siblings, 1 reply; 6+ messages in thread
From: Eduard Zingerman @ 2026-07-20 18:47 UTC (permalink / raw)
To: Yiyang Chen, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Kumar Kartikeya Dwivedi
Cc: John Fastabend, Martin KaFai Lau, Song Liu, Yonghong Song,
Jiri Olsa, Shuah Khan, Emil Tsalapatis, bpf, linux-kselftest,
linux-kernel
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))
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: Preserve stack frame number for commuted arithmetic
2026-07-20 18:47 ` Eduard Zingerman
@ 2026-07-21 7:51 ` Yiyang Chen
0 siblings, 0 replies; 6+ messages in thread
From: Yiyang Chen @ 2026-07-21 7:51 UTC (permalink / raw)
To: eddyz87
Cc: andrii, ast, bpf, chenyy23, daniel, emil, john.fastabend, jolsa,
linux-kernel, linux-kselftest, martin.lau, memxor, shuah, song,
yonghong.song
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!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-21 7:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-20 16:37 [PATCH bpf-next 0/2] bpf: Preserve stack frame number for commuted stack arithmetic Yiyang Chen
2026-07-20 16:37 ` [PATCH bpf-next 1/2] bpf: Preserve stack frame number for commuted arithmetic Yiyang Chen
2026-07-20 18:47 ` Eduard Zingerman
2026-07-21 7:51 ` Yiyang Chen
2026-07-20 16:37 ` [PATCH bpf-next 2/2] selftests/bpf: Cover stack frame number after scalar plus fp Yiyang Chen
2026-07-20 17:18 ` Eduard Zingerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®