* [PATCH bpf-next v2 0/5] bpf: infer zext_dst based on static register liveness analysis
@ 2026-08-01 1:09 Eduard Zingerman
2026-08-01 1:09 ` [PATCH bpf-next v2 1/5] bpf: do not print a newline after disassembly in bpf_verbose_insn() Eduard Zingerman
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Eduard Zingerman @ 2026-08-01 1:09 UTC (permalink / raw)
To: bpf, ast, andrii
Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87, memxor,
iii, gimm78064
Min-gyu Kim reported a bug in 32-bit operations zero extension
handling [1]. The bug was introduced by the commit [2].
The tl;dr of the bug is that the verifier does not carry zero
extension marks across pruning points. The detailed mechanism is
described in patch #3.
Fix this by reworking zero extension logic to avoid main-path based
subreg_def tracking, and instead extend the live registers analysis to
track upper register halves' liveness.
As noted in the commit message for patch #3:
There is one notable drop in precision: whenever a BPF subprogram is
called, all 64 bits of parameter registers are presumed to be used.
The assumption is that such a drop in precision would not inflict
noticeable performance penalty.
Ilya, could you please help with testing this conjecture?
Min-gyu, could you please test the proposed fix against your
reproducer?
Side note:
Patch #1 is a small refactoring for disasm.c, as patch #3 adds a new
location where bpf_verbose_insn() is called and there as well I have
to wrangle with the newline added by the disasm code.
[1] https://lore.kernel.org/bpf/CAGKGUv=sOuqQtA1Ub-5JXfA4FPosJFYKAQE4B79cK+P1erxqtg@mail.gmail.com/
[2] commit 107e16979905 ("bpf: disable and remove registers chain based liveness")
Changelog:
v1 -> v2:
- fixed BPF_JMP32 handling, these no longer for zero extension (sashiko)
- removed dead code in print_insn_for_graph() (bot+bpf-ci)
- reworked bpf_is_reg64() to drop dead code (sashiko, bot+bpf-ci)
v1: https://lore.kernel.org/bpf/20260731-static-zext-v1-0-98a4dc73e94b@gmail.com/T/
---
Eduard Zingerman (5):
bpf: do not print a newline after disassembly in bpf_verbose_insn()
bpf: track upper 32-bit register halves' liveness in compute_live_registers()
bpf: infer zext_dst based on static register liveness analysis
bpf: simplify the bpf_is_reg64()
selftests/bpf: verify zext_dst annotations for various instructions
include/linux/bpf_verifier.h | 9 +-
kernel/bpf/backtrack.c | 1 +
kernel/bpf/disasm.c | 68 ++---
kernel/bpf/fixups.c | 55 +++-
kernel/bpf/liveness.c | 144 +++++++---
kernel/bpf/verifier.c | 201 +-------------
tools/bpf/bpftool/xlated_dumper.c | 19 +-
tools/testing/selftests/bpf/disasm_helpers.c | 3 +-
tools/testing/selftests/bpf/prog_tests/verifier.c | 2 +
tools/testing/selftests/bpf/progs/verifier_zext.c | 315 ++++++++++++++++++++++
10 files changed, 519 insertions(+), 298 deletions(-)
---
base-commit: fdec474c65fd35d5a6e1497ed50a9f98c07192f0
change-id: 20260731-static-zext-938cd128273c
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH bpf-next v2 1/5] bpf: do not print a newline after disassembly in bpf_verbose_insn() 2026-08-01 1:09 [PATCH bpf-next v2 0/5] bpf: infer zext_dst based on static register liveness analysis Eduard Zingerman @ 2026-08-01 1:09 ` Eduard Zingerman 2026-08-01 1:09 ` [PATCH bpf-next v2 2/5] bpf: track upper 32-bit register halves' liveness in compute_live_registers() Eduard Zingerman ` (3 subsequent siblings) 4 siblings, 0 replies; 13+ messages in thread From: Eduard Zingerman @ 2026-08-01 1:09 UTC (permalink / raw) To: bpf, ast, andrii Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87, memxor, iii, gimm78064, Quentin Monnet At the moment there are more callsites that want bpf_verbose_insn() to not print a newline after the instruction, than callsites that want a newline. Drop '\n' from disasm.c. Non-functional change. The changes in bpftool are verified by writing a bpf program using a variety of instructions and comparing `prog dump xlated` output in the following modes: plain, opcodes, visual, visual opcodes. The output before and after the changes is identical. Cc: Quentin Monnet <qmo@kernel.org> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> --- kernel/bpf/backtrack.c | 1 + kernel/bpf/disasm.c | 68 ++++++++++++++-------------- kernel/bpf/liveness.c | 6 +-- kernel/bpf/verifier.c | 1 + tools/bpf/bpftool/xlated_dumper.c | 19 ++------ tools/testing/selftests/bpf/disasm_helpers.c | 3 +- 6 files changed, 43 insertions(+), 55 deletions(-) diff --git a/kernel/bpf/backtrack.c b/kernel/bpf/backtrack.c index 2e4ae0ef0860..7c4599e65ece 100644 --- a/kernel/bpf/backtrack.c +++ b/kernel/bpf/backtrack.c @@ -285,6 +285,7 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx, verbose(env, "stack=%s before ", env->tmp_str_buf); verbose(env, "%d: ", idx); bpf_verbose_insn(env, insn); + verbose(env, "\n"); } /* If there is a history record that some registers gained range at this insn, diff --git a/kernel/bpf/disasm.c b/kernel/bpf/disasm.c index 0391b3bc0073..50b3ca5149a0 100644 --- a/kernel/bpf/disasm.c +++ b/kernel/bpf/disasm.c @@ -139,7 +139,7 @@ static void print_bpf_end_insn(bpf_insn_print_t verbose, void *private_data, const struct bpf_insn *insn) { - verbose(private_data, "(%02x) r%d = %s%d r%d\n", + verbose(private_data, "(%02x) r%d = %s%d r%d", insn->code, insn->dst_reg, BPF_SRC(insn->code) == BPF_TO_BE ? "be" : "le", insn->imm, insn->dst_reg); @@ -149,7 +149,7 @@ static void print_bpf_bswap_insn(bpf_insn_print_t verbose, void *private_data, const struct bpf_insn *insn) { - verbose(private_data, "(%02x) r%d = bswap%d r%d\n", + verbose(private_data, "(%02x) r%d = bswap%d r%d", insn->code, insn->dst_reg, insn->imm, insn->dst_reg); } @@ -197,19 +197,19 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs, else print_bpf_end_insn(verbose, cbs->private_data, insn); } else if (BPF_OP(insn->code) == BPF_NEG) { - verbose(cbs->private_data, "(%02x) %c%d = -%c%d\n", + verbose(cbs->private_data, "(%02x) %c%d = -%c%d", insn->code, class == BPF_ALU ? 'w' : 'r', insn->dst_reg, class == BPF_ALU ? 'w' : 'r', insn->dst_reg); } else if (is_addr_space_cast(insn)) { - verbose(cbs->private_data, "(%02x) r%d = addr_space_cast(r%d, %u, %u)\n", + verbose(cbs->private_data, "(%02x) r%d = addr_space_cast(r%d, %u, %u)", insn->code, insn->dst_reg, insn->src_reg, ((u32)insn->imm) >> 16, (u16)insn->imm); } else if (is_mov_percpu_addr(insn)) { - verbose(cbs->private_data, "(%02x) r%d = &(void __percpu *)(r%d)\n", + verbose(cbs->private_data, "(%02x) r%d = &(void __percpu *)(r%d)", insn->code, insn->dst_reg, insn->src_reg); } else if (BPF_SRC(insn->code) == BPF_X) { - verbose(cbs->private_data, "(%02x) %c%d %s %s%c%d\n", + verbose(cbs->private_data, "(%02x) %c%d %s %s%c%d", insn->code, class == BPF_ALU ? 'w' : 'r', insn->dst_reg, is_sdiv_smod(insn) ? bpf_alu_sign_string[BPF_OP(insn->code) >> 4] @@ -218,7 +218,7 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs, class == BPF_ALU ? 'w' : 'r', insn->src_reg); } else { - verbose(cbs->private_data, "(%02x) %c%d %s %d\n", + verbose(cbs->private_data, "(%02x) %c%d %s %d", insn->code, class == BPF_ALU ? 'w' : 'r', insn->dst_reg, is_sdiv_smod(insn) ? bpf_alu_sign_string[BPF_OP(insn->code) >> 4] @@ -227,7 +227,7 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs, } } else if (class == BPF_STX) { if (BPF_MODE(insn->code) == BPF_MEM) - verbose(cbs->private_data, "(%02x) *(%s *)(r%d %+d) = r%d\n", + verbose(cbs->private_data, "(%02x) *(%s *)(r%d %+d) = r%d", insn->code, bpf_ldst_string[BPF_SIZE(insn->code) >> 3], insn->dst_reg, @@ -235,7 +235,7 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs, else if (BPF_MODE(insn->code) == BPF_ATOMIC && (insn->imm == BPF_ADD || insn->imm == BPF_AND || insn->imm == BPF_OR || insn->imm == BPF_XOR)) { - verbose(cbs->private_data, "(%02x) lock *(%s *)(r%d %+d) %s r%d\n", + verbose(cbs->private_data, "(%02x) lock *(%s *)(r%d %+d) %s r%d", insn->code, bpf_ldst_string[BPF_SIZE(insn->code) >> 3], insn->dst_reg, insn->off, @@ -246,7 +246,7 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs, insn->imm == (BPF_AND | BPF_FETCH) || insn->imm == (BPF_OR | BPF_FETCH) || insn->imm == (BPF_XOR | BPF_FETCH))) { - verbose(cbs->private_data, "(%02x) r%d = atomic%s_fetch_%s((%s *)(r%d %+d), r%d)\n", + verbose(cbs->private_data, "(%02x) r%d = atomic%s_fetch_%s((%s *)(r%d %+d), r%d)", insn->code, insn->src_reg, BPF_SIZE(insn->code) == BPF_DW ? "64" : "", bpf_atomic_alu_string[BPF_OP(insn->imm) >> 4], @@ -254,7 +254,7 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs, insn->dst_reg, insn->off, insn->src_reg); } else if (BPF_MODE(insn->code) == BPF_ATOMIC && insn->imm == BPF_CMPXCHG) { - verbose(cbs->private_data, "(%02x) r0 = atomic%s_cmpxchg((%s *)(r%d %+d), r0, r%d)\n", + verbose(cbs->private_data, "(%02x) r0 = atomic%s_cmpxchg((%s *)(r%d %+d), r0, r%d)", insn->code, BPF_SIZE(insn->code) == BPF_DW ? "64" : "", bpf_ldst_string[BPF_SIZE(insn->code) >> 3], @@ -262,44 +262,44 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs, insn->src_reg); } else if (BPF_MODE(insn->code) == BPF_ATOMIC && insn->imm == BPF_XCHG) { - verbose(cbs->private_data, "(%02x) r%d = atomic%s_xchg((%s *)(r%d %+d), r%d)\n", + verbose(cbs->private_data, "(%02x) r%d = atomic%s_xchg((%s *)(r%d %+d), r%d)", insn->code, insn->src_reg, BPF_SIZE(insn->code) == BPF_DW ? "64" : "", bpf_ldst_string[BPF_SIZE(insn->code) >> 3], insn->dst_reg, insn->off, insn->src_reg); } else if (BPF_MODE(insn->code) == BPF_ATOMIC && insn->imm == BPF_LOAD_ACQ) { - verbose(cbs->private_data, "(%02x) r%d = load_acquire((%s *)(r%d %+d))\n", + verbose(cbs->private_data, "(%02x) r%d = load_acquire((%s *)(r%d %+d))", insn->code, insn->dst_reg, bpf_ldst_string[BPF_SIZE(insn->code) >> 3], insn->src_reg, insn->off); } else if (BPF_MODE(insn->code) == BPF_ATOMIC && insn->imm == BPF_STORE_REL) { - verbose(cbs->private_data, "(%02x) store_release((%s *)(r%d %+d), r%d)\n", + verbose(cbs->private_data, "(%02x) store_release((%s *)(r%d %+d), r%d)", insn->code, bpf_ldst_string[BPF_SIZE(insn->code) >> 3], insn->dst_reg, insn->off, insn->src_reg); } else { - verbose(cbs->private_data, "BUG_%02x\n", insn->code); + verbose(cbs->private_data, "BUG_%02x", insn->code); } } else if (class == BPF_ST) { if (BPF_MODE(insn->code) == BPF_MEM) { - verbose(cbs->private_data, "(%02x) *(%s *)(r%d %+d) = %d\n", + verbose(cbs->private_data, "(%02x) *(%s *)(r%d %+d) = %d", insn->code, bpf_ldst_string[BPF_SIZE(insn->code) >> 3], insn->dst_reg, insn->off, insn->imm); } else if (BPF_MODE(insn->code) == 0xc0 /* BPF_NOSPEC, no UAPI */) { - verbose(cbs->private_data, "(%02x) nospec\n", insn->code); + verbose(cbs->private_data, "(%02x) nospec", insn->code); } else { - verbose(cbs->private_data, "BUG_st_%02x\n", insn->code); + verbose(cbs->private_data, "BUG_st_%02x", insn->code); } } else if (class == BPF_LDX) { if (BPF_MODE(insn->code) != BPF_MEM && BPF_MODE(insn->code) != BPF_MEMSX) { - verbose(cbs->private_data, "BUG_ldx_%02x\n", insn->code); + verbose(cbs->private_data, "BUG_ldx_%02x", insn->code); return; } - verbose(cbs->private_data, "(%02x) r%d = *(%s *)(r%d %+d)\n", + verbose(cbs->private_data, "(%02x) r%d = *(%s *)(r%d %+d)", insn->code, insn->dst_reg, BPF_MODE(insn->code) == BPF_MEM ? bpf_ldst_string[BPF_SIZE(insn->code) >> 3] : @@ -307,12 +307,12 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs, insn->src_reg, insn->off); } else if (class == BPF_LD) { if (BPF_MODE(insn->code) == BPF_ABS) { - verbose(cbs->private_data, "(%02x) r0 = *(%s *)skb[%d]\n", + verbose(cbs->private_data, "(%02x) r0 = *(%s *)skb[%d]", insn->code, bpf_ldst_string[BPF_SIZE(insn->code) >> 3], insn->imm); } else if (BPF_MODE(insn->code) == BPF_IND) { - verbose(cbs->private_data, "(%02x) r0 = *(%s *)skb[r%d + %d]\n", + verbose(cbs->private_data, "(%02x) r0 = *(%s *)skb[r%d + %d]", insn->code, bpf_ldst_string[BPF_SIZE(insn->code) >> 3], insn->src_reg, insn->imm); @@ -332,12 +332,12 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs, if (is_ptr && !allow_ptr_leaks) imm = 0; - verbose(cbs->private_data, "(%02x) r%d = %s\n", + verbose(cbs->private_data, "(%02x) r%d = %s", insn->code, insn->dst_reg, __func_imm_name(cbs, insn, imm, tmp, sizeof(tmp))); } else { - verbose(cbs->private_data, "BUG_ld_%02x\n", insn->code); + verbose(cbs->private_data, "BUG_ld_%02x", insn->code); return; } } else if (class == BPF_JMP32 || class == BPF_JMP) { @@ -347,35 +347,35 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs, char tmp[64]; if (insn->src_reg == BPF_PSEUDO_CALL) { - verbose(cbs->private_data, "(%02x) call pc%s\n", + verbose(cbs->private_data, "(%02x) call pc%s", insn->code, __func_get_name(cbs, insn, tmp, sizeof(tmp))); } else { strcpy(tmp, "unknown"); - verbose(cbs->private_data, "(%02x) call %s#%d\n", insn->code, + verbose(cbs->private_data, "(%02x) call %s#%d", insn->code, __func_get_name(cbs, insn, tmp, sizeof(tmp)), insn->imm); } } else if (insn->code == (BPF_JMP | BPF_JA)) { - verbose(cbs->private_data, "(%02x) goto pc%+d\n", + verbose(cbs->private_data, "(%02x) goto pc%+d", insn->code, insn->off); } else if (insn->code == (BPF_JMP | BPF_JA | BPF_X)) { - verbose(cbs->private_data, "(%02x) gotox r%d\n", + verbose(cbs->private_data, "(%02x) gotox r%d", insn->code, insn->dst_reg); } else if (insn->code == (BPF_JMP | BPF_JCOND) && insn->src_reg == BPF_MAY_GOTO) { - verbose(cbs->private_data, "(%02x) may_goto pc%+d\n", + verbose(cbs->private_data, "(%02x) may_goto pc%+d", insn->code, insn->off); } else if (insn->code == (BPF_JMP32 | BPF_JA)) { - verbose(cbs->private_data, "(%02x) gotol pc%+d\n", + verbose(cbs->private_data, "(%02x) gotol pc%+d", insn->code, insn->imm); } else if (insn->code == (BPF_JMP | BPF_EXIT)) { - verbose(cbs->private_data, "(%02x) exit\n", insn->code); + verbose(cbs->private_data, "(%02x) exit", insn->code); } else if (BPF_SRC(insn->code) == BPF_X) { verbose(cbs->private_data, - "(%02x) if %c%d %s %c%d goto pc%+d\n", + "(%02x) if %c%d %s %c%d goto pc%+d", insn->code, class == BPF_JMP32 ? 'w' : 'r', insn->dst_reg, bpf_jmp_string[BPF_OP(insn->code) >> 4], @@ -383,14 +383,14 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs, insn->src_reg, insn->off); } else { verbose(cbs->private_data, - "(%02x) if %c%d %s 0x%x goto pc%+d\n", + "(%02x) if %c%d %s 0x%x goto pc%+d", insn->code, class == BPF_JMP32 ? 'w' : 'r', insn->dst_reg, bpf_jmp_string[BPF_OP(insn->code) >> 4], (u32)insn->imm, insn->off); } } else { - verbose(cbs->private_data, "(%02x) %s\n", + verbose(cbs->private_data, "(%02x) %s", insn->code, bpf_class_string[class]); } } diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c index 0aadfbae0acc..ff1e68cc4bd1 100644 --- a/kernel/bpf/liveness.c +++ b/kernel/bpf/liveness.c @@ -497,7 +497,6 @@ static void print_instance(struct bpf_verifier_env *env, struct func_instance *i pos = env->log.end_pos; verbose(env, "%3d: ", insn_idx); bpf_verbose_insn(env, &insns[insn_idx]); - bpf_vlog_reset(&env->log, env->log.end_pos - 1); /* remove \n */ insn_pos = env->log.end_pos; verbose(env, "%*c;", bpf_vlog_alignment(insn_pos - pos), ' '); pos = env->log.end_pos; @@ -1043,7 +1042,6 @@ static void arg_track_log(struct bpf_verifier_env *env, struct bpf_insn *insn, i if (!printed) { verbose(env, "%3d: ", idx); bpf_verbose_insn(env, insn); - bpf_vlog_reset(&env->log, env->log.end_pos - 1); printed = true; } verbose(env, "\tr%d: ", i); verbose_arg_track(env, &at_in[i]); @@ -1058,7 +1056,6 @@ static void arg_track_log(struct bpf_verifier_env *env, struct bpf_insn *insn, i if (!printed) { verbose(env, "%3d: ", idx); bpf_verbose_insn(env, insn); - bpf_vlog_reset(&env->log, env->log.end_pos - 1); printed = true; } verbose(env, "\tsa%d: ", i); verbose_arg_track(env, &at_in[ai]); @@ -1070,7 +1067,6 @@ static void arg_track_log(struct bpf_verifier_env *env, struct bpf_insn *insn, i if (!printed) { verbose(env, "%3d: ", idx); bpf_verbose_insn(env, insn); - bpf_vlog_reset(&env->log, env->log.end_pos - 1); printed = true; } verbose(env, "\tfp%+d: ", -(i + 1) * 8); verbose_arg_track(env, &at_stack_in[i]); @@ -1545,6 +1541,7 @@ static void print_subprog_arg_access(struct bpf_verifier_env *env, verbose(env, "%3d: ", idx); bpf_verbose_insn(env, &insns[idx]); + verbose(env, "\n"); /* Collect what needs printing */ if (is_ldx_stx_call && @@ -2285,6 +2282,7 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env) verbose(env, "."); verbose(env, " "); bpf_verbose_insn(env, &insns[i]); + verbose(env, "\n"); if (bpf_is_ldimm64(&insns[i])) i++; } diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index e6f35f4e715b..6123e7e4a320 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -17475,6 +17475,7 @@ static int do_check(struct bpf_verifier_env *env) env->prev_log_pos = env->log.end_pos; verbose(env, "%d: ", env->insn_idx); bpf_verbose_insn(env, insn); + verbose(env, "\n"); env->prev_insn_print_pos = env->log.end_pos - env->prev_log_pos; env->prev_log_pos = env->log.end_pos; } diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c index 5e7cb8b36fef..5579173a61e3 100644 --- a/tools/bpf/bpftool/xlated_dumper.c +++ b/tools/bpf/bpftool/xlated_dumper.c @@ -107,14 +107,7 @@ print_insn_for_graph(void *private_data, const char *fmt, ...) p = buf; while (*p != '\0') { - if (*p == '\n') { - memmove(p + 3, p, strlen(buf) + 1 - (p - buf)); - /* Align each instruction dump row left. */ - *p++ = '\\'; - *p++ = 'l'; - /* Output multiline concatenation. */ - *p++ = '\\'; - } else if (*p == '<' || *p == '>' || *p == '|' || *p == '&') { + if (*p == '<' || *p == '>' || *p == '|' || *p == '&') { memmove(p + 1, p, strlen(buf) + 1 - (p - buf)); /* Escape special character. */ *p++ = '\\'; @@ -129,16 +122,10 @@ print_insn_for_graph(void *private_data, const char *fmt, ...) static void __printf(2, 3) print_insn_json(void *private_data, const char *fmt, ...) { - unsigned int l = strlen(fmt); - char chomped_fmt[l]; va_list args; va_start(args, fmt); - if (l > 0) { - strncpy(chomped_fmt, fmt, l - 1); - chomped_fmt[l - 1] = '\0'; - } - jsonw_vprintf_enquote(json_wtr, chomped_fmt, args); + jsonw_vprintf_enquote(json_wtr, fmt, args); va_end(args); } @@ -351,6 +338,7 @@ void dump_xlated_plain(struct dump_data *dd, void *buf, unsigned int len, printf("%4u: ", i); print_bpf_insn(&cbs, insn + i, true); + printf("\n"); if (opcodes) { printf(" "); @@ -417,6 +405,7 @@ void dump_xlated_for_graph(struct dump_data *dd, void *buf_start, void *buf_end, printf("%u: ", insn_off); print_bpf_insn(&cbs, cur, true); + printf("\\l\\\n"); if (opcodes) { printf("\\ \\ \\ \\ "); diff --git a/tools/testing/selftests/bpf/disasm_helpers.c b/tools/testing/selftests/bpf/disasm_helpers.c index f529f1c8c171..30221352568d 100644 --- a/tools/testing/selftests/bpf/disasm_helpers.c +++ b/tools/testing/selftests/bpf/disasm_helpers.c @@ -55,10 +55,9 @@ struct bpf_insn *disasm_insn(struct bpf_insn *insn, char *buf, size_t buf_sz) * for each instruction (FF stands for instruction `code` byte). * Remove the prefix inplace, and also simplify call instructions. * E.g.: "(85) call foo#10" -> "call foo". - * Also remove newline in the end (the 'max(strlen(buf) - 1, 0)' thing). */ pfx_end = buf + 5; - sfx_start = buf + max((int)strlen(buf) - 1, 0); + sfx_start = buf + (int)strlen(buf); if (strncmp(pfx_end, "call ", 5) == 0 && (tmp = strrchr(buf, '#'))) sfx_start = tmp; len = sfx_start - pfx_end; -- 2.55.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH bpf-next v2 2/5] bpf: track upper 32-bit register halves' liveness in compute_live_registers() 2026-08-01 1:09 [PATCH bpf-next v2 0/5] bpf: infer zext_dst based on static register liveness analysis Eduard Zingerman 2026-08-01 1:09 ` [PATCH bpf-next v2 1/5] bpf: do not print a newline after disassembly in bpf_verbose_insn() Eduard Zingerman @ 2026-08-01 1:09 ` Eduard Zingerman 2026-08-01 1:35 ` sashiko-bot 2026-08-01 1:09 ` [PATCH bpf-next v2 3/5] bpf: infer zext_dst based on static register liveness analysis Eduard Zingerman ` (2 subsequent siblings) 4 siblings, 1 reply; 13+ messages in thread From: Eduard Zingerman @ 2026-08-01 1:09 UTC (permalink / raw) To: bpf, ast, andrii Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87, memxor, iii, gimm78064 Extend compute_live_registers() to track upper and lower register halves' liveness separately. This is mostly straightforward: - use/def masks are extended to track 2 bits per register; - compute_insn_live_regs() is updated to properly track these 2 bits according to the instruction semantics. The only quirk is kfunc call processing logic, where we follow the verifier.c:check_kfunc_call() and verifier.c:mark_btf_func_reg_size() and infer whether the upper half of the parameter register is used by the call based on the parameter's BTF type size. Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> --- include/linux/bpf_verifier.h | 2 + kernel/bpf/liveness.c | 124 +++++++++++++++++++++++++++++++------------ kernel/bpf/verifier.c | 5 +- 3 files changed, 95 insertions(+), 36 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 682c2cd3b844..d4bffed94e0a 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -1050,6 +1050,8 @@ static inline struct bpf_subprog_info *subprog_info(struct bpf_verifier_env *env } struct bpf_call_summary { + const struct btf *btf; + const struct btf_type *func_proto; u8 num_params; bool is_void; bool fastcall; diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c index ff1e68cc4bd1..d0bd94c60fee 100644 --- a/kernel/bpf/liveness.c +++ b/kernel/bpf/liveness.c @@ -2047,29 +2047,67 @@ int bpf_compute_subprog_arg_access(struct bpf_verifier_env *env) /* Each field is a register bitmask */ struct insn_live_regs { - u16 use; /* registers read by instruction */ - u16 def; /* registers written by instruction */ - u16 in; /* registers that may be alive before instruction */ - u16 out; /* registers that may be alive after instruction */ + u32 use; /* registers read by instruction */ + u32 def; /* registers written by instruction */ + u32 in; /* registers that may be alive before instruction */ + u32 out; /* registers that may be alive after instruction */ }; /* Bitmask with 1s for all caller saved registers */ #define ALL_CALLER_SAVED_REGS ((1u << CALLER_SAVED_REGS) - 1) +static inline u32 reg32_mask(u32 n) { return BIT(n); } +static inline u32 reg64_mask(u32 n) { return BIT(n) | BIT(n + 16); } +static inline u32 mask_widen(u32 m) { return m | (m << 16); } +static inline u16 mask_lo(u32 m) { return (u16)m; } +static inline u16 mask_hi(u32 m) { return (u16)(m >> 16); } + +/* Infer use mask based on number and sizes of the function parameters. */ +static u32 call_use_mask(struct bpf_verifier_env *env, struct bpf_insn *insn) +{ + u32 i, use, size, reg_params_num; + struct bpf_call_summary cs; + const struct btf_param *p; + const struct btf_type *t; + + if (!bpf_get_call_summary(env, insn, &cs)) + /* e.g. a bpf-to-bpf call, assume that calls use full 64-bit for each param */ + return mask_widen(ALL_CALLER_SAVED_REGS & ~BIT(BPF_REG_0)); + + if (cs.func_proto) { + use = 0; + p = btf_params(cs.func_proto); + reg_params_num = min(btf_type_vlen(cs.func_proto), MAX_BPF_FUNC_REG_ARGS); + for (i = 0; i < reg_params_num; i++) { + t = btf_type_by_id(cs.btf, p[i].type); + if (IS_ERR(btf_resolve_size(cs.btf, t, &size))) + goto fallback; + use |= size <= sizeof(u32) ? reg32_mask(i + 1) : reg64_mask(i + 1); + } + return use; + } + +fallback: + /* by default assume that calls use full 64-bit for each param */ + return mask_widen(GENMASK(min_t(u8, cs.num_params, MAX_BPF_FUNC_REG_ARGS), 1)); +} + /* Compute info->{use,def} fields for the instruction */ static void compute_insn_live_regs(struct bpf_verifier_env *env, struct bpf_insn *insn, struct insn_live_regs *info) { - struct bpf_call_summary cs; - u8 class = BPF_CLASS(insn->code); - u8 code = BPF_OP(insn->code); - u8 mode = BPF_MODE(insn->code); - u16 src = BIT(insn->src_reg); - u16 dst = BIT(insn->dst_reg); - u16 r0 = BIT(0); - u16 def = 0; - u16 use = 0xffff; + const u8 class = BPF_CLASS(insn->code); + const u8 code = BPF_OP(insn->code); + const u8 mode = BPF_MODE(insn->code); + const u8 size = BPF_SIZE(insn->code); + const u32 src = reg64_mask(insn->src_reg); + const u32 dst = reg64_mask(insn->dst_reg); + const u32 src32 = mask_lo(src); + const u32 dst32 = mask_lo(dst); + const u32 r0 = reg64_mask(0); + u32 def = 0; + u32 use = U32_MAX; switch (class) { case BPF_LD: @@ -2080,8 +2118,8 @@ static void compute_insn_live_regs(struct bpf_verifier_env *env, use = 0; } break; - case BPF_LD | BPF_ABS: - case BPF_LD | BPF_IND: + case BPF_ABS: + case BPF_IND: /* stick with defaults */ break; } @@ -2089,7 +2127,15 @@ static void compute_insn_live_regs(struct bpf_verifier_env *env, case BPF_LDX: switch (mode) { case BPF_MEM: + /* a narrow load still redefines the whole register */ + def = dst; + use = src; + break; case BPF_MEMSX: + /* + * sign extension defines the whole register; + * src holds a pointer, hence is used as 64-bit. + */ def = dst; use = src; break; @@ -2107,12 +2153,19 @@ static void compute_insn_live_regs(struct bpf_verifier_env *env, switch (mode) { case BPF_MEM: def = 0; - use = dst | src; + use = dst | (size == BPF_DW ? src : src32); break; - case BPF_ATOMIC: + case BPF_ATOMIC: { + /* + * dst holds a pointer and is always used as 64-bit; + * the value operand and r0 are read as 32-bit for BPF_W atomics. + */ + u32 srcv = size == BPF_DW ? src : src32; + u32 r0v = size == BPF_DW ? r0 : mask_lo(r0); + switch (insn->imm) { case BPF_CMPXCHG: - use = r0 | dst | src; + use = r0v | dst | srcv; def = r0; break; case BPF_LOAD_ACQ: @@ -2121,10 +2174,10 @@ static void compute_insn_live_regs(struct bpf_verifier_env *env, break; case BPF_STORE_REL: def = 0; - use = dst | src; + use = dst | srcv; break; default: - use = dst | src; + use = dst | srcv; if (insn->imm & BPF_FETCH) def = src; else @@ -2132,6 +2185,7 @@ static void compute_insn_live_regs(struct bpf_verifier_env *env, } break; } + } break; case BPF_ALU: case BPF_ALU64: @@ -2145,14 +2199,14 @@ static void compute_insn_live_regs(struct bpf_verifier_env *env, if (BPF_SRC(insn->code) == BPF_K) use = 0; else - use = src; + use = class == BPF_ALU64 ? src : src32; break; default: def = dst; if (BPF_SRC(insn->code) == BPF_K) - use = dst; + use = class == BPF_ALU64 ? dst : dst32; else - use = dst | src; + use = class == BPF_ALU64 ? (dst | src) : (dst32 | src32); } break; case BPF_JMP: @@ -2174,17 +2228,14 @@ static void compute_insn_live_regs(struct bpf_verifier_env *env, use = r0; break; case BPF_CALL: - def = ALL_CALLER_SAVED_REGS; - use = def & ~BIT(BPF_REG_0); - if (bpf_get_call_summary(env, insn, &cs)) - use = GENMASK(min_t(u8, cs.num_params, MAX_BPF_FUNC_REG_ARGS), 1); + def = mask_widen(ALL_CALLER_SAVED_REGS); + use = call_use_mask(env, insn); break; default: def = 0; - if (BPF_SRC(insn->code) == BPF_K) - use = dst; - else - use = dst | src; + use = class == BPF_JMP ? dst : dst32; + if (BPF_SRC(insn->code) == BPF_X) + use |= class == BPF_JMP ? src : src32; } break; } @@ -2249,8 +2300,8 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env) int insn_idx = env->cfg.insn_postorder[i]; struct insn_live_regs *live = &state[insn_idx]; struct bpf_iarray *succ; - u16 new_out = 0; - u16 new_in = 0; + u32 new_out = 0; + u32 new_in = 0; succ = bpf_insn_successors(env, insn_idx); for (int s = 0; s < succ->cnt; ++s) @@ -2264,8 +2315,11 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env) } } - for (i = 0; i < insn_cnt; ++i) - insn_aux[i].live_regs_before = state[i].in; + for (i = 0; i < insn_cnt; ++i) { + u32 in = state[i].in; + + insn_aux[i].live_regs_before = mask_lo(in) | mask_hi(in); + } if (env->log.level & BPF_LOG_LEVEL2) { verbose(env, "Live regs before insn:\n"); diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 6123e7e4a320..8304452edc0f 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -16730,10 +16730,11 @@ bool bpf_get_call_summary(struct bpf_verifier_env *env, struct bpf_insn *call, int i; if (bpf_helper_call(call)) { - if (bpf_get_helper_proto(env, call->imm, &fn) < 0) /* error would be reported later */ return false; + cs->btf = NULL; + cs->func_proto = NULL; cs->fastcall = fn->allow_fastcall && (bpf_verifier_inlines_helper_call(env, call->imm) || bpf_jit_inlines_helper_call(call->imm)); @@ -16754,6 +16755,8 @@ bool bpf_get_call_summary(struct bpf_verifier_env *env, struct bpf_insn *call, if (err < 0) /* error would be reported later */ return false; + cs->btf = meta.btf; + cs->func_proto = meta.func_proto; cs->num_params = btf_type_vlen(meta.func_proto); cs->fastcall = meta.kfunc_flags & KF_FASTCALL; cs->is_void = btf_type_is_void(btf_type_by_id(meta.btf, meta.func_proto->type)); -- 2.55.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next v2 2/5] bpf: track upper 32-bit register halves' liveness in compute_live_registers() 2026-08-01 1:09 ` [PATCH bpf-next v2 2/5] bpf: track upper 32-bit register halves' liveness in compute_live_registers() Eduard Zingerman @ 2026-08-01 1:35 ` sashiko-bot 2026-08-01 4:40 ` Eduard Zingerman 0 siblings, 1 reply; 13+ messages in thread From: sashiko-bot @ 2026-08-01 1:35 UTC (permalink / raw) To: Eduard Zingerman; +Cc: bpf Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Skipping zero-extension for 32-bit kfunc arguments causes ABI violations on architectures that require the caller to extend 32-bit parameters. -- --- Patch [2]: [PATCH bpf-next v2 2/5] bpf: track upper 32-bit register halves' liveness in compute_live_registers() --- commit 2a034896228b7579bf93bf8ae2024cee100ebcba Author: Eduard Zingerman <eddyz87@gmail.com> bpf: track upper 32-bit register halves' liveness in compute_live_registers() This commit extends compute_live_registers() to track the upper and lower register halves' liveness separately. It updates compute_insn_live_regs() to track these bits and infers whether the upper half of a parameter register is used by a kfunc call based on its BTF type size. > diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c > index ff1e68cc4bd1c..d0bd94c60fee2 100644 > --- a/kernel/bpf/liveness.c > +++ b/kernel/bpf/liveness.c [ ... ] > +static u32 call_use_mask(struct bpf_verifier_env *env, struct bpf_insn *insn) > +{ > + u32 i, use, size, reg_params_num; > + struct bpf_call_summary cs; > + const struct btf_param *p; > + const struct btf_type *t; > + > + if (!bpf_get_call_summary(env, insn, &cs)) > + /* e.g. a bpf-to-bpf call, assume that calls use full 64-bit for each param */ > + return mask_widen(ALL_CALLER_SAVED_REGS & ~BIT(BPF_REG_0)); > + > + if (cs.func_proto) { > + use = 0; > + p = btf_params(cs.func_proto); > + reg_params_num = min(btf_type_vlen(cs.func_proto), MAX_BPF_FUNC_REG_ARGS); > + for (i = 0; i < reg_params_num; i++) { > + t = btf_type_by_id(cs.btf, p[i].type); > + if (IS_ERR(btf_resolve_size(cs.btf, t, &size))) > + goto fallback; > + use |= size <= sizeof(u32) ? reg32_mask(i + 1) : reg64_mask(i + 1); [Severity: High] Does this break the C ABI on architectures that require the caller to zero-extend or sign-extend 32-bit parameters passed in 64-bit registers? By only marking the lower 32 bits as used for smaller parameters with reg32_mask, the upper 32 bits are marked as dead before the kfunc call. Consequently, the verifier computes zext_dst = false for the instruction that previously defined the argument. This causes the JIT to skip inserting a BPF_ZEXT_REG instruction to clear the upper 32 bits on architectures where bpf_jit_needs_zext() is true, such as s390x and PowerPC64. If a BPF program computes a 32-bit value and passes it to a kfunc expecting a 32-bit parameter, the kfunc might operate on garbage data in the upper 32 bits. > + } > + return use; > + } -- Sashiko AI review · https://sashiko.dev/#/patchset/20260731-static-zext-v2-0-da4aa161e8c5@gmail.com?part=2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next v2 2/5] bpf: track upper 32-bit register halves' liveness in compute_live_registers() 2026-08-01 1:35 ` sashiko-bot @ 2026-08-01 4:40 ` Eduard Zingerman 0 siblings, 0 replies; 13+ messages in thread From: Eduard Zingerman @ 2026-08-01 4:40 UTC (permalink / raw) To: sashiko-reviews; +Cc: bpf On Sat, 2026-08-01 at 01:35 +0000, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [High] Skipping zero-extension for 32-bit kfunc arguments causes ABI violations on architectures that require the caller to extend 32-bit parameters. > -- > > --- Patch [2]: [PATCH bpf-next v2 2/5] bpf: track upper 32-bit register halves' liveness in compute_live_registers() --- > commit 2a034896228b7579bf93bf8ae2024cee100ebcba > Author: Eduard Zingerman <eddyz87@gmail.com> > > bpf: track upper 32-bit register halves' liveness in compute_live_registers() > > This commit extends compute_live_registers() to track the upper and lower > register halves' liveness separately. It updates compute_insn_live_regs() > to track these bits and infers whether the upper half of a parameter > register is used by a kfunc call based on its BTF type size. > > > diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c > > index ff1e68cc4bd1c..d0bd94c60fee2 100644 > > --- a/kernel/bpf/liveness.c > > +++ b/kernel/bpf/liveness.c > > [ ... ] > > > +static u32 call_use_mask(struct bpf_verifier_env *env, struct bpf_insn *insn) > > +{ > > + u32 i, use, size, reg_params_num; > > + struct bpf_call_summary cs; > > + const struct btf_param *p; > > + const struct btf_type *t; > > + > > + if (!bpf_get_call_summary(env, insn, &cs)) > > + /* e.g. a bpf-to-bpf call, assume that calls use full 64-bit for each param */ > > + return mask_widen(ALL_CALLER_SAVED_REGS & ~BIT(BPF_REG_0)); > > + > > + if (cs.func_proto) { > > + use = 0; > > + p = btf_params(cs.func_proto); > > + reg_params_num = min(btf_type_vlen(cs.func_proto), MAX_BPF_FUNC_REG_ARGS); > > + for (i = 0; i < reg_params_num; i++) { > > + t = btf_type_by_id(cs.btf, p[i].type); > > + if (IS_ERR(btf_resolve_size(cs.btf, t, &size))) > > + goto fallback; > > + use |= size <= sizeof(u32) ? reg32_mask(i + 1) : reg64_mask(i + 1); > > [Severity: High] > Does this break the C ABI on architectures that require the caller to > zero-extend or sign-extend 32-bit parameters passed in 64-bit registers? > > By only marking the lower 32 bits as used for smaller parameters with > reg32_mask, the upper 32 bits are marked as dead before the kfunc call. > Consequently, the verifier computes zext_dst = false for the instruction > that previously defined the argument. > > This causes the JIT to skip inserting a BPF_ZEXT_REG instruction to clear > the upper 32 bits on architectures where bpf_jit_needs_zext() is true, > such as s390x and PowerPC64. > > If a BPF program computes a 32-bit value and passes it to a kfunc expecting > a 32-bit parameter, the kfunc might operate on garbage data in the upper > 32 bits. Ok, I misread the original logic: static void mark_insn_zext(...) { ... env->insn_aux_data[def_idx - 1].zext_dst = true; ... } /* mark_btf_func_reg_size() is used when the reg size is determined by * the BTF func_proto's return value size and argument. */ static void __mark_btf_func_reg_size(...) { ... if (regno == BPF_REG_0) { /* Function return value */ ... } else if (reg_size == sizeof(u64)) { /* Function argument */ mark_insn_zext(env, reg); } } Meaning that the function parameters were always marked for zero extension. This is also confirmed by ELF Application Binary Interface s390x Supplement [1]: > ‹simple_arg›: A simple_arg is one of the following: > • One of the simple integer types no more than 64 bits wide. This includes > signed char, short, int, long, long long, _BitInt(𝑁) with 𝑁 ≤ 64, > their unsigned counterparts, _Bool, and any enum type. If such an argument > is shorter than 64 bits, replace it by a full 64-bit integer representing the > same number, using sign or zero extension, as appropriate. This is good news, as it makes changes to liveness tracking much simpler, there is no need to inspect BTF after all. [1] https://github.com/IBM/s390x-abi/blob/main/docs/lzsabi_s390x.pdf ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH bpf-next v2 3/5] bpf: infer zext_dst based on static register liveness analysis 2026-08-01 1:09 [PATCH bpf-next v2 0/5] bpf: infer zext_dst based on static register liveness analysis Eduard Zingerman 2026-08-01 1:09 ` [PATCH bpf-next v2 1/5] bpf: do not print a newline after disassembly in bpf_verbose_insn() Eduard Zingerman 2026-08-01 1:09 ` [PATCH bpf-next v2 2/5] bpf: track upper 32-bit register halves' liveness in compute_live_registers() Eduard Zingerman @ 2026-08-01 1:09 ` Eduard Zingerman 2026-08-01 1:42 ` sashiko-bot 2026-08-01 1:09 ` [PATCH bpf-next v2 4/5] bpf: simplify the bpf_is_reg64() Eduard Zingerman 2026-08-01 1:09 ` [PATCH bpf-next v2 5/5] selftests/bpf: verify zext_dst annotations for various instructions Eduard Zingerman 4 siblings, 1 reply; 13+ messages in thread From: Eduard Zingerman @ 2026-08-01 1:09 UTC (permalink / raw) To: bpf, ast, andrii Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87, memxor, iii, gimm78064 As reported in the thread [1], the verifier's 32-bit operations zero extension logic is broken. This logic is responsible for correct semantics of 32-bit operations on s390 architecture. According to BPF semantics, operation `w1 += 1` is supposed to zero extend the upper half of the register `r1`. On s390 the JIT relies on the verifier emitting explicit zero extension before such operations. The verifier attempts to minimize the amount of zero extensions inserted by tracking whether upper halves of the 64-bit registers are ever used. Previously such tracking worked as follows: - bpf_reg_state->subreg_def field was set by do_check_insn() for each operation defining lower but not the upper halves of the register. - Whenever an operation reading the whole register was verified, the verifier checked register's subreg_def and set bpf_insn_aux_data->zext_dst flag as true via a call to mark_insn_zext() function. - After the verification was complete, a special pass bpf_opt_subreg_zext_lo32_rnd_hi32() extended 32-bit operations with bpf_insn_aux_data->zext_dst set as true by adding explicit zero extension. Note that the logic above relies on bpf_reg_state->subreg_def, which is a property of a current verifier state. Before the commit [2] two additional steps happened: - The verifier tracked upper and lower register halves' liveness as flags REG_LIVE_READ{32,64} in bpf_reg_state->live. - The function propagate_liveness() called mark_insn_zext() in order to transfer the knowledge about which registers have their upper halves alive (and thus might require zero extension). The commit [2] removed the two steps described above, hence making possible a situation like below: - The register's upper half is set and is used on some verification path P1 and the register happens not to be marked as precise. - The checkpoint C is created while processing some instruction between register initialization and usage. - On some other verification path P2 the register's upper half is not initialized and that path ends hitting the checkpoint C. - In such a case the register's initialization on path P2 would lack zext_dst mark, making it possible for the program to inject an arbitrary value in the register's upper half. This commit replaces subreg_def based logic with computing zext_dst statically, as a part of the bpf_compute_live_registers() analysis: - The analysis now tracks usage of upper and lower halves of the registers separately. - If some instruction defines a 32-bit subregister, but not the whole register, *and* the upper half of the register is alive after that instruction, the instruction is marked as zext_dst. There is one notable drop in precision: whenever a BPF subprogram is called, all 64 bits of parameter registers are presumed to be used. The assumption is that such a drop in precision would not inflict a noticeable performance penalty. [1] https://lore.kernel.org/bpf/CAGKGUv=sOuqQtA1Ub-5JXfA4FPosJFYKAQE4B79cK+P1erxqtg@mail.gmail.com/ [2] commit 107e16979905 ("bpf: disable and remove registers chain based liveness") Fixes: 107e16979905 ("bpf: disable and remove registers chain based liveness") Reported-by: Min-gyu Kim <gimm78064@gmail.com> Link: https://lore.kernel.org/bpf/CAGKGUv=sOuqQtA1Ub-5JXfA4FPosJFYKAQE4B79cK+P1erxqtg@mail.gmail.com/ Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> --- include/linux/bpf_verifier.h | 6 +-- kernel/bpf/fixups.c | 17 ++++--- kernel/bpf/liveness.c | 14 ++++++ kernel/bpf/verifier.c | 105 +------------------------------------------ 4 files changed, 27 insertions(+), 115 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index d4bffed94e0a..66f506535cbd 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -162,11 +162,6 @@ struct bpf_reg_state { * pointing to bpf_func_state. */ u32 frameno; - /* Tracks subreg definition. The stored value is the insn_idx of the - * writing insn. This is safe because subreg_def is used before any insn - * patching which only happens after main verification finished. - */ - s32 subreg_def; /* if (!precise && SCALAR_VALUE) min/max/tnum don't affect safety */ bool precise; }; @@ -1657,5 +1652,6 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env); int bpf_jit_subprogs(struct bpf_verifier_env *env); int bpf_fixup_call_args(struct bpf_verifier_env *env); int bpf_do_misc_fixups(struct bpf_verifier_env *env); +int bpf_insn_def32(struct bpf_insn *insn); #endif /* _LINUX_BPF_VERIFIER_H */ diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c index a0bddada7964..6125598d16a8 100644 --- a/kernel/bpf/fixups.c +++ b/kernel/bpf/fixups.c @@ -44,15 +44,18 @@ static int insn_def_regno(const struct bpf_insn *insn) } } -/* Return TRUE if INSN has defined any 32-bit value explicitly. */ -static bool insn_has_def32(struct bpf_insn *insn) +/* + * Return the 32-bit subregister defined by INSN, or -1 if INSN does not + * explicitly define a 32-bit value. + */ +int bpf_insn_def32(struct bpf_insn *insn) { int dst_reg = insn_def_regno(insn); - if (dst_reg == -1) - return false; + if (dst_reg < 0 || bpf_is_reg64(insn, dst_reg, NULL, DST_OP)) + return -1; - return !bpf_is_reg64(insn, dst_reg, NULL, DST_OP); + return dst_reg; } static int kfunc_desc_cmp_by_imm_off(const void *a, const void *b) @@ -169,7 +172,7 @@ static void adjust_insn_aux_data(struct bpf_verifier_env *env, * (cnt == 1) is taken or not. There is no guarantee INSN at OFF is the * original insn at old prog. */ - data[off].zext_dst = insn_has_def32(insn + off + cnt - 1); + data[off].zext_dst = bpf_insn_def32(insn + off + cnt - 1) >= 0; if (cnt == 1) return; @@ -181,7 +184,7 @@ static void adjust_insn_aux_data(struct bpf_verifier_env *env, for (i = off; i < off + cnt - 1; i++) { /* Expand insni[off]'s seen count to the patched range. */ data[i].seen = old_seen; - data[i].zext_dst = insn_has_def32(insn + i); + data[i].zext_dst = bpf_insn_def32(insn + i) >= 0; } /* diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c index d0bd94c60fee..77b84b367be7 100644 --- a/kernel/bpf/liveness.c +++ b/kernel/bpf/liveness.c @@ -2257,6 +2257,7 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env) struct bpf_insn *insns = env->prog->insnsi; struct insn_live_regs *state; int insn_cnt = env->prog->len; + u64 pos, insn_pos; int err = 0, i, j; bool changed; @@ -2316,9 +2317,18 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env) } for (i = 0; i < insn_cnt; ++i) { + int def32 = bpf_insn_def32(&insns[i]); + u32 out = state[i].out; u32 in = state[i].in; insn_aux[i].live_regs_before = mask_lo(in) | mask_hi(in); + /* + * On architectures where 32-bit operations do not reset upper halves + * of the registers, the verifier needs to zero extend a destination + * register if an instruction defines a 32-bit subregister and the + * upper half of that register is alive after the instruction. + */ + insn_aux[i].zext_dst = def32 >= 0 && (mask_hi(out) & BIT(def32)); } if (env->log.level & BPF_LOG_LEVEL2) { @@ -2335,7 +2345,11 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env) else verbose(env, "."); verbose(env, " "); + pos = env->log.end_pos; bpf_verbose_insn(env, &insns[i]); + insn_pos = env->log.end_pos; + if (insn_aux[i].zext_dst) + verbose(env, "%*c; zext", bpf_vlog_alignment(insn_pos - pos), ' '); verbose(env, "\n"); if (bpf_is_ldimm64(&insns[i])) i++; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 8304452edc0f..0709dded013e 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -2129,12 +2129,9 @@ static int reg_bounds_sanity_check(struct bpf_verifier_env *env, /* Mark a register as having a completely unknown (scalar) value. */ void bpf_mark_reg_unknown_imprecise(struct bpf_reg_state *reg) { - s32 subreg_def = reg->subreg_def; - memset(reg, 0, sizeof(*reg)); reg->type = SCALAR_VALUE; reg->var_off = tnum_unknown; - reg->subreg_def = subreg_def; __mark_reg_unbounded(reg); } @@ -2210,7 +2207,6 @@ static int mark_btf_ld_reg(struct bpf_verifier_env *env, } } -#define DEF_NOT_SUBREG (0) static void init_reg_state(struct bpf_verifier_env *env, struct bpf_func_state *state) { @@ -2219,7 +2215,6 @@ static void init_reg_state(struct bpf_verifier_env *env, for (i = 0; i < MAX_BPF_REG; i++) { bpf_mark_reg_not_init(env, ®s[i]); - regs[i].subreg_def = DEF_NOT_SUBREG; } /* frame pointer */ @@ -3122,30 +3117,14 @@ bool bpf_is_reg64(struct bpf_insn *insn, return true; } -static void mark_insn_zext(struct bpf_verifier_env *env, - struct bpf_reg_state *reg) -{ - s32 def_idx = reg->subreg_def; - - if (def_idx == DEF_NOT_SUBREG) - return; - - env->insn_aux_data[def_idx - 1].zext_dst = true; - /* The dst will be zero extended, so won't be sub-register anymore. */ - reg->subreg_def = DEF_NOT_SUBREG; -} - static int __check_reg_arg(struct bpf_verifier_env *env, struct bpf_reg_state *regs, u32 regno, enum bpf_reg_arg_type t) { - struct bpf_insn *insn = env->prog->insnsi + env->insn_idx; struct bpf_reg_state *reg; - bool rw64; mark_reg_scratched(env, regno); reg = ®s[regno]; - rw64 = bpf_is_reg64(insn, regno, reg, t); if (t == SRC_OP) { /* check whether register used as source operand can be read */ if (reg->type == NOT_INIT) { @@ -3156,9 +3135,6 @@ static int __check_reg_arg(struct bpf_verifier_env *env, struct bpf_reg_state *r if (regno == BPF_REG_FP) return 0; - if (rw64) - mark_insn_zext(env, reg); - return 0; } else { /* check whether register used as dest operand can be written to */ @@ -3166,7 +3142,6 @@ static int __check_reg_arg(struct bpf_verifier_env *env, struct bpf_reg_state *r verbose(env, "frame pointer is read only\n"); return -EACCES; } - reg->subreg_def = rw64 ? DEF_NOT_SUBREG : env->insn_idx + 1; if (t == DST_OP) mark_reg_unknown(env, regs, regno); } @@ -3822,11 +3797,6 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env, if (size <= spill_size && bpf_stack_narrow_access_ok(off, size, spill_size)) { - /* The earlier check_reg_arg() has decided the - * subreg_def for this insn. Save it first. - */ - s32 subreg_def = state->regs[dst_regno].subreg_def; - if (env->bpf_capable && size == 4 && spill_size == 4 && get_reg_width(reg) <= 32) /* Ensure stack slot has an ID to build a relation @@ -3834,7 +3804,6 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env, */ assign_scalar_id_before_mov(env, reg); state->regs[dst_regno] = *reg; - state->regs[dst_regno].subreg_def = subreg_def; /* Break the relation on a narrowing fill. * coerce_reg_to_size will adjust the boundaries. @@ -6271,12 +6240,6 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b } else { mark_reg_known_zero(env, regs, value_regno); - /* A load of ctx field could have different - * actual load size with the one encoded in the - * insn. When the dst is PTR, it is for sure not - * a sub-register. - */ - regs[value_regno].subreg_def = DEF_NOT_SUBREG; if (base_type(info.reg_type) == PTR_TO_BTF_ID) { regs[value_regno].btf = info.btf; regs[value_regno].btf_id = info.btf_id; @@ -7380,10 +7343,6 @@ static int process_dynptr_func(struct bpf_verifier_env *env, struct bpf_reg_stat if (spi < 0) return spi; - /* - * For CONST_PTR_TO_DYNPTR, reg is already scratched by check_reg_arg - * in check_helper_call and mark_btf_func_reg_size in check_kfunc_call. - */ mark_stack_slots_scratched(env, spi, BPF_DYNPTR_NR_SLOTS); reg = &state->stack[spi].spilled_ptr; @@ -9463,7 +9422,6 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, /* All non-void global functions return a 64-bit SCALAR_VALUE. */ if (!subprog_returns_void(env, subprog)) { mark_reg_unknown(env, caller->regs, BPF_REG_0); - caller->regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG; } if (env->subprog_info[subprog].might_throw) { @@ -10482,9 +10440,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn } invalidate_outgoing_stack_args(env, cur_func(env)); - /* helper call returns 64-bit value. */ - regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG; - /* update return register (already marked as written above) */ ret_type = fn->ret_type; ret_flag = type_flag(ret_type); @@ -10722,30 +10677,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn return 0; } -/* mark_btf_func_reg_size() is used when the reg size is determined by - * the BTF func_proto's return value size and argument. - */ -static void __mark_btf_func_reg_size(struct bpf_verifier_env *env, struct bpf_reg_state *regs, - u32 regno, size_t reg_size) -{ - struct bpf_reg_state *reg = ®s[regno]; - - if (regno == BPF_REG_0) { - /* Function return value */ - reg->subreg_def = reg_size == sizeof(u64) ? - DEF_NOT_SUBREG : env->insn_idx + 1; - } else if (reg_size == sizeof(u64)) { - /* Function argument */ - mark_insn_zext(env, reg); - } -} - -static void mark_btf_func_reg_size(struct bpf_verifier_env *env, u32 regno, - size_t reg_size) -{ - return __mark_btf_func_reg_size(env, cur_regs(env), regno, reg_size); -} - static bool is_kfunc_acquire(struct bpf_call_arg_meta *meta) { return meta->kfunc_flags & KF_ACQUIRE; @@ -12974,7 +12905,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, struct bpf_call_arg_meta meta; struct bpf_insn_aux_data *insn_aux; int err, insn_idx = *insn_idx_p; - const struct btf_param *args; u32 i, nargs, ptr_type_id; struct btf *desc_btf; int id; @@ -13018,7 +12948,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, verbose(env, "failed to mark s32 range for retval in forked state for lock\n"); return err; } - __mark_btf_func_reg_size(env, regs, BPF_REG_0, sizeof(u32)); } else if (!insn->off && insn->imm == special_kfunc_list[KF___bpf_trap]) { verbose(env, "unexpected __bpf_trap() due to uninitialized variable?\n"); return -EFAULT; @@ -13168,7 +13097,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, u32 regno = caller_saved[i]; bpf_mark_reg_not_init(env, ®s[regno]); - regs[regno].subreg_def = DEF_NOT_SUBREG; } invalidate_outgoing_stack_args(env, cur_func(env)); @@ -13190,7 +13118,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, if (meta.btf == btf_vmlinux && (meta.func_id == special_kfunc_list[KF_bpf_res_spin_lock] || meta.func_id == special_kfunc_list[KF_bpf_res_spin_lock_irqsave])) __mark_reg_const_zero(env, ®s[BPF_REG_0]); - mark_btf_func_reg_size(env, BPF_REG_0, t->size); } else if (btf_type_is_ptr(t)) { ptr_type = btf_type_skip_modifiers(desc_btf, t->type, &ptr_type_id); err = check_special_kfunc(env, &meta, regs, insn_aux, ptr_type, desc_btf); @@ -13281,7 +13208,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, /* For mark_ptr_or_null_reg, see 93c230e3f5bd6 */ regs[BPF_REG_0].id = ++env->id_gen; } - mark_btf_func_reg_size(env, BPF_REG_0, sizeof(void *)); if (is_kfunc_acquire(&meta)) { id = acquire_reference(env, insn_idx, 0); if (id < 0) @@ -13318,18 +13244,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, caller_info->stack_arg_cnt = stack_arg_cnt; } - args = (const struct btf_param *)(meta.func_proto + 1); - for (i = 0; i < min_t(int, nargs, MAX_BPF_FUNC_REG_ARGS); i++) { - u32 regno = i + 1; - - t = btf_type_skip_modifiers(desc_btf, args[i].type, NULL); - if (btf_type_is_ptr(t)) - mark_btf_func_reg_size(env, regno, sizeof(void *)); - else - /* scalar. ensured by check_kfunc_args() */ - mark_btf_func_reg_size(env, regno, t->size); - } - if (bpf_is_iter_next_kfunc(&meta)) { err = process_iter_next_call(env, insn_idx, &meta); if (err) @@ -14823,8 +14737,6 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env, if (dst_reg->type != PTR_TO_ARENA) *dst_reg = *src_reg; - dst_reg->subreg_def = env->insn_idx + 1; - if (BPF_CLASS(insn->code) == BPF_ALU64) /* * 32-bit operations zero upper bits automatically. @@ -15028,8 +14940,6 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) mark_reg_unknown(env, regs, insn->dst_reg); if (insn->imm == 1) { /* cast from as(1) to as(0) */ dst_reg->type = PTR_TO_ARENA; - /* PTR_TO_ARENA is 32-bit */ - dst_reg->subreg_def = env->insn_idx + 1; } } else if (insn->off == 0) { /* case: R1 = R2 @@ -15037,7 +14947,6 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) */ assign_scalar_id_before_mov(env, src_reg); *dst_reg = *src_reg; - dst_reg->subreg_def = DEF_NOT_SUBREG; } else { /* case: R1 = (s8, s16 s32)R2 */ if (is_pointer_value(env, insn->src_reg)) { @@ -15055,7 +14964,6 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) if (!no_sext) clear_scalar_id(dst_reg); coerce_reg_to_size_sx(dst_reg, insn->off >> 3); - dst_reg->subreg_def = DEF_NOT_SUBREG; } else { mark_reg_unknown(env, regs, insn->dst_reg); } @@ -15080,7 +14988,6 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) */ if (!is_src_reg_u32) clear_scalar_id(dst_reg); - dst_reg->subreg_def = env->insn_idx + 1; } else { /* case: W1 = (s8, s16)W2 */ bool no_sext = reg_umax(src_reg) < (1ULL << (insn->off - 1)); @@ -15090,7 +14997,6 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) *dst_reg = *src_reg; if (!no_sext) clear_scalar_id(dst_reg); - dst_reg->subreg_def = env->insn_idx + 1; coerce_subreg_to_size_sx(dst_reg, insn->off >> 3); } } else { @@ -15959,12 +15865,8 @@ static void sync_linked_regs(struct bpf_verifier_env *env, struct bpf_verifier_s continue; if ((!(reg->id & BPF_ADD_CONST) && !(known_reg->id & BPF_ADD_CONST)) || reg->delta == known_reg->delta) { - s32 saved_subreg_def = reg->subreg_def; - *reg = *known_reg; - reg->subreg_def = saved_subreg_def; } else { - s32 saved_subreg_def = reg->subreg_def; s32 saved_off = reg->delta; u32 saved_id = reg->id; @@ -15974,12 +15876,11 @@ static void sync_linked_regs(struct bpf_verifier_env *env, struct bpf_verifier_s /* reg = known_reg; reg += delta */ *reg = *known_reg; /* - * Must preserve off, id and subreg_def flag, - * otherwise another sync_linked_regs() will be incorrect. + * Must preserve off and id, otherwise another sync_linked_regs() + * will be incorrect. */ reg->delta = saved_off; reg->id = saved_id; - reg->subreg_def = saved_subreg_def; scalar32_min_max_add(reg, &fake_reg); scalar_min_max_add(reg, &fake_reg); @@ -16414,8 +16315,6 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn) * Already marked as written above. */ mark_reg_unknown(env, regs, BPF_REG_0); - /* ld_abs load up to 32-bit skb data. */ - regs[BPF_REG_0].subreg_def = env->insn_idx + 1; /* * See bpf_gen_ld_abs() which emits a hidden BPF_EXIT with r0=0 * which must be explored by the verifier when in a subprog. -- 2.55.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next v2 3/5] bpf: infer zext_dst based on static register liveness analysis 2026-08-01 1:09 ` [PATCH bpf-next v2 3/5] bpf: infer zext_dst based on static register liveness analysis Eduard Zingerman @ 2026-08-01 1:42 ` sashiko-bot 2026-08-01 5:41 ` Eduard Zingerman 0 siblings, 1 reply; 13+ messages in thread From: sashiko-bot @ 2026-08-01 1:42 UTC (permalink / raw) To: Eduard Zingerman; +Cc: bpf Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: New issues: - [High] Dynamically downgraded 64-bit arena operations and pointer casts are missing explicit zero-extension on architectures requiring it. Pre-existing issues: - [High] Kfuncs bpf_mem_cgroup_memory_events and bpf_mem_cgroup_vm_events do not properly validate negative enum values, leading to out-of-bounds array reads. -- --- Patch [3]: [PATCH bpf-next v2 3/5] bpf: infer zext_dst based on static register liveness analysis --- commit 625c020066cfbb5a384d20ff5e1ad79faa898949 Author: Eduard Zingerman <eddyz87@gmail.com> bpf: infer zext_dst based on static register liveness analysis This commit replaces the verifier's subreg_def based logic with computing zext_dst statically as part of bpf_compute_live_registers(). It tracks the liveness of the upper and lower halves of registers separately to emit explicit zero extensions on architectures that need them. Link: https://lore.kernel.org/bpf/CAGKGUv=sOuqQtA1Ub-5JXfA4FPosJFYKAQE4B79cK+P1erxqtg@mail.gmail.com/ [ ... ] > diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c > index d0bd94c60fee2..77b84b367be74 100644 > --- a/kernel/bpf/liveness.c > +++ b/kernel/bpf/liveness.c > @@ -2316,9 +2317,18 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env) > } > > for (i = 0; i < insn_cnt; ++i) { > + int def32 = bpf_insn_def32(&insns[i]); > + u32 out = state[i].out; > u32 in = state[i].in; > > insn_aux[i].live_regs_before = mask_lo(in) | mask_hi(in); > + /* > + * On architectures where 32-bit operations do not reset upper halves > + * of the registers, the verifier needs to zero extend a destination > + * register if an instruction defines a 32-bit subregister and the > + * upper half of that register is alive after the instruction. > + */ > + insn_aux[i].zext_dst = def32 >= 0 && (mask_hi(out) & BIT(def32)); > } [Severity: High] Does this statically evaluate zext_dst before some instructions are dynamically downgraded? If an ALU64 operation with a PTR_TO_ARENA operand is statically evaluated here, zext_dst evaluates to false because it is still a 64-bit instruction. Later during verification, adjust_reg_min_max_vals() sets needs_zext = true. Then bpf_do_misc_fixups() downgrades the instruction in-place to a 32-bit ALU operation: kernel/bpf/fixups.c:bpf_do_misc_fixups() { ... if (env->insn_aux_data[i + delta].needs_zext) /* Convert BPF_CLASS(insn->code) == BPF_ALU64 to 32-bit ALU */ insn->code = BPF_ALU | BPF_OP(insn->code) | BPF_SRC(insn->code); ... } Since this bypasses bpf_patch_insn_data(), zext_dst remains false. Could this cause bpf_opt_subreg_zext_lo32_rnd_hi32() to skip injecting the required zero extension, leaving garbage in the upper 32 bits on architectures like s390x and potentially leading to out-of-bounds accesses? [ ... ] [Severity: High] This is a pre-existing issue, but I noticed a potential out-of-bounds array read in some memcg kfuncs due to missing negative value checks on enums. In mm/bpf_memcontrol.c:bpf_mem_cgroup_memory_events(): if (unlikely(event >= MEMCG_NR_MEMORY_EVENTS)) return (unsigned long)-1; return atomic_long_read(&memcg->memory_events[event]); Because the BPF verifier treats the event enum as a signed integer and doesn't check enum ranges, can a BPF program pass a negative value to bypass this check (e.g., -1 >= MEMCG_NR_MEMORY_EVENTS evaluates to false) and cause an out-of-bounds read? Similarly in mm/memcontrol.c:memcg_vm_event_item_valid(): if (idx >= NR_VM_EVENT_ITEMS) return false; return !BAD_STAT_IDX(memcg_events_index(idx)); Could a negative idx bypass this bounds check and cause an out-of-bounds access inside memcg_events_index()? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260731-static-zext-v2-0-da4aa161e8c5@gmail.com?part=3 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next v2 3/5] bpf: infer zext_dst based on static register liveness analysis 2026-08-01 1:42 ` sashiko-bot @ 2026-08-01 5:41 ` Eduard Zingerman 0 siblings, 0 replies; 13+ messages in thread From: Eduard Zingerman @ 2026-08-01 5:41 UTC (permalink / raw) To: sashiko-reviews; +Cc: bpf On Sat, 2026-08-01 at 01:42 +0000, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: > > New issues: > - [High] Dynamically downgraded 64-bit arena operations and pointer casts are missing explicit zero-extension on architectures requiring it. > > Pre-existing issues: > - [High] Kfuncs bpf_mem_cgroup_memory_events and bpf_mem_cgroup_vm_events do not properly validate negative enum values, leading to out-of-bounds array reads. > -- > > --- Patch [3]: [PATCH bpf-next v2 3/5] bpf: infer zext_dst based on static register liveness analysis --- > commit 625c020066cfbb5a384d20ff5e1ad79faa898949 > Author: Eduard Zingerman <eddyz87@gmail.com> > > bpf: infer zext_dst based on static register liveness analysis > > This commit replaces the verifier's subreg_def based logic with computing > zext_dst statically as part of bpf_compute_live_registers(). It tracks the > liveness of the upper and lower halves of registers separately to emit explicit > zero extensions on architectures that need them. > > Link: https://lore.kernel.org/bpf/CAGKGUv=sOuqQtA1Ub-5JXfA4FPosJFYKAQE4B79cK+P1erxqtg@mail.gmail.com/ > > [ ... ] > > > diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c > > index d0bd94c60fee2..77b84b367be74 100644 > > --- a/kernel/bpf/liveness.c > > +++ b/kernel/bpf/liveness.c > > @@ -2316,9 +2317,18 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env) > > } > > > > for (i = 0; i < insn_cnt; ++i) { > > + int def32 = bpf_insn_def32(&insns[i]); > > + u32 out = state[i].out; > > u32 in = state[i].in; > > > > insn_aux[i].live_regs_before = mask_lo(in) | mask_hi(in); > > + /* > > + * On architectures where 32-bit operations do not reset upper halves > > + * of the registers, the verifier needs to zero extend a destination > > + * register if an instruction defines a 32-bit subregister and the > > + * upper half of that register is alive after the instruction. > > + */ > > + insn_aux[i].zext_dst = def32 >= 0 && (mask_hi(out) & BIT(def32)); > > } > > [Severity: High] > Does this statically evaluate zext_dst before some instructions are dynamically > downgraded? > > If an ALU64 operation with a PTR_TO_ARENA operand is statically evaluated > here, zext_dst evaluates to false because it is still a 64-bit instruction. > > Later during verification, adjust_reg_min_max_vals() sets needs_zext = true. > Then bpf_do_misc_fixups() downgrades the instruction in-place to a 32-bit ALU > operation: > > kernel/bpf/fixups.c:bpf_do_misc_fixups() { > ... > if (env->insn_aux_data[i + delta].needs_zext) > /* Convert BPF_CLASS(insn->code) == BPF_ALU64 to 32-bit ALU */ > insn->code = BPF_ALU | BPF_OP(insn->code) | BPF_SRC(insn->code); > ... > } > > Since this bypasses bpf_patch_insn_data(), zext_dst remains false. Could this > cause bpf_opt_subreg_zext_lo32_rnd_hi32() to skip injecting the required zero > extension, leaving garbage in the upper 32 bits on architectures like s390x > and potentially leading to out-of-bounds accesses? Yeap, need to figure something out. ... ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH bpf-next v2 4/5] bpf: simplify the bpf_is_reg64() 2026-08-01 1:09 [PATCH bpf-next v2 0/5] bpf: infer zext_dst based on static register liveness analysis Eduard Zingerman ` (2 preceding siblings ...) 2026-08-01 1:09 ` [PATCH bpf-next v2 3/5] bpf: infer zext_dst based on static register liveness analysis Eduard Zingerman @ 2026-08-01 1:09 ` Eduard Zingerman 2026-08-01 1:34 ` sashiko-bot 2026-08-01 1:09 ` [PATCH bpf-next v2 5/5] selftests/bpf: verify zext_dst annotations for various instructions Eduard Zingerman 4 siblings, 1 reply; 13+ messages in thread From: Eduard Zingerman @ 2026-08-01 1:09 UTC (permalink / raw) To: bpf, ast, andrii Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87, memxor, iii, gimm78064 After the previous commit bpf_is_reg64() is only used in a context where destination register's property is queried, and only for instructions for which insn_def_regno() >= 0. Hence, simplify the function by: - removing unused parameters; - removing code paths considering BPF_JMP{,32} instructions; - streamlining the condition expressions. Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> --- include/linux/bpf_verifier.h | 1 - kernel/bpf/fixups.c | 40 +++++++++++++++++++- kernel/bpf/verifier.c | 90 -------------------------------------------- 3 files changed, 38 insertions(+), 93 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 66f506535cbd..2dbbd51cb923 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -1628,7 +1628,6 @@ struct bpf_kfunc_desc_tab { }; /* Functions exported from verifier.c, used by fixups.c */ -bool bpf_is_reg64(struct bpf_insn *insn, u32 regno, struct bpf_reg_state *reg, enum bpf_reg_arg_type t); void bpf_clear_insn_aux_data(struct bpf_verifier_env *env, int start, int len); void bpf_mark_subprog_exc_cb(struct bpf_verifier_env *env, int subprog); bool bpf_allow_tail_call_in_subprogs(struct bpf_verifier_env *env); diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c index 6125598d16a8..ac0a388b4021 100644 --- a/kernel/bpf/fixups.c +++ b/kernel/bpf/fixups.c @@ -44,6 +44,42 @@ static int insn_def_regno(const struct bpf_insn *insn) } } +/* + * For use only in combination with insn_def_regno() >= 0. + * Returns TRUE if the destination register operates on 64-bit, + * otherwise return FALSE. + */ +static bool bpf_is_reg64(struct bpf_insn *insn) +{ + u8 class = BPF_CLASS(insn->code); + u8 mode = BPF_MODE(insn->code); + u8 size = BPF_SIZE(insn->code); + u8 op = BPF_OP(insn->code); + + /* subregister endiness swap */ + if ((class == BPF_ALU || class == BPF_ALU64) && op == BPF_END && insn->imm != 64) + return false; + + /* w0 += 1 */ + if (class == BPF_ALU && op != BPF_END) + return false; + + /* non 64-bit loads */ + if (class == BPF_LDX && mode == BPF_MEM && size != BPF_DW) + return false; + + /* atomics, see insn_def_regno() */ + if (class == BPF_STX && size != BPF_DW) + return false; + + /* both LD_IND and LD_ABS return 32-bit data. */ + if (class == BPF_LD && (mode == BPF_IND || mode == BPF_ABS)) + return false; + + /* Conservatively return true at default. */ + return true; +} + /* * Return the 32-bit subregister defined by INSN, or -1 if INSN does not * explicitly define a 32-bit value. @@ -52,7 +88,7 @@ int bpf_insn_def32(struct bpf_insn *insn) { int dst_reg = insn_def_regno(insn); - if (dst_reg < 0 || bpf_is_reg64(insn, dst_reg, NULL, DST_OP)) + if (dst_reg < 0 || bpf_is_reg64(insn)) return -1; return dst_reg; @@ -623,7 +659,7 @@ int bpf_opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env, * BPF_STX + SRC_OP, so it is safe to pass NULL * here. */ - if (bpf_is_reg64(&insn, load_reg, NULL, DST_OP)) { + if (bpf_is_reg64(&insn)) { if (class == BPF_LD && BPF_MODE(code) == BPF_IMM) i++; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 0709dded013e..c3cddaec953b 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -3027,96 +3027,6 @@ static void mark_stack_slots_scratched(struct bpf_verifier_env *env, mark_stack_slot_scratched(env, spi - i); } -/* This function is supposed to be used by the following 32-bit optimization - * code only. It returns TRUE if the source or destination register operates - * on 64-bit, otherwise return FALSE. - */ -bool bpf_is_reg64(struct bpf_insn *insn, - u32 regno, struct bpf_reg_state *reg, enum bpf_reg_arg_type t) -{ - u8 code, class, op; - - code = insn->code; - class = BPF_CLASS(code); - op = BPF_OP(code); - if (class == BPF_JMP) { - /* BPF_EXIT for "main" will reach here. Return TRUE - * conservatively. - */ - if (op == BPF_EXIT) - return true; - if (op == BPF_CALL) { - /* BPF to BPF call will reach here because of marking - * caller saved clobber with DST_OP_NO_MARK for which we - * don't care the register def because they are anyway - * marked as NOT_INIT already. - */ - if (insn->src_reg == BPF_PSEUDO_CALL) - return false; - /* Helper call will reach here because of arg type - * check, conservatively return TRUE. - */ - if (t == SRC_OP) - return true; - - return false; - } - } - - if (class == BPF_ALU64 && op == BPF_END && (insn->imm == 16 || insn->imm == 32)) - return false; - - if (class == BPF_ALU64 || class == BPF_JMP || - (class == BPF_ALU && op == BPF_END && insn->imm == 64)) - return true; - - if (class == BPF_ALU || class == BPF_JMP32) - return false; - - if (class == BPF_LDX) { - if (t != SRC_OP) - return BPF_SIZE(code) == BPF_DW || BPF_MODE(code) == BPF_MEMSX; - /* LDX source must be ptr. */ - return true; - } - - if (class == BPF_STX) { - /* BPF_STX (including atomic variants) has one or more source - * operands, one of which is a ptr. Check whether the caller is - * asking about it. - */ - if (t == SRC_OP && reg->type != SCALAR_VALUE) - return true; - return BPF_SIZE(code) == BPF_DW; - } - - if (class == BPF_LD) { - u8 mode = BPF_MODE(code); - - /* LD_IMM64 */ - if (mode == BPF_IMM) - return true; - - /* Both LD_IND and LD_ABS return 32-bit data. */ - if (t != SRC_OP) - return false; - - /* Implicit ctx ptr. */ - if (regno == BPF_REG_6) - return true; - - /* Explicit source could be any width. */ - return true; - } - - if (class == BPF_ST) - /* The only source register for BPF_ST is a ptr. */ - return true; - - /* Conservatively return true at default. */ - return true; -} - static int __check_reg_arg(struct bpf_verifier_env *env, struct bpf_reg_state *regs, u32 regno, enum bpf_reg_arg_type t) { -- 2.55.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next v2 4/5] bpf: simplify the bpf_is_reg64() 2026-08-01 1:09 ` [PATCH bpf-next v2 4/5] bpf: simplify the bpf_is_reg64() Eduard Zingerman @ 2026-08-01 1:34 ` sashiko-bot 0 siblings, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-08-01 1:34 UTC (permalink / raw) To: Eduard Zingerman; +Cc: bpf Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] `bpf_is_reg64()` incorrectly identifies 32-bit `BPF_PROBE_MEM` and `BPF_PROBE_MEM32` loads as 64-bit defining instructions, bypassing explicit zero-extensions. - [Low] Stale comment referencing a deleted parameter of `bpf_is_reg64`. -- --- Patch [4]: [PATCH bpf-next v2 4/5] bpf: simplify the bpf_is_reg64() --- commit ee97fd37754941b6dd81dccf5f68e7f82c74ea6d Author: Eduard Zingerman <eddyz87@gmail.com> bpf: simplify the bpf_is_reg64() This patch simplifies the bpf_is_reg64() function by removing unused parameters and code paths related to BPF_JMP instructions. It also streamlines the condition expressions for identifying 64-bit registers. > diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c > index 6125598d16a84..ac0a388b40218 100644 > --- a/kernel/bpf/fixups.c > +++ b/kernel/bpf/fixups.c [ ... ] > @@ -44,6 +44,42 @@ static int insn_def_regno(const struct bpf_insn *insn) > } > } > > +/* > + * For use only in combination with insn_def_regno() >= 0. > + * Returns TRUE if the destination register operates on 64-bit, > + * otherwise return FALSE. > + */ > +static bool bpf_is_reg64(struct bpf_insn *insn) > +{ > + u8 class = BPF_CLASS(insn->code); > + u8 mode = BPF_MODE(insn->code); > + u8 size = BPF_SIZE(insn->code); > + u8 op = BPF_OP(insn->code); [ ... ] > + /* non 64-bit loads */ > + if (class == BPF_LDX && mode == BPF_MEM && size != BPF_DW) > + return false; [Severity: High] Does this condition miss BPF_PROBE_MEM and BPF_PROBE_MEM32 loads? Because this check strictly matches the BPF_MEM mode, it seems these other 32-bit load modes would bypass this check and fall through to returning true. Could this cause bpf_insn_def32() to inappropriately return -1, skipping necessary zero-extension for architectures that rely on explicit BPF_ZEXT_REG patching? If so, the upper 32 bits might contain garbage data, potentially leading to out-of-bounds memory accesses if the register is later used in an address calculation. > + > + /* atomics, see insn_def_regno() */ > + if (class == BPF_STX && size != BPF_DW) > + return false; [ ... ] > @@ -623,7 +659,7 @@ int bpf_opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env, > * BPF_STX + SRC_OP, so it is safe to pass NULL > * here. > */ > - if (bpf_is_reg64(&insn, load_reg, NULL, DST_OP)) { > + if (bpf_is_reg64(&insn)) { [Severity: Low] This isn't a bug, but should the comment directly above this call be removed as well? It references a fourth parameter that has been deleted by this patch, which could be confusing for future readers. > if (class == BPF_LD && > BPF_MODE(code) == BPF_IMM) > i++; -- Sashiko AI review · https://sashiko.dev/#/patchset/20260731-static-zext-v2-0-da4aa161e8c5@gmail.com?part=4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH bpf-next v2 5/5] selftests/bpf: verify zext_dst annotations for various instructions 2026-08-01 1:09 [PATCH bpf-next v2 0/5] bpf: infer zext_dst based on static register liveness analysis Eduard Zingerman ` (3 preceding siblings ...) 2026-08-01 1:09 ` [PATCH bpf-next v2 4/5] bpf: simplify the bpf_is_reg64() Eduard Zingerman @ 2026-08-01 1:09 ` Eduard Zingerman 2026-08-01 1:33 ` sashiko-bot 4 siblings, 1 reply; 13+ messages in thread From: Eduard Zingerman @ 2026-08-01 1:09 UTC (permalink / raw) To: bpf, ast, andrii Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87, memxor, iii, gimm78064 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 and subprogram parameters are considered to use full 64 bits; - kfunc parameters are read according to their BTF type width. - 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). Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> --- tools/testing/selftests/bpf/prog_tests/verifier.c | 2 + tools/testing/selftests/bpf/progs/verifier_zext.c | 315 ++++++++++++++++++++++ 2 files changed, 317 insertions(+) diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c index be97f6887f0e..fba562c81969 100644 --- a/tools/testing/selftests/bpf/prog_tests/verifier.c +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c @@ -124,6 +124,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 @@ -277,6 +278,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..2233cddde1e1 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/verifier_zext.c @@ -0,0 +1,315 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> +#include "../../../include/linux/filter.h" +#include "bpf_misc.h" + +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); +} + +extern long bpf_kfunc_call_test4(signed char a, short b, int c, long d) __ksym; + +/* Force kfunc extern BTF generation. */ +int __kfunc_btf_root(void) +{ + return bpf_kfunc_call_test4(0, 0, 0, 0); +} + +/* kfunc parameters are read according to their BTF type width */ +SEC("tc") +__success __log_level(2) +__not_msg("w3 = 1{{.*}}; zext") /* int c -> read as 32-bit */ +__msg("w4 = 1{{ +}}; zext") /* long d -> read as 64-bit */ +__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); +} + +char _license[] SEC("license") = "GPL"; -- 2.55.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next v2 5/5] selftests/bpf: verify zext_dst annotations for various instructions 2026-08-01 1:09 ` [PATCH bpf-next v2 5/5] selftests/bpf: verify zext_dst annotations for various instructions Eduard Zingerman @ 2026-08-01 1:33 ` sashiko-bot 2026-08-01 4:24 ` Eduard Zingerman 0 siblings, 1 reply; 13+ messages in thread From: sashiko-bot @ 2026-08-01 1:33 UTC (permalink / raw) To: Eduard Zingerman; +Cc: bpf Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Missing bpf_testmod dependency check in test_verifier_zext, causing spurious test failures when the module is not loaded. -- --- Patch [5]: [PATCH bpf-next v2 5/5] selftests/bpf: verify zext_dst annotations for various instructions --- commit 9e656f5e6b04a69526327df6f70d8c362d02d3d4 Author: Eduard Zingerman <eddyz87@gmail.com> selftests/bpf: verify zext_dst annotations for various instructions This commit adds a suite of selftests to verify the verifier's zero extension behavior across various instructions and state pruning scenarios. It covers cases like 32-bit and 64-bit operations, atomic instructions, kfunc parameters, and CFG edge cases. > diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c > index be97f6887f0e7..fba562c819691 100644 > --- a/tools/testing/selftests/bpf/prog_tests/verifier.c > +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c [ ... ] > @@ -277,6 +278,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); } [Severity: Medium] Since verifier_zext.c calls bpf_kfunc_call_test4 (an extern kfunc from bpf_testmod), will this test fail spuriously if bpf_testmod is not loaded? Should this check env.has_testmod and call test__skip() before RUN_TESTS to gracefully skip the test when the module is unavailable? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260731-static-zext-v2-0-da4aa161e8c5@gmail.com?part=5 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next v2 5/5] selftests/bpf: verify zext_dst annotations for various instructions 2026-08-01 1:33 ` sashiko-bot @ 2026-08-01 4:24 ` Eduard Zingerman 0 siblings, 0 replies; 13+ messages in thread From: Eduard Zingerman @ 2026-08-01 4:24 UTC (permalink / raw) To: sashiko-reviews; +Cc: bpf On Sat, 2026-08-01 at 01:33 +0000, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Medium] Missing bpf_testmod dependency check in test_verifier_zext, causing spurious test failures when the module is not loaded. > -- > > --- Patch [5]: [PATCH bpf-next v2 5/5] selftests/bpf: verify zext_dst annotations for various instructions --- > commit 9e656f5e6b04a69526327df6f70d8c362d02d3d4 > Author: Eduard Zingerman <eddyz87@gmail.com> > > selftests/bpf: verify zext_dst annotations for various instructions > > This commit adds a suite of selftests to verify the verifier's zero extension > behavior across various instructions and state pruning scenarios. It covers > cases like 32-bit and 64-bit operations, atomic instructions, kfunc parameters, > and CFG edge cases. > > > diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c > > index be97f6887f0e7..fba562c819691 100644 > > --- a/tools/testing/selftests/bpf/prog_tests/verifier.c > > +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c > [ ... ] > > @@ -277,6 +278,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); } > > [Severity: Medium] > Since verifier_zext.c calls bpf_kfunc_call_test4 (an extern kfunc from > bpf_testmod), will this test fail spuriously if bpf_testmod is not loaded? > > Should this check env.has_testmod and call test__skip() before RUN_TESTS > to gracefully skip the test when the module is unavailable? Idk, bpf_kfunc_call_test4() is used by the test progs/kfunc_call_test.c prog_tests/kfunc_call.c, which itself does not check for env.has_testmod. I don't think we enforce this rigorously, nor that we should. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-08-01 5:41 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-01 1:09 [PATCH bpf-next v2 0/5] bpf: infer zext_dst based on static register liveness analysis Eduard Zingerman 2026-08-01 1:09 ` [PATCH bpf-next v2 1/5] bpf: do not print a newline after disassembly in bpf_verbose_insn() Eduard Zingerman 2026-08-01 1:09 ` [PATCH bpf-next v2 2/5] bpf: track upper 32-bit register halves' liveness in compute_live_registers() Eduard Zingerman 2026-08-01 1:35 ` sashiko-bot 2026-08-01 4:40 ` Eduard Zingerman 2026-08-01 1:09 ` [PATCH bpf-next v2 3/5] bpf: infer zext_dst based on static register liveness analysis Eduard Zingerman 2026-08-01 1:42 ` sashiko-bot 2026-08-01 5:41 ` Eduard Zingerman 2026-08-01 1:09 ` [PATCH bpf-next v2 4/5] bpf: simplify the bpf_is_reg64() Eduard Zingerman 2026-08-01 1:34 ` sashiko-bot 2026-08-01 1:09 ` [PATCH bpf-next v2 5/5] selftests/bpf: verify zext_dst annotations for various instructions Eduard Zingerman 2026-08-01 1:33 ` sashiko-bot 2026-08-01 4:24 ` Eduard Zingerman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox