diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 39a851e690ec..76b8b7627a10 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -1243,6 +1243,11 @@ static inline void bpf_bt_set_frame_slot(struct backtrack_state *bt, u32 frame, bt->stack_masks[frame] |= 1ull << slot; } +static inline void bpf_bt_set_frame_slot_mask(struct backtrack_state *bt, u32 frame, u64 mask) +{ + bt->stack_masks[frame] |= mask; +} + static inline void bt_set_frame_stack_arg_slot(struct backtrack_state *bt, u32 frame, u32 slot) { bt->stack_arg_masks[frame] |= 1 << slot; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 21a365d436a5..25aea4271cd0 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -3702,14 +3702,21 @@ static int check_stack_write_var_off(struct bpf_verifier_env *env, * SCALAR. This function does not deal with register filling; the caller must * ensure that all spilled registers in the stack range have been marked as * read. + * + * STACK_SPILL bytes backed by spilled scalar const zeroes are also considered + * zero bytes. In that case, mark the contributing stack slots precise so + * pruning cannot reuse a zero-spill state for a later non-zero spill state. + * + * Returns an error if precision backtracking fails. */ -static void mark_reg_stack_read(struct bpf_verifier_env *env, - /* func where src register points to */ - struct bpf_func_state *ptr_state, - int min_off, int max_off, int dst_regno) +static int mark_reg_stack_read(struct bpf_verifier_env *env, + /* func where src register points to */ + struct bpf_func_state *ptr_state, + int min_off, int max_off, int dst_regno) { struct bpf_verifier_state *vstate = env->cur_state; struct bpf_func_state *state = vstate->frame[vstate->curframe]; + u64 zero_spill_mask = 0; int i, slot, spi; u8 *stype; int zeros = 0; @@ -3719,19 +3726,33 @@ static void mark_reg_stack_read(struct bpf_verifier_env *env, spi = slot / BPF_REG_SIZE; mark_stack_slot_scratched(env, spi); stype = ptr_state->stack[spi].slot_type; - if (stype[slot % BPF_REG_SIZE] != STACK_ZERO) - break; - zeros++; + if (stype[slot % BPF_REG_SIZE] == STACK_ZERO) { + zeros++; + continue; + } + if (stype[slot % BPF_REG_SIZE] == STACK_SPILL && + bpf_register_is_null(&ptr_state->stack[spi].spilled_ptr)) { + zero_spill_mask |= 1ull << spi; + zeros++; + continue; + } + break; } if (zeros == max_off - min_off) { /* Any access_size read into register is zero extended, * so the whole register == const_zero. */ __mark_reg_const_zero(env, &state->regs[dst_regno]); + if (zero_spill_mask) { + bpf_bt_set_frame_slot_mask(&env->bt, ptr_state->frameno, zero_spill_mask); + return mark_chain_precision_batch(env, env->cur_state); + } } else { /* have read misc data from the stack */ mark_reg_unknown(env, state->regs, dst_regno); } + + return 0; } /* Read the stack at 'off' and put the results into the register indicated by @@ -3753,6 +3774,7 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env, int i, slot = -off - 1, spi = slot / BPF_REG_SIZE; struct bpf_reg_state *reg; u8 *stype, type; + int err; int insn_flags = INSN_F_STACK_ACCESS; int hist_spi = spi, hist_frame = reg_state->frameno; @@ -3835,7 +3857,10 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env, __mark_reg_const_zero(env, &state->regs[dst_regno]); insn_flags = 0; /* not restoring original register state */ } else { - mark_reg_unknown(env, state->regs, dst_regno); + err = mark_reg_stack_read(env, reg_state, off, off + size, + dst_regno); + if (err) + return err; insn_flags = 0; /* not restoring original register state */ } } @@ -3880,8 +3905,11 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env, } return -EACCES; } - if (dst_regno >= 0) - mark_reg_stack_read(env, reg_state, off, off + size, dst_regno); + if (dst_regno >= 0) { + err = mark_reg_stack_read(env, reg_state, off, off + size, dst_regno); + if (err) + return err; + } insn_flags = 0; /* we are not restoring spilled register */ } if (insn_flags) @@ -3935,7 +3963,10 @@ static int check_stack_read_var_off(struct bpf_verifier_env *env, struct bpf_reg min_off = reg_smin(reg) + off; max_off = reg_smax(reg) + off; - mark_reg_stack_read(env, ptr_state, min_off, max_off + size, dst_regno); + err = mark_reg_stack_read(env, ptr_state, min_off, max_off + size, + dst_regno); + if (err) + return err; check_fastcall_stack_contract(env, ptr_state, env->insn_idx, min_off); return 0; } diff --git a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c index 0174887e28f5..72c691333703 100644 --- a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c +++ b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c @@ -634,6 +634,32 @@ __naked void partial_stack_load_preserves_partial_zeros(void) : __clobber_common); } +SEC("raw_tp") +__log_level(2) +__success +__msg("mark_precise: frame0: regs= stack=-8") +__msg("R2=0") +__naked void stack_load_preserves_mixed_zero_and_zero_spill(void) +{ + asm volatile ( + /* fp-8 has scalar const-zero spill bytes and STACK_ZERO bytes. */ + ".8byte %[fp4_st_zero];" /* LLVM-18+: *(u32 *)(r10 -4) = 0; */ + "r0 = 0;" + "*(u32 *)(r10 -8) = r0;" + + "r1 = %[single_byte_buf];" + "r2 = *(u64 *)(r10 -8);" + "r1 += r2;" + "*(u8 *)(r1 + 0) = r2;" /* this should be fine */ + + "r0 = 0;" + "exit;" + : + : __imm_ptr(single_byte_buf), + __imm_insn(fp4_st_zero, BPF_ST_MEM(BPF_W, BPF_REG_FP, -4, 0)) + : __clobber_common); +} + char two_byte_buf[2] SEC(".data.two_byte_buf"); SEC("raw_tp") diff --git a/tools/testing/selftests/bpf/progs/verifier_var_off.c b/tools/testing/selftests/bpf/progs/verifier_var_off.c index f345466bca68..24cd0a763673 100644 --- a/tools/testing/selftests/bpf/progs/verifier_var_off.c +++ b/tools/testing/selftests/bpf/progs/verifier_var_off.c @@ -59,6 +59,116 @@ __naked void stack_read_priv_vs_unpriv(void) " ::: __clobber_all); } +SEC("cgroup/skb") +__description("variable-offset stack read preserves spilled zero") +__success +__log_level(2) +__msg("mark_precise: frame0: regs= stack=-8") +__msg("R3=0") +__retval(0) +__naked void stack_read_var_off_preserves_spilled_zero(void) +{ + asm volatile (" \ + r0 = 0; \ + *(u64*)(r10 - 8) = r0; \ + r2 = *(u32*)(r1 + 0); \ + r2 &= 7; \ + r2 -= 8; \ + r2 += r10; \ + r3 = *(u8*)(r2 + 0); \ + r1 = r10; \ + r1 += -1; \ + r1 += r3; \ + *(u8*)(r1 + 0) = r3; \ + r0 = 0; \ + exit; \ +" ::: __clobber_all); +} + +SEC("cgroup/skb") +__description("variable-offset stack read preserves spilled zero across slots") +__success +__log_level(2) +__msg("mark_precise: frame0: regs= stack=-8,-16") +__msg("R3=0") +__retval(0) +__naked void stack_read_var_off_preserves_spilled_zero_across_slots(void) +{ + asm volatile (" \ + r0 = 0; \ + *(u64*)(r10 - 8) = r0; \ + *(u64*)(r10 - 16) = r0; \ + r2 = *(u32*)(r1 + 0); \ + r2 &= 15; \ + r2 -= 16; \ + r2 += r10; \ + r3 = *(u8*)(r2 + 0); \ + r1 = r10; \ + r1 += -1; \ + r1 += r3; \ + *(u8*)(r1 + 0) = r3; \ + r0 = 0; \ + exit; \ +" ::: __clobber_all); +} + +SEC("cgroup/skb") +__description("variable-offset stack read preserves partial spilled zero") +__success +__log_level(2) +__msg("mark_precise: frame0: regs= stack=-8") +__msg("R3=0") +__retval(0) +__naked void stack_read_var_off_preserves_partial_spilled_zero(void) +{ + asm volatile (" \ + r0 = 0; \ + *(u8*)(r10 - 9) = r0; \ + *(u8*)(r10 - 10) = r0; \ + *(u8*)(r10 - 11) = r0; \ + *(u8*)(r10 - 12) = r0; \ + *(u8*)(r10 - 13) = r0; \ + *(u8*)(r10 - 14) = r0; \ + *(u8*)(r10 - 15) = r0; \ + *(u32*)(r10 - 8) = r0; \ + r2 = *(u32*)(r1 + 0); \ + r2 &= 15; \ + if r2 > 10 goto l0_%=; \ + r2 -= 15; \ + r2 += r10; \ + r3 = *(u8*)(r2 + 0); \ + r1 = r10; \ + r1 += -1; \ + r1 += r3; \ + *(u8*)(r1 + 0) = r3; \ +l0_%=: r0 = 0; \ + exit; \ +" ::: __clobber_all); +} + +SEC("cgroup/skb") +__description("variable-offset stack read partial spill with misc data") +__failure +__msg("invalid variable-offset write to stack R1") +__naked void stack_read_var_off_partial_spill_with_misc_data(void) +{ + asm volatile (" \ + r0 = 0; \ + *(u32*)(r10 - 8) = r0; \ + r2 = *(u32*)(r1 + 0); \ + r2 &= 7; \ + r2 -= 8; \ + r2 += r10; \ + r3 = *(u8*)(r2 + 0); \ + r1 = r10; \ + r1 += -1; \ + r1 += r3; \ + *(u8*)(r1 + 0) = 0; \ + r0 = 0; \ + exit; \ +" ::: __clobber_all); +} + SEC("cgroup/skb") __description("variable-offset stack read, uninitialized") __success