Re: [PATCH bpf-next 3/5] bpf: infer zext_dst based on static register liveness analysis

On Fri, 2026-07-31 at 19:26 +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] Incorrect static liveness tracking for 64-bit conditional jumps leads to missing zero-extensions (`zext_dst`), resulting in verifier/runtime state divergence and potential privilege escalation.
> --
> 
> --- Patch [3]: [PATCH bpf-next 3/5] bpf: infer zext_dst based on static register liveness analysis ---
> commit ab8592d6a434dbfed0cbb0bd067d9032b308db8d
> Author: Eduard Zingerman <[email protected]>
> 
> bpf: infer zext_dst based on static register liveness analysis
> 
> This commit replaces the dynamic subreg_def based zero-extension
> tracking with static liveness analysis to correctly infer zext_dst
> and prevent missing zero-extensions on architectures like s390x.
> 
> 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 c996b8a03e792..ea002550671a8 100644
> > --- a/kernel/bpf/liveness.c
> > +++ b/kernel/bpf/liveness.c
> > @@ -2317,9 +2318,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: Critical]
> Does this code correctly evaluate liveness for 64-bit conditional jumps?
> 
> The new zext_dst logic relies on mask_hi(out) accurately reflecting if the
> upper half of a register is alive.
> 
> However, looking at compute_insn_live_regs() in kernel/bpf/liveness.c for
> conditional jumps:
> 
>     const u8 class = BPF_CLASS(insn->code);
>     const u8 code = BPF_OP(insn->code);
>     ...
>     case BPF_JMP:
>     case BPF_JMP32:
>         switch (code) {
>         ...
>         default:
>             def = 0;
>             if (BPF_SRC(insn->code) == BPF_K)
>                 use = dst;
>             else
>                 use = dst | (code == BPF_JMP32 ? src : src32);
>         }
> 
> Can the code variable ever equal BPF_JMP32?
> 
> BPF_OP() extracts the operation code (which is a multiple of 16), while
> BPF_JMP32 is a class (0x06). This means the check code == BPF_JMP32 evaluates
> to false, and the source register usage always falls back to src32, even for
> 64-bit jumps (BPF_JMP).
> 
> If a 32-bit operation defines a register, and it is followed by a 64-bit
> conditional jump using that register as a source with no other uses of the
> upper 32 bits, the upper half won't be marked as alive.
> 
> As a result, zext_dst wouldn't be set, which could lead to the JIT skipping
> the mandatory zero extension. This would cause the 64-bit jump to compare
> against uninitialized garbage in the upper bits, potentially allowing
> out-of-bounds access.
> 
> Should this check use class == BPF_JMP32 instead?
> 
> >  	}

This is the same (real) issue as reported for the previous patch.