On Thu, 2026-06-11 at 19:11 +0900, Woojin Ji wrote:
> 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);
Nit: Instead of looping over the mask here, I'd extend the `bpf_bt_*` api
with something like `btf_set_frame_mask(&env->bt, ptr_state->frameno,
zero_spill_mask)`.
> + }
> + return mark_chain_precision_batch(env, env->cur_state);
I tested this change against a big corpus of BPF programs
(selftests, sched_ext, Meta internal, cilium) and see no veristat regressions,
despite this mark_chain_precision_batch() call.
> + }
> } 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)
Please add some __msg() here to verify that `r3` read from stack is
considered to be zero and mark_precise trail. (In other tests as well).
> +__naked void stack_read_var_off_preserves_spilled_zero(void)
> +{
> + asm volatile (
> + "r0 = 0; "
> + " *(u64 *)(r10 - 8) = r0; "
^ ^
Why additional spaces?
> + "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);
> +}
Please add a test cases demonstrating the behavior when spill size is
less than 8 bytes, with neighboring slots being STACK_ZERO + spill or
STACK_MISC + spill.
> +
> +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; "
Huh?
> + "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]>
