Yes, I will add a selftest with a truly varying offset, as suggested. I will include this in the next version.
> -----Original Messages----- > From: "Eduard Zingerman" <[email protected]> > Send time:Friday, 05/06/2026 10:23:41 > To: "Nuoqi Gui" <[email protected]>, [email protected], > [email protected], [email protected] > Cc: "Martin KaFai Lau" <[email protected]>, "Kumar Kartikeya Dwivedi" > <[email protected]>, "Song Liu" <[email protected]>, "Yonghong Song" > <[email protected]>, "Jiri Olsa" <[email protected]>, "Shuah Khan" > <[email protected]>, "Shenghao Yuan" <[email protected]>, "Yazhou Tang" > <[email protected]>, "Matt Bobrowski" <[email protected]>, > "Emil Tsalapatis" <[email protected]>, [email protected], > [email protected], [email protected] > Subject: Re: [PATCH bpf-next v2 2/2] selftests/bpf: add tests for > PTR_TO_FLOW_KEYS constant offset bounds > > 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 <[email protected]> > > --- > > .../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); > }
