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

On 8/18/26 11:13 PM, [email protected] wrote:
> 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 <[email protected]>
>
> 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?

We should be okay here. If freplace replacing the original subprog 0, it
is up to users to check the result if the freplace returning R0:R2.

>