Re: [PATCH bpf-next v7 07/10] bpf: Enable aggregate return types up to 16 bytes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] BPF extension programs (BPF_PROG_TYPE_EXT) bypass return value verification for BPF_REG_2 when replacing a function that returns a 16-byte struct, allowing uninitialized memory or kernel pointers to leak.
--

--- Patch [7]: [PATCH bpf-next v7 07/10] bpf: Enable aggregate return types up to 16 bytes ---
commit 19d85b11c1fe37fb0f420056e54cf29ffa04a0bb
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Enable aggregate return types up to 16 bytes

Relax btf_distill_func_proto() to accept a by-value struct or union that
the R0:R2 convention added in earlier patches can carry.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 5e9f6a4875244..58b81c6f5288c 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
[ ... ]
> @@ -7993,6 +7993,19 @@ static int btf_validate_return_type(struct bpf_verifier_env *env, struct btf *bt
>  	if (btf_type_is_void(t) || btf_type_is_int(t) || btf_is_any_enum(t))
>  		return 0;
>  
> +	if (btf_type_is_struct(t) && t->size <= 16) {
> +		/*
> +		 * A global function's caller models the return as an opaque
> +		 * scalar pair, so it may only return scalars by value. A local
> +		 * function is verified inline, so a pointer field stays tracked
> +		 * and needs no such restriction.
> +		 */
> +		bool local_func = subprog && !is_global;
> +
> +		if (local_func || btf_type_is_scalar_struct(env, btf, t, 0))
> +			return 0;
> +	}
> +
>  	return -EOPNOTSUPP;
>  }

[Severity: High]
Does this allow BPF_PROG_TYPE_EXT extension programs to bypass return value 
verification for BPF_REG_2?

Since this patch enables global functions to return structs up to 16 bytes, a
BPF_PROG_TYPE_EXT program can now replace such a function. This is allowed by
bpf_check_attach_target() because attach_uses_trampoline_retval() is false for
extension programs.

However, during verification of the extension program,
bpf_compute_subprog_ret_regs() explicitly skips subprog 0 (the extension
program itself), which leaves ret_reg_pair as false:

kernel/bpf/verifier.c:bpf_compute_subprog_ret_regs() {
	/*
	 * Skip the main program: its return value is the program's exit code,
	 * read out of R0, so it never uses the register pair. An extension does
	 * have a real prototype for subprog 0, but bpf_check_attach_target()
	 * refuses to replace a function returning more than 8 bytes.
	 */
	for (subprog = 1; subprog < env->subprog_cnt; subprog++) {
        ...
}

Because ret_reg_pair is false, process_bpf_exit_full() later invokes
check_return_code(..., BPF_REG_0) for subprog 0 instead of checking the full
register pair. Could this fail to validate R2 on exit, potentially allowing an
uninitialized or sensitive kernel pointer to leak to the caller?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819055239.3293449-1-yonghong.song@linux.dev?part=7