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 issu=
e(s) to consider:
> - [Critical] Incorrect static liveness tracking for 64-bit conditional ju=
mps leads to missing zero-extensions (`zext_dst`), resulting in verifier/ru=
ntime state divergence and potential privilege escalation.
> --
>=20
> --- Patch [3]: [PATCH bpf-next 3/5] bpf: infer zext_dst based on static r=
egister liveness analysis ---
> commit ab8592d6a434dbfed0cbb0bd067d9032b308db8d
> Author: Eduard Zingerman <[email protected]>
>=20
> bpf: infer zext_dst based on static register liveness analysis
>=20
> 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.
>=20
> Link: https://lore.kernel.org/bpf/CAGKGUv=3DsOuqQtA1Ub-5JXfA4FPosJFYKAQE4=
[email protected]/
>=20
> > 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_verifi=
er_env *env)
> >  	}
> > =20
> >  	for (i =3D 0; i < insn_cnt; ++i) {
> > +		int def32 =3D bpf_insn_def32(&insns[i]);
> > +		u32 out =3D state[i].out;
> >  		u32 in =3D state[i].in;
> > =20
> >  		insn_aux[i].live_regs_before =3D mask_lo(in) | mask_hi(in);
> > +		/*
> > +		 * On architectures where 32-bit operations do not reset upper halve=
s
> > +		 * 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 =3D def32 >=3D 0 && (mask_hi(out) & BIT(def32))=
;
>=20
> [Severity: Critical]
> Does this code correctly evaluate liveness for 64-bit conditional jumps?
>=20
> The new zext_dst logic relies on mask_hi(out) accurately reflecting if th=
e
> upper half of a register is alive.
>=20
> However, looking at compute_insn_live_regs() in kernel/bpf/liveness.c for
> conditional jumps:
>=20
>     const u8 class =3D BPF_CLASS(insn->code);
>     const u8 code =3D BPF_OP(insn->code);
>     ...
>     case BPF_JMP:
>     case BPF_JMP32:
>         switch (code) {
>         ...
>         default:
>             def =3D 0;
>             if (BPF_SRC(insn->code) =3D=3D BPF_K)
>                 use =3D dst;
>             else
>                 use =3D dst | (code =3D=3D BPF_JMP32 ? src : src32);
>         }
>=20
> Can the code variable ever equal BPF_JMP32?
>=20
> BPF_OP() extracts the operation code (which is a multiple of 16), while
> BPF_JMP32 is a class (0x06). This means the check code =3D=3D BPF_JMP32 e=
valuates
> to false, and the source register usage always falls back to src32, even =
for
> 64-bit jumps (BPF_JMP).
>=20
> 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 th=
e
> upper 32 bits, the upper half won't be marked as alive.
>=20
> As a result, zext_dst wouldn't be set, which could lead to the JIT skippi=
ng
> 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.
>=20
> Should this check use class =3D=3D BPF_JMP32 instead?
>=20
> >  	}

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