Re: [PATCH bpf-next v5 16/16] selftests/bpf: Add verifier tests for stack argument validation

bot+bpf-ci
2026-04-18T00:52:33Z

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> index a96b25ebff23..aef21cf2987b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/verifier_stack_arg.c b/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
> new file mode 100644
> index 000000000000..d212b6c3cac7
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
> @@ -0,0 +1,463 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
> +
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"

[ ... ]

> +__noinline __used
> +static int subprog_8args(int a, int b, int c, int d, int e, int f, int g, int h)
> +{
> + return a + b + c + d + e + f + g + h;
> +}

Is subprog_8args intended to be used in any of the tests? It does not
seem to be called anywhere in this file.

Reference: https://lore.kernel.org/bpf/20260417043846.66CFEC2BCB0@smtp.kernel.org/

[ ... ]

> +SEC("tc")
> +__description("stack_arg: read from uninitialized stack arg slot")
> +__failure
> +__arch_x86_64
> +__msg("invalid read from stack arg")
> +__naked void stack_arg_read_uninitialized(void)
> +{
> + asm volatile (
> + "r0 = *(u64 *)(r11 + 8);"
> + "r0 = 0;"
> + "exit;"
> + ::: __clobber_all
> + );
> +}

Should this read from r11 - 8 instead? Since outgoing stack arguments
are at negative offsets from r11, reading from r11 + 8 is out of bounds
and might be rejected for the wrong reason, rather than testing an
uninitialized but valid stack argument slot.

Reference: https://lore.kernel.org/bpf/20260417043846.66CFEC2BCB0@smtp.kernel.org/

[ ... ]

> +SEC("tc")
> +__description("stack_arg: release_reference invalidates stack arg slot")
> +__failure
> +__arch_x86_64
> +__msg("R1 invalid sock access")
> +__naked void stack_arg_release_ref(void)
> +{

[ ... ]

> + /* Release the reference — invalidates the stack arg slot */
> + "r1 = r0;"
> + "call %[bpf_sk_release];"
> + /* Call subprog that dereferences arg6 — should fail */
> + "r1 = 1;"
> + "r2 = 2;"
> + "r3 = 3;"
> + "r4 = 4;"
> + "r5 = 5;"
> + "call subprog_deref_arg6;"

Can this test pass even if the verifier fails to invalidate the stack
slot? subprog_deref_arg6() performs an 8-byte read on the argument.
Since BPF sockets only allow up to 4-byte reads, an 8-byte read is
always invalid.

If the reference isn't correctly invalidated, the valid socket pointer
will still be rejected with "R1 invalid sock access".

To properly test the invalidation, should the subprogram perform a valid
4-byte read instead?

Reference: https://lore.kernel.org/bpf/20260417043846.66CFEC2BCB0@smtp.kernel.org/

[ ... ]

> +SEC("tc")
> +__description("stack_arg: pkt pointer in stack arg slot invalidated after pull_data")
> +__failure
> +__arch_x86_64
> +__msg("invalid access to packet")
> +__naked void stack_arg_stale_pkt_ptr(void)
> +{
> + asm volatile (
> + "r6 = r1;"
> + "r7 = *(u32 *)(r6 + %[__sk_buff_data]);"
> + "r8 = *(u32 *)(r6 + %[__sk_buff_data_end]);"
> + /* check pkt has at least 1 byte */
> + "r0 = r7;"
> + "r0 += 1;"
> + "if r0 > r8 goto l0_%=;"
> + /* Store valid pkt pointer to outgoing arg6 slot */
> + "*(u64 *)(r11 - 8) = r7;"
> + /* bpf_skb_pull_data invalidates all pkt pointers */
> + "r1 = r6;"
> + "r2 = 0;"
> + "call %[bpf_skb_pull_data];"
> + /* Call subprog that dereferences arg6 — should fail */
> + "r1 = 1;"
> + "r2 = 2;"
> + "r3 = 3;"
> + "r4 = 4;"
> + "r5 = 5;"
> + "call subprog_deref_arg6;"

Similarly, does this properly test packet pointer invalidation? The test
verifies 1 byte of the packet pointer, but subprog_deref_arg6() reads
8 bytes.

If bpf_skb_pull_data() fails to invalidate the pointer, the 8-byte read
would still be rejected as out-of-bounds, yielding the expected "invalid
access to packet" message.

Should the packet bounds check verify at least 8 bytes so that a valid
read is guaranteed if the pointer isn't invalidated?

Reference: https://lore.kernel.org/bpf/20260417043846.66CFEC2BCB0@smtp.kernel.org/

[ ... ]

> +SEC("tc")
> +__description("stack_arg: null propagation rejects deref on null branch")
> +__failure
> +__arch_x86_64
> +__msg("R1 invalid mem access")
> +__naked void stack_arg_null_propagation_fail(void)
> +{
> + asm volatile (
> + "r1 = 0;"
> + "*(u64 *)(r10 - 8) = r1;"
> + /* r0 = bpf_map_lookup_elem(&map_hash_8b, &key) */
> + "r2 = r10;"
> + "r2 += -8;"
> + "r1 = %[map_hash_8b] ll;"
> + "call %[bpf_map_lookup_elem];"
> + /* Store PTR_TO_MAP_VALUE_OR_NULL to outgoing arg6 slot */
> + "*(u64 *)(r11 - 8) = r0;"
> + /* null check on r0 */
> + "if r0 != 0 goto l0_%=;"
> + /*
> + * On null branch, outgoing slot is SCALAR(0).
> + * Call subprog that dereferences arg6 — should fail.
> + */
> + "r1 = 0;"
> + "r2 = 0;"
> + "r3 = 0;"
> + "r4 = 0;"
> + "r5 = 0;"
> + "call subprog_deref_arg6;"

Does the __msg() substring match accept the wrong failure mode here?
Dereferencing an unrefined PTR_TO_MAP_VALUE_OR_NULL yields "R1 invalid
mem access 'map_value_or_null'", while dereferencing SCALAR yields
"R1 invalid mem access 'scalar'".

The substring match "R1 invalid mem access" accepts both, allowing the
test to pass even if null propagation fails and the register is still
tracked as a map pointer.

Should it match the exact scalar error, or perhaps test the non-null
branch for success instead?

Reference: https://lore.kernel.org/bpf/20260417043846.66CFEC2BCB0@smtp.kernel.org/

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