Re: [PATCH bpf-next v4 7/7] selftests/bpf: verify zext_dst annotations for various instructions
Min-gyu Kim <[email protected]>
| Newsgroups | org.kernel.vger.bpf |
|---|---|
| Message-ID | <CAGKGUvmh9puwO6BLME1sZq1QCdvMk-P1qsBcwB0PyDs+Dpa2Rw@mail.gmail.com> |
Hello, Thank you for preparing the fix. I tested the v4 series against my reproducer on s390x. Tested revisions I first built and booted the exact final commit of the v4 series applied by the patchwork bot: 8b365b3c68b474a1053ec0755dacdc751578afb0 Linux 7.2.0-rc6-g8b365b3c68b4 I then fetched, built, and booted the current master directly from the official bpf-next repository on 2026-08-17: repository: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git commit: 8eb18920646415f6b4f100b34bd456c4fe002196 Linux: 7.2.0-rc6-g8eb189206464 I verified that the v4 final commit is an ancestor of this master revision, which contains 106 subsequent commits. Both tested source trees were clean. Test conditions Both kernels were tested in s390x QEMU guests with Debian trixie userspace. The reproducer ran as uid 1000 with every capability set, including the bounding set, cleared: CapInh: 0000000000000000 CapPrm: 0000000000000000 CapEff: 0000000000000000 CapBnd: 0000000000000000 CapAmb: 0000000000000000 The relevant runtime and build conditions were: kernel.unprivileged_bpf_disabled = 0 net.core.bpf_jit_enable = 1 CONFIG_BPF_JIT_ALWAYS_ON=y The BPF program was accepted, attached as a socket filter, and JIT-executed in every run. The result was therefore not caused by verifier rejection or interpreter fallback. Results The results were identical on the v4 final commit and the later bpf-next master revision. I ran my reproducer ten times on each kernel. All runs stopped before a kernel address could be disclosed or an out-of-bounds read/write primitive could be established. None reached the task, credential, or privilege-escalation stages. I also ran a focused regression test using the same verifier/JIT mismatch trigger to distinguish the attempted out-of-bounds accesses from valid map value accesses. On each kernel, all 20 runs confirmed that both the read and write remained within the map value. Finally, I ran the merged zext_lost_across_checkpoint regression program with BPF_F_TEST_STATE_FREQ and BPF_F_TEST_RND_HI32. On each kernel, BPF_PROG_TEST_RUN completed all 20 repetitions with: PASS: zext_lost_across_checkpoint retval=0 repeat=20 The kernel taint value remained zero after testing. I found no BTF error, BUG, WARNING, Oops, user process fault, kernel panic, timeout, or signal termination. These results confirm that the merged v4 series prevents the s390x verifier/JIT zero-extension mismatch exercised by my reproducer. The same behavior is preserved in the tested current bpf-next master revision. For the v4 series, my test result is: Tested-by: Min-gyu Kim <[email protected]> Thank you for your time. Thanks, Min-gyu Kim 2026년 8월 8일 (토) 오전 6:00, Eduard Zingerman <[email protected]>님이 작성: > > Includes the following test cases: > - a test showing that zero extension flags do not propagate through > state pruning in the unpatched kernel. > - a 32-bit subregister consumed by MOV32 and ALU32 operations > (never zext'ed); > - a 64-bit MOV (never zext'ed); > - a narrow (32-bit) BPF_LDX load whose result is read as 64-bit; > - 32-bit atomic fetch_add and cmpxchg whose result is read as 64-bit; > - a CFG case where a 32-bit definition's upper half is used only on one > of two branches; > - no zext for dead registers; > - LD_ABS defines only lower 32 bits, hence needs zext when the result > is used as 64-bits; > - helper, kfunc and subprogram parameters are considered to use full > 64 bits; > - a 32-bit subregister consumed by JMP32 (X/K) operations; > - a 32-bit subregister consumed by JMP (X/K) operations; > - a 64-bit register consumed by both JMP and JMP32 operations > (never zext'ed); > - ALU64 and address space cast operations on arena pointers; > - memory loads using BPF_PROBE_MEM instructions. > > Signed-off-by: Eduard Zingerman <[email protected]> > --- > tools/testing/selftests/bpf/prog_tests/verifier.c | 2 + > tools/testing/selftests/bpf/progs/verifier_zext.c | 392 ++++++++++++++++++++++ > 2 files changed, 394 insertions(+) > > diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c > index b79bafca68f7..0baa74618fa0 100644 > --- a/tools/testing/selftests/bpf/prog_tests/verifier.c > +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c > @@ -126,6 +126,7 @@ > #include "verifier_jit_inline.skel.h" > #include "irq.skel.h" > #include "verifier_ctx_ptr_param.skel.h" > +#include "verifier_zext.skel.h" > > #define MAX_ENTRIES 11 > > @@ -281,6 +282,7 @@ void test_irq(void) { RUN(irq); } > void test_verifier_mtu(void) { RUN(verifier_mtu); } > void test_verifier_jit_inline(void) { RUN(verifier_jit_inline); } > void test_verifier_ctx_ptr_param(void) { RUN(verifier_ctx_ptr_param); } > +void test_verifier_zext(void) { RUN_TESTS(verifier_zext); } > > static int init_test_val_map(struct bpf_object *obj, char *map_name) > { > diff --git a/tools/testing/selftests/bpf/progs/verifier_zext.c b/tools/testing/selftests/bpf/progs/verifier_zext.c > new file mode 100644 > index 000000000000..8f2362da91d6 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/verifier_zext.c > @@ -0,0 +1,392 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <linux/bpf.h> > +#include <bpf/bpf_helpers.h> > +#include "../../../include/linux/filter.h" > +#include <bpf_arena_common.h> > +#include <bpf/bpf_core_read.h> > +#include "bpf_misc.h" > + > +struct { > + __uint(type, BPF_MAP_TYPE_ARENA); > + __uint(map_flags, BPF_F_MMAPABLE | BPF_F_NO_USER_CONV); > + __uint(max_entries, 1); > +} arena SEC(".maps"); > + > +extern long bpf_kfunc_call_test4(signed char a, short b, int c, long d) __ksym; > + > +/* to retain debug info for BTF generation */ > +void __kfunc_btf_root(void) > +{ > + bpf_kfunc_call_test4(0, 0, 0, 0); > + bpf_arena_alloc_pages(0, 0, 0, 0, 0); > + bpf_rdonly_cast(0, 0); > +} > + > +SEC("socket") > +__flag(BPF_F_TEST_STATE_FREQ) > +__flag(BPF_F_TEST_RND_HI32) > +__success __retval(0) > +__naked void zext_lost_across_checkpoint(void) > +{ > + asm volatile (" \ > + call %[bpf_ktime_get_ns]; \ > + r8 = r0; \ > + r6 = 0xdeadbeefcafebabe ll; /* inject some value for r6's upper half */ \ > + if r8 != 0 goto 1f; /* fall-through cached first, branch pruned */ \ > + r6 = 32; /* full 64-bit def */ \ > + goto 2f; \ > +1: w6 = 32; /* 32-bit def, zext mark lost */ \ > +2: r0 = r6; /* buggy verifier believed upper 32 bits are 0 */ \ > + /* and thus did not zero extended w6 = 32. */ \ > + r0 >>= 32; \ > + exit; \ > +" : > + : __imm(bpf_ktime_get_ns) > + : __clobber_all); > +} > + > +/* 32-bit ALU result read as 64-bit -> zext */ > +SEC("socket") > +__success __log_level(2) > +__msg("w1 = w0{{ +}}; zext") > +__naked void zext_alu32_hi_used(void) > +{ > + asm volatile (" \ > + call %[bpf_get_prandom_u32]; \ > + w1 = w0; \ > + r0 = r1; \ > + exit; \ > +" : > + : __imm(bpf_get_prandom_u32) > + : __clobber_all); > +} > + > +/* 32-bit ALU result read only as 32-bit -> no zext */ > +SEC("socket") > +__success __log_level(2) > +__not_msg("; zext") > +__naked void no_zext_alu32_hi_unused(void) > +{ > + asm volatile (" \ > + call %[bpf_get_prandom_u32]; \ > + w1 = w0; /* MOV */ \ > + w2 = w1; \ > + w2 += w1; /* ALU32, BPF_X */ \ > + w2 += 1; /* ALU32, BPF_K */ \ > + w2 = w2; /* keep w2 alive for previous instruction */ \ > + r0 = 0; \ > + exit; \ > +" : > + : __imm(bpf_get_prandom_u32) > + : __clobber_all); > +} > + > +/* 64-bit definition is never zero extended */ > +SEC("socket") > +__success __log_level(2) > +__not_msg("r1 = r0{{.*}}; zext") > +__naked void no_zext_mov64(void) > +{ > + asm volatile (" \ > + call %[bpf_get_prandom_u32]; \ > + r1 = r0; \ > + r0 = r1; \ > + exit; \ > +" : > + : __imm(bpf_get_prandom_u32) > + : __clobber_all); > +} > + > +/* Narrow load result read as 64-bit -> zext */ > +SEC("socket") > +__success __log_level(2) > +__msg("r1 = *(u32 *)(r10 -8){{ +}}; zext") > +__naked void zext_narrow_load_hi_used(void) > +{ > + asm volatile (" \ > + r0 = 0; \ > + *(u64 *)(r10 - 8) = r0; \ > + r1 = *(u32 *)(r10 - 8); \ > + r0 = r1; \ > + exit; \ > +" ::: __clobber_all); > +} > + > +/* 32-bit atomic fetch result read as 64-bit -> zext */ > +SEC("socket") > +__success __log_level(2) > +__msg("r1 = atomic_fetch_add((u32 *)(r10 -8), r1){{ +}}; zext") > +__naked void zext_atomic_fetch32_hi_used(void) > +{ > + asm volatile (" \ > + r1 = 0; \ > + *(u64 *)(r10 - 8) = r1; \ > + w1 = 1; \ > + .8byte %[fetch_add32]; \ > + r0 = r1; \ > + exit; \ > +" : > + : __imm_insn(fetch_add32, > + BPF_ATOMIC_OP(BPF_W, BPF_ADD | BPF_FETCH, BPF_REG_10, BPF_REG_1, -8)) > + : __clobber_all); > +} > + > +/* 32-bit atomic cmpxchg result (r0) read as 64-bit -> zext */ > +SEC("socket") > +__success __log_level(2) > +__msg("r0 = atomic_cmpxchg((u32 *)(r10 -8), r0, r1){{ +}}; zext") > +__naked void zext_cmpxchg32_hi_used(void) > +{ > + asm volatile (" \ > + r1 = 0; \ > + *(u64 *)(r10 - 8) = r1; \ > + w0 = 0; \ > + w1 = 1; \ > + .8byte %[cmpxchg32]; \ > + r2 = r0; \ > + r0 = r2; \ > + exit; \ > +" : > + : __imm_insn(cmpxchg32, > + BPF_ATOMIC_OP(BPF_W, BPF_CMPXCHG, BPF_REG_10, BPF_REG_1, -8)) > + : __clobber_all); > +} > + > +/* 32-bit def before a branch, upper half used on one branch -> zext */ > +SEC("socket") > +__success __log_level(2) > +__msg("w6 = 32{{ +}}; zext") > +__naked void zext_cfg_hi_used_one_branch(void) > +{ > + asm volatile (" \ > + call %[bpf_get_prandom_u32]; \ > + w6 = 32; \ > + if r0 == 0 goto 1f; \ > + r0 = r6; \ > + exit; \ > +1: r0 = 0; \ > + exit; \ > +" : > + : __imm(bpf_get_prandom_u32) > + : __clobber_all); > +} > + > +/* r1's upper half is dead, so 'w1 = 1' must NOT be marked for zero extension. */ > +SEC("socket") > +__success __log_level(2) > +__not_msg("w1 = 1{{.*}}; zext") > +__naked void no_zext_other_reg_hi_used(void) > +{ > + asm volatile (" \ > + call %[bpf_get_prandom_u32]; \ > + r6 = r0; \ > + r6 <<= 32; \ > + w1 = 1; \ > + r0 = r6; \ > + exit; \ > +" : > + : __imm(bpf_get_prandom_u32) > + : __clobber_all); > +} > + > +/* LD_ABS defines r0; when r0 is read as 64-bit it must be zero extended */ > +SEC("socket") > +__success __log_level(2) > +__msg("r0 = *(u8 *)skb[0]{{.*}}; zext") > +__naked void zext_ld_abs_hi_used(void) > +{ > + asm volatile (" \ > + r6 = r1; \ > + r0 = *(u8 *)skb[0]; \ > + r7 = r0; \ > + r0 = r7; \ > + exit; \ > +" ::: __clobber_all); > +} > + > +/* Helper parameters are read as 64-bit (call_use_mask() fallback) */ > +SEC("socket") > +__success __log_level(2) > +__msg("w2 = 1{{ +}}; zext") > +__naked void helper_param_read_as_64bit(void) > +{ > + asm volatile (" \ > + r1 = r10; \ > + r1 += -8; \ > + w2 = 1; \ > + call %[bpf_trace_printk]; \ > + r0 = 0; \ > + exit; \ > +" : > + : __imm(bpf_trace_printk) > + : __clobber_all); > +} > + > +static __used __naked int subprog_reads_arg_as_64bit(void) > +{ > + asm volatile (" \ > + r0 = r1; \ > + exit; \ > +" ::: __clobber_all); > +} > + > +/* subprogram parameters are conservatively read as 64-bit */ > +SEC("socket") > +__success __log_level(2) > +__msg("w1 = w0{{ +}}; zext") > +__naked void subprog_param_read_as_64bit(void) > +{ > + asm volatile (" \ > + call %[bpf_get_prandom_u32]; \ > + w1 = w0; \ > + call subprog_reads_arg_as_64bit; \ > + r0 = 0; \ > + exit; \ > +" : > + : __imm(bpf_get_prandom_u32) > + : __clobber_all); > +} > + > +/* kfunc parameters are zero extended */ > +SEC("tc") > +__success __log_level(2) > +__msg("w1 = 1{{ +}}; zext") > +__msg("w2 = 1{{ +}}; zext") > +__msg("w3 = 1{{ +}}; zext") > +__msg("w4 = 1{{ +}}; zext") > +__naked void kfunc_param_read_per_btf(void) > +{ > + asm volatile (" \ > + w1 = 1; \ > + w2 = 1; \ > + w3 = 1; \ > + w4 = 1; \ > + call bpf_kfunc_call_test4; \ > + r0 = 0; \ > + exit; \ > +" ::: __clobber_all); > +} > + > +SEC("socket") > +__success __log_level(2) > +__not_msg("; zext") > +__naked void alu32_and_32bit_conditional(void) > +{ > + asm volatile (" \ > + call %[bpf_get_prandom_u32]; \ > + w1 = w0; \ > + if w1 > 42 goto 1f; /* BPF_K */ \ > + w2 = 28; \ > + if w2 > w1 goto 1f; /* BPF_X */ \ > + r0 = 0; \ > +1: exit; \ > +" : > + : __imm(bpf_get_prandom_u32) > + : __clobber_all); > +} > + > +SEC("socket") > +__success __log_level(2) > +__msg("w1 = w0{{ +}}; zext") > +__naked void alu32_and_64bit_conditional(void) > +{ > + asm volatile (" \ > + call %[bpf_get_prandom_u32]; \ > + w1 = w0; \ > + if r1 > 42 goto 1f; /* BPF_K */ \ > + r2 = 28; \ > + if r2 > r1 goto 1f; /* BPF_X */ \ > + r0 = 0; \ > +1: exit; \ > +" : > + : __imm(bpf_get_prandom_u32) > + : __clobber_all); > +} > + > +SEC("socket") > +__success __log_level(2) > +__not_msg("; zext") > +__naked void alu64_and_conditionals(void) > +{ > + asm volatile (" \ > + call %[bpf_get_prandom_u32]; \ > + r1 = r0; \ > + if w1 > 42 goto 1f; /* BPF_K */ \ > + if r1 > 42 goto 1f; /* BPF_K */ \ > + r2 = 28; \ > + if w2 > w1 goto 1f; /* BPF_X */ \ > + if r2 > r1 goto 1f; /* BPF_X */ \ > + r0 = 0; \ > +1: exit; \ > +" : > + : __imm(bpf_get_prandom_u32) > + : __clobber_all); > +} > + > +#ifdef __BPF_FEATURE_ADDR_SPACE_CAST > + > +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid") > +__arch_s390x > +__xlated("7: w1 = w0") > +__xlated("8: w1 = w1") > +__xlated("9: w1 += 8") > +__xlated("10: w1 = w1") > +__xlated("11: w2 = w1") > +__xlated("12: w2 = w2") > +__xlated("13: *(u64 *)(r1 +0) = r2") > +__naked void arena_ptr(void) > +{ > + asm volatile (" \ > + r1 = %[arena] ll; \ > + r2 = 0; \ > + r3 = 1; \ > + r4 = 0; \ > + r5 = 0; \ > + call %[bpf_arena_alloc_pages]; \ > + r1 = addr_space_cast(r0, 0, 1); /* needs zext */ \ > + r1 += 8; /* needs zext */ \ > + r2 = addr_space_cast(r1, 1, 0); /* needs zext because of BPF_F_NO_USER_CONV */ \ > + *(u64 *)(r1 +0) = r2; \ > + r0 = 0; \ > + exit; \ > +" : > + : __imm(bpf_arena_alloc_pages), > + __imm_addr(arena) > + : __clobber_all); > +} > + > +#endif > + > +/* Check if probe mem loads keep their zero extension. */ > +SEC("socket") > +__success __log_level(2) > +__arch_s390x > +__xlated("3: r1 = *(u64 *)(r0 +0)") > +__xlated("4: r2 = *(u32 *)(r0 +0)") > +__xlated("5: w2 = w2") > +__xlated("6: r3 = *(u16 *)(r0 +0)") > +__xlated("7: w3 = w3") > +__xlated("8: r4 = *(u8 *)(r0 +0)") > +__xlated("9: w4 = w4") > +__naked void probe_mem(void) > +{ > + asm volatile (" \ > + r1 = 0; \ > + r2 = 0; \ > + call %[bpf_rdonly_cast]; \ > + r1 = *(u64 *)(r0 + 0); /* BPF_PROBE_MEM */ \ > + r2 = *(u32 *)(r0 + 0); /* BPF_PROBE_MEM */ \ > + r3 = *(u16 *)(r0 + 0); /* BPF_PROBE_MEM */ \ > + r4 = *(u8 *)(r0 + 0); /* BPF_PROBE_MEM */ \ > + r0 = r1; /* make the registers used */ \ > + r0 += r2; \ > + r0 += r3; \ > + r0 += r4; \ > +1: exit; \ > +" : > + : __imm(bpf_rdonly_cast) > + : __clobber_all); > +} > + > +char _license[] SEC("license") = "GPL"; > > -- > 2.55.0