mark_reg_stack_read() currently treats a variable-offset stack read as known-zero only when every byte in the read range has STACK_ZERO type. This loses precision for stack bytes that are represented as STACK_SPILL but come from a spilled scalar const zero.
Fixed-offset stack reads already preserve such partial reads from a spilled scalar zero as const zero. Variable-offset stack writes also keep a spilled scalar zero intact when a zero write overlaps it. The read side is therefore inconsistent and can reject otherwise valid programs: a byte read from a spilled zero becomes an unknown u8 and may then make a stack access through that value appear out of bounds. Treat STACK_SPILL bytes backed by a spilled scalar const zero as zero bytes in mark_reg_stack_read(). When such a spilled scalar is used to prove the destination register is const zero, mark every contributing source stack slot precise before state pruning can use an explored zero-spill path for a later non-zero spill path. This has to be done eagerly for variable-offset loads. Fixed-offset stack fills can record one source stack slot in the jump history and propagate destination precision back to that slot later, but a variable-offset load may source bytes from multiple stack slots. Seed precision backtracking with every zero-spill slot that contributes to the const-zero classification instead. Add verifier selftests for both sides: accepted programs that read a byte through a variable stack offset from spilled zero scalars, including a multi-slot range, and a rejected program that would be accepted unsafely by a naive implementation that promotes spilled zero bytes to const zero without tracking the source spilled slot precisely. Use BPF_F_TEST_STATE_FREQ for the rejected test so it does not depend on the current verifier checkpoint heuristic thresholds. Tested: ./test_progs -t verifier_var_off Assisted-by: opencode:gpt-5.5 Signed-off-by: Woojin Ji <[email protected]> --- kernel/bpf/verifier.c | 52 ++++++++++--- .../testing/selftests/bpf/progs/verifier_var_off.c | 88 ++++++++++++++++++++++ 2 files changed, 130 insertions(+), 10 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 7fb88e1cd..c4b89fbd9 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -4058,14 +4058,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. + * + * If the const-zero classification depends on spilled scalar zeroes, mark the + * contributing stack slots precise so pruning cannot reuse a zero-spill state + * for a later path containing a different spilled scalar. + * + * 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; @@ -4075,19 +4082,38 @@ 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_is_spilled_scalar_reg(&ptr_state->stack[spi]) && + tnum_is_const(ptr_state->stack[spi].spilled_ptr.var_off) && + ptr_state->stack[spi].spilled_ptr.var_off.value == 0) { + 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) { + for (spi = 0; spi < MAX_BPF_STACK / BPF_REG_SIZE; spi++) { + if (zero_spill_mask & (1ull << spi)) + bpf_bt_set_frame_slot(&env->bt, ptr_state->frameno, spi); + } + 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 @@ -4109,6 +4135,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_stack_access_flags(reg_state->frameno, spi); stype = reg_state->stack[spi].slot_type; @@ -4235,8 +4262,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) @@ -4291,7 +4321,9 @@ static int check_stack_read_var_off(struct bpf_verifier_env *env, min_off = reg->smin_value + off; max_off = reg->smax_value + 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_var_off.c b/tools/testing/selftests/bpf/progs/verifier_var_off.c index f345466bc..2d4878270 100644 --- a/tools/testing/selftests/bpf/progs/verifier_var_off.c +++ b/tools/testing/selftests/bpf/progs/verifier_var_off.c @@ -59,6 +59,94 @@ __naked void stack_read_priv_vs_unpriv(void) " ::: __clobber_all); } +SEC("cgroup/skb") +__description("variable-offset stack read preserves spilled zero") +__success +__failure_unpriv __msg_unpriv("R2 variable stack access prohibited for !root") +__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 +__failure_unpriv __msg_unpriv("R2 variable stack access prohibited for !root") +__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 tracks spilled zero precisely") +__failure +__flag(BPF_F_TEST_STATE_FREQ) +__msg("invalid variable-offset write to stack R1") +__failure_unpriv __msg_unpriv("R2 variable stack access prohibited for !root") +__naked void stack_read_var_off_tracks_spilled_zero_precisely(void) +{ + asm volatile ( + "r6 = *(u32 *)(r1 + 0); " + "r6 &= 1; " + "r0 = 0; " + "if r6 != 0 goto " + "+" + "2; " + " *(u64 *)(r10 - 8) = r0; " + "goto " + "+" + "2; " + "r0 = 1; " + " *(u64 *)(r10 - 8) = r0; " + "r0 = 0; " + "r2 = *(u32 *)(r1 + 4); " + "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 --- base-commit: ddd664bbff63e09e7a7f9acae9c43605d4cf185f change-id: 20260610-bpf-stack-var-off-zero-v1-34ad1bc3b533 Best regards, -- Woojin Ji <[email protected]>
