Re: [PATCH bpf-next v2 07/13] bpf: Add verifier support for 16-byte returns in R0:R2

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Kfuncs returning 9-15 byte structs leak uninitialized kernel memory via R2 because the verifier bypasses zero-extension.
--

--- Patch [7]: [PATCH bpf-next v2 07/13] bpf: Add verifier support for 16-byte returns in R0:R2 ---
commit 5c235db7cbd600a1b91b24fbad96652d702b6b3a
Author: Yonghong Song <[email protected]>

bpf: Add verifier support for 16-byte returns in R0:R2

This commit adds verifier support for returning values in two registers
(R0 and R2) for structures between 9 and 16 bytes or __int128. It adds
handling for subprogram and kfunc returns to map the second return
register to BPF_REG_2.

[ ... ]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 5584178a0e1c7..60b9e587e0947 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -10859,6 +10901,22 @@ static void mark_btf_func_reg_size(struct bpf_verifier_env *env, u32 regno,
>  	return __mark_btf_func_reg_size(env, cur_regs(env), regno, reg_size);
>  }
>  
> +/* Mark the register(s) holding a @size byte kfunc return value as unknown
> + * scalars. All of them are processed the same way, only the size differs:
> + * a single register may hold a sub-register sized value, while both halves
> + * of a register pair are treated as 64-bit wide.
> + */
> +static void mark_kfunc_ret_regs(struct bpf_verifier_env *env,
> +				struct bpf_reg_state *regs, u32 size)
> +{
> +	u32 i, nregs = ret_regs_cnt(size);
> +
> +	for (i = 0; i < nregs; i++) {
> +		mark_reg_unknown(env, regs, ret_regs[i]);
> +		mark_ret_reg_size(env, regs, ret_regs[i], nregs == 1 ? size : sizeof(u64));

[Severity: Medium]
For kfuncs returning a struct between 9 and 15 bytes in size, does
forcing sizeof(u64) for both return registers bypass zero-extension for R2?

If the native compiler only populates the lower bytes of the second return
register (e.g. RDX on x86_64) for a <16 byte struct, the upper bits will be
left uninitialized. However, setting the size to sizeof(u64) here sets
subreg_def to DEF_NOT_SUBREG for R2, which treats R2 as a fully initialized
64-bit scalar.

Since the verifier's zero-extension logic only supports zero-extending the
destination register (R0 for calls), couldn't a BPF program performing a
64-bit read of R2 leak up to 7 bytes of uninitialized kernel register state?

> +	}
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7