> 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