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

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.

> +	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.

> +		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.

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

...