Re: [PATCH bpf-next v2 2/5] bpf: track upper 32-bit register halves' liveness in compute_live_registers()

On Sat, 2026-08-01 at 01:35 +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issu=
e(s) to consider:
> - [High] Skipping zero-extension for 32-bit kfunc arguments causes ABI vi=
olations on architectures that require the caller to extend 32-bit paramete=
rs.
> --
>=20
> --- Patch [2]: [PATCH bpf-next v2 2/5] bpf: track upper 32-bit register h=
alves' liveness in compute_live_registers() ---
> commit 2a034896228b7579bf93bf8ae2024cee100ebcba
> Author: Eduard Zingerman <[email protected]>
>=20
> bpf: track upper 32-bit register halves' liveness in compute_live_registe=
rs()
>=20
> This commit extends compute_live_registers() to track the upper and lower=
=20
> register halves' liveness separately. It updates compute_insn_live_regs()=
=20
> to track these bits and infers whether the upper half of a parameter=20
> register is used by a kfunc call based on its BTF type size.
>=20
> > diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
> > index ff1e68cc4bd1c..d0bd94c60fee2 100644
> > --- a/kernel/bpf/liveness.c
> > +++ b/kernel/bpf/liveness.c
>=20
> [ ... ]
>=20
> > +static u32 call_use_mask(struct bpf_verifier_env *env, struct bpf_insn=
 *insn)
> > +{
> > +	u32 i, use, size, reg_params_num;
> > +	struct bpf_call_summary cs;
> > +	const struct btf_param *p;
> > +	const struct btf_type *t;
> > +
> > +	if (!bpf_get_call_summary(env, insn, &cs))
> > +		/* e.g. a bpf-to-bpf call, assume that calls use full 64-bit for eac=
h param */
> > +		return mask_widen(ALL_CALLER_SAVED_REGS & ~BIT(BPF_REG_0));
> > +
> > +	if (cs.func_proto) {
> > +		use =3D 0;
> > +		p =3D btf_params(cs.func_proto);
> > +		reg_params_num =3D min(btf_type_vlen(cs.func_proto), MAX_BPF_FUNC_RE=
G_ARGS);
> > +		for (i =3D 0; i < reg_params_num; i++) {
> > +			t =3D btf_type_by_id(cs.btf, p[i].type);
> > +			if (IS_ERR(btf_resolve_size(cs.btf, t, &size)))
> > +				goto fallback;
> > +			use |=3D size <=3D sizeof(u32) ? reg32_mask(i + 1) : reg64_mask(i +=
 1);
>=20
> [Severity: High]
> Does this break the C ABI on architectures that require the caller to
> zero-extend or sign-extend 32-bit parameters passed in 64-bit registers?
>=20
> By only marking the lower 32 bits as used for smaller parameters with
> reg32_mask, the upper 32 bits are marked as dead before the kfunc call.
> Consequently, the verifier computes zext_dst =3D false for the instructio=
n
> that previously defined the argument.
>=20
> This causes the JIT to skip inserting a BPF_ZEXT_REG instruction to clear
> the upper 32 bits on architectures where bpf_jit_needs_zext() is true,
> such as s390x and PowerPC64.
>=20
> If a BPF program computes a 32-bit value and passes it to a kfunc expecti=
ng
> a 32-bit parameter, the kfunc might operate on garbage data in the upper
> 32 bits.

Ok, I misread the original logic:

  static void mark_insn_zext(...)
  {
        ...
	env->insn_aux_data[def_idx - 1].zext_dst =3D true;
        ...
  }

  /* mark_btf_func_reg_size() is used when the reg size is determined by
   * the BTF func_proto's return value size and argument.
   */
  static void __mark_btf_func_reg_size(...)
  {
	...
	if (regno =3D=3D BPF_REG_0) {
		/* Function return value */
		...
	} else if (reg_size =3D=3D sizeof(u64)) {
		/* Function argument */
		mark_insn_zext(env, reg);
	}
  }

Meaning that the function parameters were always marked for zero
extension. This is also confirmed by ELF Application Binary Interface
s390x Supplement [1]:

> =E2=80=B9simple_arg=E2=80=BA: A simple_arg is one of the following:
> =E2=80=A2 One of the simple integer types no more than 64 bits wide. This=
 includes
> signed char, short, int, long, long long, _BitInt(=F0=9D=91=81) with =F0=
=9D=91=81 =E2=89=A4 64,
> their unsigned counterparts, _Bool, and any enum type. If such an argumen=
t
> is shorter than 64 bits, replace it by a full 64-bit integer representing=
 the
> same number, using sign or zero extension, as appropriate.

This is good news, as it makes changes to liveness tracking much simpler,
there is no need to inspect BTF after all.

[1] https://github.com/IBM/s390x-abi/blob/main/docs/lzsabi_s390x.pdf