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

On Sat, 2026-08-01 at 01:42 +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issu=
e(s) to consider:
>=20
> New issues:
> - [High] Dynamically downgraded 64-bit arena operations and pointer casts=
 are missing explicit zero-extension on architectures requiring it.
>=20
> Pre-existing issues:
> - [High] Kfuncs bpf_mem_cgroup_memory_events and bpf_mem_cgroup_vm_events=
 do not properly validate negative enum values, leading to out-of-bounds ar=
ray reads.
> --
>=20
> --- Patch [3]: [PATCH bpf-next v2 3/5] bpf: infer zext_dst based on stati=
c register liveness analysis ---
> commit 625c020066cfbb5a384d20ff5e1ad79faa898949
> Author: Eduard Zingerman <[email protected]>
>=20
> bpf: infer zext_dst based on static register liveness analysis
>=20
> This commit replaces the verifier's subreg_def based logic with computing
> zext_dst statically as part of bpf_compute_live_registers(). It tracks th=
e
> liveness of the upper and lower halves of registers separately to emit ex=
plicit
> zero extensions on architectures that need them.
>=20
> Link: https://lore.kernel.org/bpf/CAGKGUv=3DsOuqQtA1Ub-5JXfA4FPosJFYKAQE4=
[email protected]/
>=20
> [ ... ]
>=20
> > diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
> > index d0bd94c60fee2..77b84b367be74 100644
> > --- a/kernel/bpf/liveness.c
> > +++ b/kernel/bpf/liveness.c
> > @@ -2316,9 +2317,18 @@ int bpf_compute_live_registers(struct bpf_verifi=
er_env *env)
> > =C2=A0	}
> > =C2=A0
> > =C2=A0	for (i =3D 0; i < insn_cnt; ++i) {
> > +		int def32 =3D bpf_insn_def32(&insns[i]);
> > +		u32 out =3D state[i].out;
> > =C2=A0		u32 in =3D state[i].in;
> > =C2=A0
> > =C2=A0		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))=
;
> > =C2=A0	}
>=20
> [Severity: High]
> Does this statically evaluate zext_dst before some instructions are dynam=
ically
> downgraded?
>=20
> If an ALU64 operation with a PTR_TO_ARENA operand is statically evaluated
> here, zext_dst evaluates to false because it is still a 64-bit instructio=
n.
>=20
> Later during verification, adjust_reg_min_max_vals() sets needs_zext =3D =
true.
> Then bpf_do_misc_fixups() downgrades the instruction in-place to a 32-bit=
 ALU
> operation:
>=20
> kernel/bpf/fixups.c:bpf_do_misc_fixups() {
> =C2=A0=C2=A0=C2=A0 ...
> =C2=A0=C2=A0=C2=A0 if (env->insn_aux_data[i + delta].needs_zext)
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* Convert BPF_CLASS(insn->cod=
e) =3D=3D BPF_ALU64 to 32-bit ALU */
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 insn->code =3D BPF_ALU | BPF_O=
P(insn->code) | BPF_SRC(insn->code);
> =C2=A0=C2=A0=C2=A0 ...
> }
>=20
> Since this bypasses bpf_patch_insn_data(), zext_dst remains false. Could =
this
> cause bpf_opt_subreg_zext_lo32_rnd_hi32() to skip injecting the required =
zero
> extension, leaving garbage in the upper 32 bits on architectures like s39=
0x
> and potentially leading to out-of-bounds accesses?

Yeap, need to figure something out.

...