* [bpf-next v2 0/2] bpf: Preserve stack frame number for commuted arithmetic @ 2026-07-21 9:39 Yiyang Chen 2026-07-21 9:39 ` [PATCH 1/2] " Yiyang Chen 2026-07-21 9:39 ` [PATCH 2/2] selftests/bpf: Cover stack frame number after scalar plus fp Yiyang Chen 0 siblings, 2 replies; 9+ messages in thread From: Yiyang Chen @ 2026-07-21 9:39 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 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] bpf: Preserve stack frame number for commuted arithmetic 2026-07-21 9:39 [bpf-next v2 0/2] bpf: Preserve stack frame number for commuted arithmetic Yiyang Chen @ 2026-07-21 9:39 ` Yiyang Chen 2026-07-21 9:48 ` sashiko-bot 2026-07-22 0:12 ` Eduard Zingerman 2026-07-21 9:39 ` [PATCH 2/2] selftests/bpf: Cover stack frame number after scalar plus fp Yiyang Chen 1 sibling, 2 replies; 9+ messages in thread From: Yiyang Chen @ 2026-07-21 9:39 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi Cc: John Fastabend, Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa, Shuah Khan, Emil Tsalapatis, bpf, linux-kselftest, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] bpf: Preserve stack frame number for commuted arithmetic 2026-07-21 9:39 ` [PATCH 1/2] " Yiyang Chen @ 2026-07-21 9:48 ` sashiko-bot 2026-07-22 7:44 ` Shung-Hsi Yu 2026-07-22 0:12 ` Eduard Zingerman 1 sibling, 1 reply; 9+ messages in thread From: sashiko-bot @ 2026-07-21 9:48 UTC (permalink / raw) To: Yiyang Chen; +Cc: bpf Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] The patch completely breaks the security check that prohibits subtracting a pointer from a scalar (`scalar -= pointer`), allowing unprivileged eBPF programs to bypass memory isolation and achieve arbitrary kernel memory read/write. -- --- Patch [1]: [PATCH 1/2] bpf: Preserve stack frame number for commuted arithmetic --- commit 3b8c2832d6eb05fc6ef4d1f2af9ee96ccdf5566e Author: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn> bpf: Preserve stack frame number for commuted arithmetic This patch modifies how commuted pointer arithmetic is handled in adjust_reg_min_max_vals(). It uses a temporary off_reg to copy the destination register before calling adjust_ptr_min_max_vals(). > 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 [ ... ] > @@ -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); [Severity: Critical] Does passing a pointer to the local stack variable off_reg here break the pointer equality check in adjust_ptr_min_max_vals()? In adjust_ptr_min_max_vals(), we have this check to prevent subtracting a pointer from a scalar: case BPF_SUB: if (dst_reg == off_reg) { /* scalar -= pointer. Creates an unknown scalar */ verbose(env, "R%d tried to subtract pointer from scalar\n", dst); return -EACCES; } Since off_reg is now a stack pointer from adjust_reg_min_max_vals(), it will never equal the verifier state array pointer dst_reg. Could this regression allow eBPF programs to bypass the restriction and execute scalar -= pointer, potentially breaking memory isolation? -- Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784563950.git.chenyy23@mails.tsinghua.edu.cn?part=1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] bpf: Preserve stack frame number for commuted arithmetic 2026-07-21 9:48 ` sashiko-bot @ 2026-07-22 7:44 ` Shung-Hsi Yu 0 siblings, 0 replies; 9+ messages in thread From: Shung-Hsi Yu @ 2026-07-22 7:44 UTC (permalink / raw) To: Yiyang Chen, sashiko-reviews; +Cc: bpf On Tue, Jul 21, 2026 at 09:48:28AM +0000, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Critical] The patch completely breaks the security check that prohibits subtracting a pointer from a scalar (`scalar -= pointer`), allowing unprivileged eBPF programs to bypass memory isolation and achieve arbitrary kernel memory read/write. > -- > > --- Patch [1]: [PATCH 1/2] bpf: Preserve stack frame number for commuted arithmetic --- > commit 3b8c2832d6eb05fc6ef4d1f2af9ee96ccdf5566e > Author: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn> > > bpf: Preserve stack frame number for commuted arithmetic > > This patch modifies how commuted pointer arithmetic is handled in > adjust_reg_min_max_vals(). It uses a temporary off_reg to copy the > destination register before calling adjust_ptr_min_max_vals(). > > > 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 > > [ ... ] > > > @@ -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); > > [Severity: Critical] > Does passing a pointer to the local stack variable off_reg here break the > pointer equality check in adjust_ptr_min_max_vals()? > > In adjust_ptr_min_max_vals(), we have this check to prevent subtracting a > pointer from a scalar: > > case BPF_SUB: > if (dst_reg == off_reg) { > /* scalar -= pointer. Creates an unknown scalar */ > verbose(env, "R%d tried to subtract pointer from scalar\n", > dst); > return -EACCES; > } > > Since off_reg is now a stack pointer from adjust_reg_min_max_vals(), it will > never equal the verifier state array pointer dst_reg. Could this regression > allow eBPF programs to bypass the restriction and execute scalar -= pointer, > potentially breaking memory isolation? This seems right. adjust_ptr_min_max_vals() has the assumptions that either one of these cases has to be true 1. off_reg == dst_reg && ptr_reg == src_reg 2. ptr_reg == dst_reg && off_reg == src_reg By passing neither dst_reg and src_reg as off_reg we're breaking the assumption. Perhaps we should have add a ptr_is_dst_reg argument for adjust_ptr_min_max_vals(), which could also then passed down to sanitize_ptr_alu() as well (which currently computes ptr_is_dst_reg itself). ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] bpf: Preserve stack frame number for commuted arithmetic 2026-07-21 9:39 ` [PATCH 1/2] " Yiyang Chen 2026-07-21 9:48 ` sashiko-bot @ 2026-07-22 0:12 ` Eduard Zingerman 2026-07-22 8:46 ` Shung-Hsi Yu 1 sibling, 1 reply; 9+ messages in thread From: Eduard Zingerman @ 2026-07-22 0:12 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 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] bpf: Preserve stack frame number for commuted arithmetic 2026-07-22 0:12 ` Eduard Zingerman @ 2026-07-22 8:46 ` Shung-Hsi Yu 0 siblings, 0 replies; 9+ messages in thread From: Shung-Hsi Yu @ 2026-07-22 8:46 UTC (permalink / raw) To: Yiyang Chen Cc: Eduard Zingerman, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Kumar Kartikeya Dwivedi, John Fastabend, Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa, Shuah Khan, Emil Tsalapatis, bpf, linux-kselftest, linux-kernel 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); [...] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] selftests/bpf: Cover stack frame number after scalar plus fp 2026-07-21 9:39 [bpf-next v2 0/2] bpf: Preserve stack frame number for commuted arithmetic Yiyang Chen 2026-07-21 9:39 ` [PATCH 1/2] " Yiyang Chen @ 2026-07-21 9:39 ` Yiyang Chen 2026-07-21 10:29 ` bot+bpf-ci 1 sibling, 1 reply; 9+ messages in thread From: Yiyang Chen @ 2026-07-21 9:39 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi Cc: John Fastabend, Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa, Shuah Khan, Emil Tsalapatis, bpf, linux-kselftest, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] selftests/bpf: Cover stack frame number after scalar plus fp 2026-07-21 9:39 ` [PATCH 2/2] selftests/bpf: Cover stack frame number after scalar plus fp Yiyang Chen @ 2026-07-21 10:29 ` bot+bpf-ci 2026-07-21 23:53 ` Eduard Zingerman 0 siblings, 1 reply; 9+ messages in thread From: bot+bpf-ci @ 2026-07-21 10:29 UTC (permalink / raw) To: chenyy23, ast, daniel, andrii, eddyz87, memxor Cc: john.fastabend, martin.lau, song, yonghong.song, jolsa, shuah, emil, bpf, linux-kselftest, linux-kernel, chenyy23, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai [-- 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] selftests/bpf: Cover stack frame number after scalar plus fp 2026-07-21 10:29 ` bot+bpf-ci @ 2026-07-21 23:53 ` Eduard Zingerman 0 siblings, 0 replies; 9+ messages in thread From: Eduard Zingerman @ 2026-07-21 23:53 UTC (permalink / raw) To: bot+bpf-ci, chenyy23, ast, daniel, andrii, memxor Cc: john.fastabend, martin.lau, song, yonghong.song, jolsa, shuah, emil, bpf, linux-kselftest, linux-kernel, martin.lau, clm, ihor.solodrai 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-07-22 8:46 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-21 9:39 [bpf-next v2 0/2] bpf: Preserve stack frame number for commuted arithmetic Yiyang Chen 2026-07-21 9:39 ` [PATCH 1/2] " Yiyang Chen 2026-07-21 9:48 ` sashiko-bot 2026-07-22 7:44 ` Shung-Hsi Yu 2026-07-22 0:12 ` Eduard Zingerman 2026-07-22 8:46 ` Shung-Hsi Yu 2026-07-21 9:39 ` [PATCH 2/2] selftests/bpf: Cover stack frame number after scalar plus fp Yiyang Chen 2026-07-21 10:29 ` bot+bpf-ci 2026-07-21 23:53 ` Eduard Zingerman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.