Re: [PATCH bpf-next 1/4] bpf: Mark pending sub-register zero extension before pruning a state
Eduard Zingerman <[email protected]>
| Newsgroups | org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On Wed, 2026-08-05 at 20:44 +0200, Daniel Borkmann wrote:
> A 32-bit write records the writing instruction in reg->subreg_def, and a
> later 64-bit read of that register calls mark_insn_zext() to record that
> the definition has to be zero extended. Architectures whose JIT sets
> bpf_jit_needs_zext() rely on that mark to emit the extension.
>
> The mark is produced by walking the path from the definition to the read.
> If the walk stops at a state equivalent to an already explored one, the
> reads the explored path performs from there on are not repeated for this
> path's registers, so a definition whose only 64-bit read lies beyond the
> pruning point never gets marked and keeps a garbage upper half.
>
> Example with BPF_F_TEST_STATE_FREQ making every instruction a checkpoint:
>
> r7 = *(u32 *)(r1 + offsetof(struct __sk_buff, len))
> r6 = 0 /* 64-bit define */
> if r7 != 0 goto l1
> goto l0 path A, explored first
> l1: w6 = 0 /* 32-bit define */ path B, explored second
> l0: r0 = r6 /* 64-bit read */
> r0 >>= 32
> exit
>
> Now, path A is the fall-through of the conditional and is explored first.
> It reaches l0 with r6 defined by the 64-bit r6 = 0, so reg->subreg_def is
> DEF_NOT_SUBREG and the read marks nothing. The walk runs on to exit and
> leaves a checkpoint at every instruction along the way. Path B is explored
> second. w6 = 0 sets r6->subreg_def to that instruction, so a zero extension
> is pending and only the 64-bit read at l0 can resolve it. B then arrives
> at l0, where bpf_is_state_visited() finds the checkpoint A left behind:
> r6 is the scalar 0 in both states, so they are equivalent and B is pruned
> before r0 = r6 is verified.
>
> Without the fix nothing happens at that point, so the one read that would
> have called mark_insn_zext() for w6 is never walked and the definition
> stays unmarked:
>
> l1: w6 = 0 /* subreg_def = w6, pending */
> l0: r0 = r6 <--- B pruned, read never walked, w6 stays unmarked
>
> The JIT of an architecture that needs explicit zero extension then emits
> none, and the upper half of w6's definition is left undefined (under
> BPF_F_TEST_RND_HI32 it holds the randomized half, which r0 >>= 32 returns).
> This used to be handled by the registers chain based liveness: the
> propagate_liveness() called mark_insn_zext() for every parent register
> whose read mark was REG_LIVE_READ64, which carried the requirement across
> a pruned state. Commit 107e16979905 ("bpf: disable and remove registers
> chain based liveness") removed that machinery and with it the propagation,
> leaving the mark dependent on the path actually being walked.
>
> Mark the pending definitions where the walk stops instead, i.e. on the way
> into the prune rather than at the read that is no longer reached:
>
> l1: w6 = 0 /* subreg_def = w6, pending */
> l0: r0 = r6 <--- mark w6, r6 is live here
>
> The set of registers to mark is the one the pruning decision was made on:
> func_states_equal() compares the registers live at the instruction, per
> insn_aux_data[].live_regs_before, and those are exactly the registers that
> can still be read. A register that is not live there is never read again
> and needs nothing. This is conservative in one direction: a live register
> whose remaining reads are all 32-bit also gets its definition marked,
> which costs a zero extension that is not needed. However, it never marks
> too little, and it does not weaken pruning. On x86-64 the effect is only
> observable with BPF_F_TEST_RND_HI32.
>
> The one live scalar whose subreg_def can point at a call insn is r0 of a
> kfunc returning a 32-bit value. Marking that one is harmless, the fixup pass
> skips kfunc calls since their zero extension is done by the caller.
>
> Fixes: 107e16979905 ("bpf: disable and remove registers chain based liveness")
> Reported-by: STAR Labs SG <[email protected]>
> Signed-off-by: Daniel Borkmann <[email protected]>
> ---
Hi Daniel,
I have this series in progress:
https://lore.kernel.org/bpf/[email protected]/
Fixing that exact issue. It needs one more iteration.
...