Re: [PATCH bpf-next v2 2/2] selftests/bpf: add tests for PTR_TO_FLOW_KEYS constant offset bounds

[Eduard Zingerman <eddyz87@gmail.com>]

On Fri, 2026-06-05 at 02:07 +0800, Nuoqi Gui wrote:
> Add verifier tests covering constant pointer arithmetic on a
> PTR_TO_FLOW_KEYS register, which regressed with commit 022ac0750883
> ("bpf: use reg->var_off instead of reg->off for pointers"): an
> out-of-bounds offset introduced as flow_keys += K and then dereferenced
> at insn->off 0 was accepted, while the equivalent flow_keys + K direct
> offset was rejected.
> 
> The tests check that:
>  - in-bounds constant arithmetic on the keys pointer is still accepted,
>  - an out-of-bounds offset introduced via constant arithmetic is rejected
>    for both read and write, with the same diagnostic as the direct
>    insn->off form.
> 
> Signed-off-by: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
> ---
>  .../selftests/bpf/prog_tests/verifier.c       |  2 +
>  .../selftests/bpf/progs/verifier_flow_keys.c  | 77 +++++++++++++++++++
>  2 files changed, 79 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/progs/verifier_flow_keys.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> index 219ff2969868..dae26dda3782 100644
> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> @@ -38,6 +38,7 @@
>  #include "verifier_div0.skel.h"
>  #include "verifier_div_mod_bounds.skel.h"
>  #include "verifier_div_overflow.skel.h"
> +#include "verifier_flow_keys.skel.h"
>  #include "verifier_global_subprogs.skel.h"
>  #include "verifier_global_ptr_args.skel.h"
>  #include "verifier_gotol.skel.h"
> @@ -189,6 +190,7 @@ void test_verifier_direct_stack_access_wraparound(void) { RUN(verifier_direct_st
>  void test_verifier_div0(void)                 { RUN(verifier_div0); }
>  void test_verifier_div_mod_bounds(void)       { RUN(verifier_div_mod_bounds); }
>  void test_verifier_div_overflow(void)         { RUN(verifier_div_overflow); }
> +void test_verifier_flow_keys(void)            { RUN(verifier_flow_keys); }
>  void test_verifier_global_subprogs(void)      { RUN(verifier_global_subprogs); }
>  void test_verifier_global_ptr_args(void)      { RUN(verifier_global_ptr_args); }
>  void test_verifier_gotol(void)                { RUN(verifier_gotol); }
> diff --git a/tools/testing/selftests/bpf/progs/verifier_flow_keys.c b/tools/testing/selftests/bpf/progs/verifier_flow_keys.c
> new file mode 100644
> index 000000000000..512e5d1d2665
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_flow_keys.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Constant-offset bounds checks for PTR_TO_FLOW_KEYS pointer arithmetic. */
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +
> +/* sizeof(struct bpf_flow_keys) is well under 4096, so +0x1000 is OOB. */
> +
> +SEC("flow_dissector")
> +__description("flow_keys: in-bounds constant pointer arithmetic accepted")
> +__success
> +__naked void flow_keys_const_inbounds(void)
> +{
> +	asm volatile ("					\
> +	r1 = *(u64 *)(r1 + %[flow_keys]);		\
> +	r1 += 8;					\
> +	r0 = *(u64 *)(r1 + 0);				\
> +	r0 = 0;						\
> +	exit;						\
> +"	:
> +	: __imm_const(flow_keys, offsetof(struct __sk_buff, flow_keys))
> +	: __clobber_all);
> +}
> +
> +SEC("flow_dissector")
> +__description("flow_keys: OOB via constant pointer arithmetic rejected")
> +__failure __msg("invalid access to flow keys off=4096 size=8")
> +__naked void flow_keys_const_oob_read(void)
> +{
> +	asm volatile ("					\
> +	r1 = *(u64 *)(r1 + %[flow_keys]);		\
> +	r1 += 4096;					\
> +	r0 = *(u64 *)(r1 + 0);				\
> +	r0 = 0;						\
> +	exit;						\
> +"	:
> +	: __imm_const(flow_keys, offsetof(struct __sk_buff, flow_keys))
> +	: __clobber_all);
> +}
> +
> +SEC("flow_dissector")
> +__description("flow_keys: OOB write via constant pointer arithmetic rejected")
> +__failure __msg("invalid access to flow keys off=4096 size=8")
> +__naked void flow_keys_const_oob_write(void)
> +{
> +	asm volatile ("					\
> +	r1 = *(u64 *)(r1 + %[flow_keys]);		\
> +	r1 += 4096;					\
> +	r2 = 0;						\
> +	*(u64 *)(r1 + 0) = r2;				\
> +	r0 = 0;						\
> +	exit;						\
> +"	:
> +	: __imm_const(flow_keys, offsetof(struct __sk_buff, flow_keys))
> +	: __clobber_all);
> +}
> +
> +/* Equivalent OOB expressed directly in insn->off; this form was always
> + * rejected and is kept to show both forms now share one diagnostic.
> + */
> +SEC("flow_dissector")
> +__description("flow_keys: OOB via insn->off rejected")
> +__failure __msg("invalid access to flow keys off=4096 size=8")
> +__naked void flow_keys_insn_off_oob(void)
> +{
> +	asm volatile ("					\
> +	r1 = *(u64 *)(r1 + %[flow_keys]);		\
> +	r0 = *(u64 *)(r1 + 4096);			\
> +	r0 = 0;						\
> +	exit;						\
> +"	:
> +	: __imm_const(flow_keys, offsetof(struct __sk_buff, flow_keys))
> +	: __clobber_all);
> +}
> +
> +char _license[] SEC("license") = "GPL";

Could you please also add a test with a truly varying offset?
Like below:

__naked void flow_keys_var_read(void)
{
	asm volatile ("					\
	r6 = r1;					\
	call %[bpf_get_prandom_u32];			\
	r0 &= 0xFFFF;					\
	r1 = *(u64 *)(r6 + %[flow_keys]);		\
	r1 += r0;					\
	r0 = *(u64 *)(r1 + 0);				\
	r0 = 0;						\
	exit;						\
"	:
	: __imm_const(flow_keys, offsetof(struct __sk_buff, flow_keys)),
	  __imm(bpf_get_prandom_u32)
	: __clobber_all);
}