Re: [PATCH bpf-next 02/12] bpf: Add verifier support for 16-byte returns in R0:R2

[Yonghong Song <yonghong.song@linux.dev>]

On 7/9/26 3:21 PM, Eduard Zingerman wrote:
> On Wed, 2026-07-08 at 13:09 -0700, Yonghong Song wrote:
>> LLVM 23 added support for returning a value in two registers for an
>> __int128, or a struct/union whose size is greater than 8 but not more
>> than 16 bytes. See LLVM patches [1] and [2].
>>
>> Before LLVM 23 the BPF backend could not return these values at all. A
>> by-value struct or union return (of any size) was rejected at compile
>> time with:
>>
>>    error: aggregate returns are not supported
>>
>> and an __int128 return failed later in the backend with:
>>
>>    fatal error: error in backend: unable to allocate function return #1
>>
>> Both are resolved in LLVM 23, which lowers such returns into the R0:R2
>> register pair.
>>
>> This patch is one of several preparation patches that build up
>> <=16 byte return support incrementally:
>>    - the verifier R0:R2 modelling in this patch,
>>    - the x86 JIT move and JIT-capability gate,
>>    - precision-backtracking and live-register tracking of R2,
>>    - forcing the JIT for such programs, and
>>    - guarding the trampoline paths.
>> Finally, the patch "bpf: Enable 16-byte aggregate return types" enables
>> support for 16-byte returns.
> Nit: I think the above paragraphs belong to a cover letter.

Will add this to the cover letter.

>
>> This patch adds handling for '>8' byte returns in several places: BPF
>> subprogram returns (the main program, and both global and static
>> subprograms) and kfunc returns.
>>
>>    [1] https://github.com/llvm/llvm-project/pull/190894
>>    [2] https://github.com/llvm/llvm-project/pull/206876
>>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   kernel/bpf/verifier.c | 93 ++++++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 88 insertions(+), 5 deletions(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 3f34ce1a3107..03ffb5f839fa 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -424,6 +424,35 @@ static bool subprog_returns_void(struct bpf_verifier_env *env, int subprog)
>>   	return btf_type_is_void(type);
>>   }
>>   
>> +static bool bpf_ret_reg_pair(struct bpf_verifier_env *env, int subprog)
>> +{
>> +	const struct btf_type *type, *func, *func_proto;
>> +	const struct btf *btf = env->prog->aux->btf;
>> +	u32 btf_id;
>> +
>> +	if (!btf || !env->prog->aux->func_info)
>> +		return false;
>> +
>> +	btf_id = env->prog->aux->func_info[subprog].type_id;
>> +
>> +	func = btf_type_by_id(btf, btf_id);
>> +	if (!func)
>> +		return false;
> `if (!func) return false` checks here and below are not necessary
> after BTF validation.

Ack. Will remove them.

>
>> +
>> +	func_proto = btf_type_by_id(btf, func->type);
>> +	if (!func_proto)
>> +		return false;
>> +
>> +	type = btf_type_skip_modifiers(btf, func_proto->type, NULL);
>> +	if (!type)
>> +		return false;
>> +
>> +	if (btf_type_is_struct(type) || btf_type_is_scalar(type))
>> +		return type->size > 8 && type->size <= 16;
>> +
>> +	return false;
>> +}
>> +
>>   static const char *subprog_name(const struct bpf_verifier_env *env, int subprog)
>>   {
>>   	struct bpf_func_info *info;
>> @@ -9459,10 +9488,17 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>   		clear_caller_saved_regs(env, caller->regs);
>>   		invalidate_outgoing_stack_args(env, cur_func(env));
>>   
>> -		/* All non-void global functions return a 64-bit SCALAR_VALUE. */
>> +		/*
>> +		 * A non-void global function returns a 64-bit SCALAR_VALUE in
>> +		 * R0, or a >8 byte SCALAR_VALUE in the R0:R2 register pair.
>> +		 */
>>   		if (!subprog_returns_void(env, subprog)) {
>>   			mark_reg_unknown(env, caller->regs, BPF_REG_0);
>>   			caller->regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG;
>> +			if (bpf_ret_reg_pair(env, subprog)) {
>> +				mark_reg_unknown(env, caller->regs, BPF_REG_2);
>> +				caller->regs[BPF_REG_2].subreg_def = DEF_NOT_SUBREG;
>> +			}
>>   		}
>>   
>>   		if (env->subprog_info[subprog].might_throw) {
>> @@ -9825,6 +9861,13 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
>>   	} else {
>>   		/* return to the caller whatever r0 had in the callee */
>>   		caller->regs[BPF_REG_0] = *r0;
>> +		if (bpf_ret_reg_pair(env, callee->subprogno)) {
>> +			if (callee->regs[BPF_REG_2].type == PTR_TO_STACK) {
> Let's consolidate all such checks in one place and not copy-paste them.

Will try to consolidate them together.

>
>> +				verbose(env, "cannot return stack pointer to the caller\n");
>> +				return -EINVAL;
>> +			}
>> +			caller->regs[BPF_REG_2] = callee->regs[BPF_REG_2];
>> +		}
>>   	}
>>   
>>   	/* for callbacks like bpf_loop or bpf_for_each_map_elem go back to callsite,
>> @@ -10745,6 +10788,18 @@ 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);
>>   }
>>   
>> +static void mark_kfunc_ret_reg_size(struct bpf_verifier_env *env,
>> +				    struct bpf_reg_state *regs, u32 size)
>> +{
>> +	if (size > 8) {
>> +		mark_btf_func_reg_size(env, BPF_REG_0, 8);
>> +		mark_reg_unknown(env, regs, BPF_REG_2);
>> +		regs[BPF_REG_2].subreg_def = DEF_NOT_SUBREG;
>> +	} else {
>> +		mark_btf_func_reg_size(env, BPF_REG_0, size);
>> +	}
>> +}
> This function makes R2 handling very asymmetric compared to R0:
> - mark_reg_unknown() for r0 is done in check_kfunc_call()
> - DEF_NOT_SUBREG for r0 is done in mark_btf_func_reg_size()
>
> I think the code should be structured to keep R0 and R2 processing
> uniform.

Ack

>
>> +
>>   static bool is_kfunc_acquire(struct bpf_kfunc_call_arg_meta *meta)
>>   {
>>   	return meta->kfunc_flags & KF_ACQUIRE;
>> @@ -13208,7 +13263,22 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>   		if (meta.btf == btf_vmlinux && (meta.func_id == special_kfunc_list[KF_bpf_res_spin_lock] ||
>>   		    meta.func_id == special_kfunc_list[KF_bpf_res_spin_lock_irqsave]))
>>   			__mark_reg_const_zero(env, &regs[BPF_REG_0]);
>> -		mark_btf_func_reg_size(env, BPF_REG_0, t->size);
>> +		mark_kfunc_ret_reg_size(env, regs, t->size);
>> +	} else if (btf_type_is_struct(t)) {
>> +		/*
>> +		 * The returned struct comes back as raw register bits modeled
>> +		 * as an unknown scalar, so it must contain only scalars:
>> +		 * otherwise a pointer field would be laundered into a scalar
>> +		 * and escape provenance and reference tracking.
>> +		 */
>> +		if (!__btf_type_is_scalar_struct(env, desc_btf, t, 0)) {
>> +			verbose(env, "kernel function %s returns %s %s that is not composed of scalars\n",
>> +				func_name, btf_type_str(t),
>> +				btf_name_by_offset(desc_btf, t->name_off));
> Should this also check the size of the struct?

Later on, there is a size condition for struct (<= 16), so we should be okay here.

>
>> +			return -EINVAL;
>> +		}
>> +		mark_reg_unknown(env, regs, BPF_REG_0);
>> +		mark_kfunc_ret_reg_size(env, regs, t->size);
>>   	} else if (btf_type_is_ptr(t)) {
>>   		ptr_type = btf_type_skip_modifiers(desc_btf, t->type, &ptr_type_id);
>>   		err = check_special_kfunc(env, &meta, regs, insn_aux, ptr_type, desc_btf);
> [...]