Re: [PATCH bpf-next v4 09/13] bpf: Enable aggregate return types up to 16 bytes

On 8/12/26 3:47 PM, Eduard Zingerman wrote:
> On Mon, 2026-08-10 at 17:09 -0700, Yonghong Song wrote:
>> Relax btf_distill_func_proto() to accept a by-value struct or union that
>> the R0:R2 convention added in earlier patches can carry:
>>
>>   - a struct or union larger than 8 and up to 16 bytes, returned in the
>>     R0:R2 register pair, matching what LLVM emits for the BPF target;
>>   - a struct or union up to 8 bytes, returned in R0 alone.
>>
>> A >8 byte scalar (__int128) was already accepted and is unchanged.
>> Everything else stays rejected: a return type larger than 16 bytes, and any
>> type that __get_type_size() cannot return in registers at all (e.g. an
>> array), which it already reports as ret < 0.
>>
>> btf_distill_func_proto() also builds the trampoline (fentry/fexit/fmod_ret)
>> and struct_ops function models, so relaxing it widens what those can attach
>> to. A >8 byte return stays rejected on every path that reads the target's
>> return value: commit c48796aa6c39 ("bpf: Reject >8 byte return values on
>> return-reading trampoline paths") covers fexit, fmod_ret and fsession plus
>> their _multi variants, and struct_ops, and an fentry-only trampoline never
>> sets BPF_TRAMP_F_CALL_ORIG so it does not touch the return value at all. A
>> struct or union of 8 bytes or less is newly accepted for those paths; its
>> single eightbyte is returned in R0 like any other scalar.
>>
>> btf_validate_return_type() is relaxed as well, so that it accepts a
>> by-value struct or union up to 16 bytes in addition to void and scalars.
>>
>> With btf_distill_func_proto() and btf_validate_return_type() relaxed, the
>> verifier, JIT, precision-backtracking and live-register support from the
>> earlier patches becomes reachable: this final patch enables <=16 byte
>> aggregate return values end to end.
>>
>> Signed-off-by: Yonghong Song <[email protected]>
>> ---
> Acked-by: Eduard Zingerman <[email protected]>
>
> ...
>
>> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
>> index 6606187ed4f4..5551abcea1d3 100644
>> --- a/kernel/bpf/btf.c
>> +++ b/kernel/bpf/btf.c
>> @@ -7592,7 +7592,12 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
>>   		return -EINVAL;
>>   	}
>>   	ret = __get_type_size(btf, func->type, &t);
>> -	if (ret < 0 || btf_type_is_struct(t)) {
>> +	/*
>> +	 * __get_type_size() already restricts a non-negative ret to void, a
>> +	 * pointer, an int, an enum or a struct/union, so only the size is checked
>> +	 * here.
>> +	 */
> Nit: I'd drop this comment.

Will do.

>
>> +	if (ret < 0 || ret > 16) {
>>   		bpf_log(log,
>>   			"The function %s return type %s is unsupported.\n",
>>   			tname, btf_type_str(t));
> ...
>
>> @@ -7988,6 +7993,35 @@ 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 >8 byte struct/union is returned in the R0:R2 register pair.
>> +		 * A global function is verified in isolation, so its caller models
>> +		 * the return as an opaque R0:R2 scalar pair; it must therefore
>> +		 * contain only scalars, otherwise a pointer field would be
>> +		 * laundered into a scalar and escape provenance and reference
>> +		 * tracking. That requirement is enforced here: do_check_common()
>> +		 * propagates the error for global functions and for the main
>> +		 * program.
>> +		 *
>> +		 * A local (static) function is verified inline and its R0:R2 are
>> +		 * copied as precise register state (with the JIT forced on when
>> +		 * the pair is consumed), so a pointer field stays tracked and needs
>> +		 * no such restriction. Accepting it here is not by itself what
>> +		 * makes it legal: btf_check_subprog_call() drops any error other
>> +		 * than -EFAULT. What it avoids is needlessly marking the
>> +		 * subprogram's BTF unreliable.
>> +		 *
>> +		 * The main program (subprog 0) takes the scalar-only path as well,
>> +		 * but its return value is the program's exit code, so a >8 byte
>> +		 * return is rejected separately at BPF_EXIT.
>> +		 */
> Nit: the comment is way too long, I'd drop the justification.
>       Especially given that at the moment btf_validate_return_type()
>       would only be called from main/global subprograms/callback subprograms.

Will do.

>
>> +		bool local_func = subprog && !is_global;
>> +
>> +		if (local_func || __btf_type_is_scalar_struct(env, btf, t, 0))
>> +			return 0;
>> +	}
>> +
>>   	return -EOPNOTSUPP;
>>   }
>>   
> ...
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 8f0c4aed0781..f8294359c85d 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -11088,9 +11088,9 @@ static bool is_kfunc_arg_implicit(const struct bpf_call_arg_meta *meta, u32 arg_
>>   }
>>   
>>   /* Returns true if struct is composed of scalars, 4 levels of nesting allowed */
>> -static bool __btf_type_is_scalar_struct(struct bpf_verifier_env *env,
>> -					const struct btf *btf,
>> -					const struct btf_type *t, int rec)
>> +bool __btf_type_is_scalar_struct(struct bpf_verifier_env *env,
>> +				 const struct btf *btf,
>> +				 const struct btf_type *t, int rec)
> Nit: since this function is now exported, let's drop the '__' prefix.

Will change.

>
>>   {
>>   	const struct btf_type *member_type;
>>   	const struct btf_member *member;
> ...