Re: [PATCH bpf-next v6 02/10] bpf: Add helpers to describe the R0:R2 return register pair

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

On 8/16/26 10:17 PM, bot+bpf-ci@kernel.org wrote:
>> 8 but <=16 bytes). It introduces bpf_compute_subprog_ret_regs() to
> derive this from BTF prototypes.
>
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index bc2af02547fe..f70d5878fbff 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
>> @@ -819,6 +819,8 @@ struct bpf_subprog_info {
>>   	bool is_async_cb: 1;
>>   	bool is_exception_cb: 1;
>>   	bool args_cached: 1;
>> +	/* true if the return value is passed in the R0:R2 register pair */
>> +	bool ret_reg_pair: 1;
>>   	/* true if bpf_fastcall stack region is used by functions that can't be inlined */
>>   	bool keep_fastcall_stack: 1;
>>   	bool changes_pkt_data: 1;
>> @@ -1055,6 +1057,11 @@ static inline struct bpf_subprog_info *subprog_info(struct bpf_verifier_env *env
>>   	return &env->subprog_info[subprog];
>>   }
>>
>> +static inline bool bpf_ret_reg_pair(struct bpf_verifier_env *env, int subprog)
>> +{
>> +	return subprog_info(env, subprog)->ret_reg_pair;
>> +}
>> +
> [ ... ]
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index b3c474ba7140..f1f1268d29c6 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
> [ ... ]
>
>> @@ -385,27 +385,70 @@ bool bpf_subprog_is_global(const struct bpf_verifier_env *env, int subprog)
>>   	return aux && aux[subprog].linkage == BTF_FUNC_GLOBAL;
>>   }
>>
>> -static bool subprog_returns_void(struct bpf_verifier_env *env, int subprog)
>> +static const struct btf_type *subprog_ret_type(struct bpf_verifier_env *env, int subprog)
>>   {
>> -	const struct btf_type *type, *func, *func_proto;
>> +	const struct btf_type *func, *func_proto;
>>   	const struct btf *btf = env->prog->aux->btf;
>>   	u32 btf_id;
>>
>> +	if (!btf || !env->prog->aux->func_info)
>> +		return NULL;
>> +
>>   	btf_id = env->prog->aux->func_info[subprog].type_id;
>>
>> +	/* Both already validated by prepare_btf_func() at prog load. */
>>   	func = btf_type_by_id(btf, btf_id);
>> -	if (verifier_bug_if(!func, env, "btf_id %u not found", btf_id))
>> -		return false;
>> -
>>   	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;
>> +	return btf_type_skip_modifiers(btf, func_proto->type, NULL);
>> +}
>>
>> -	return btf_type_is_void(type);
>> +static bool subprog_returns_void(struct bpf_verifier_env *env, int subprog)
>> +{
>> +	const struct btf_type *type = subprog_ret_type(env, subprog);
>> +
>> +	return type && btf_type_is_void(type);
>> +}
>> +
>> +static u32 ret_regs_cnt(u32 size)
>> +{
>> +	return size > 8 && size <= 16 ? 2 : 1;
>> +}
>> +
>> +static int bpf_compute_subprog_ret_regs(struct bpf_verifier_env *env)
>> +{
>> +	const struct btf *btf = env->prog->aux->btf;
>> +	const struct btf_type *type;
>> +	int subprog;
>> +	u32 size;
>> +
>> +	if (!env->prog->jit_requested || bpf_prog_is_offloaded(env->prog->aux))
>> +		return 0;
>> +
>> +	/*
>> +	 * 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++) {
>> +		type = subprog_ret_type(env, subprog);
>> +		/*
>> +		 * This runs before btf_validate_return_type(), so apply the same
>> +		 * type filter here.
>> +		 */
>> +		if (!type || !(btf_type_is_struct(type) || btf_type_is_scalar(type)))
>> +			continue;
> Does the comment's claim about applying "the same type filter" hold?
>
> btf_validate_return_type() accepts void, int, or enum types via:
>
> kernel/bpf/btf.c:btf_validate_return_type() {
>      /* We always accept void or scalars. */
>      if (btf_type_is_void(t) || btf_type_is_int(t) || btf_is_any_enum(t))
>          return 0;
>
>      return -EOPNOTSUPP;
> }
>
> The new filter instead accepts struct/union or scalar (int or enum, but
> not ENUM64):
>
>      if (!type || !(btf_type_is_struct(type) || btf_type_is_scalar(type)))
>
> btf_type_is_scalar() is defined as btf_type_is_int() || btf_type_is_enum(),
> and btf_type_is_enum() excludes BTF_KIND_ENUM64.
>
> The filters differ in several ways. btf_validate_return_type() accepts
> void and ENUM64 (via btf_is_any_enum()) but rejects struct/union at this
> commit. The new filter accepts struct/union but filters out void and
> ENUM64.
>
> The divergence is harmless since every dropped type is at most 8 bytes and
> would need only one register anyway. But the comment implies an equivalence
> that does not exist, and specifically suggests structs are validated
> elsewhere when btf_validate_return_type() at this commit still rejects
> them with -EOPNOTSUPP.
>
> Would describing what the filter actually needs be clearer? Something like
> "only struct/union and integer/enum returns can exceed 8 bytes and need
> sizing" rather than claiming parity with btf_validate_return_type()?

You are right. I missed enum64. I think the above 'if' condition should be
   
  if (!type || !(btf_type_is_struct(type) || btf_type_is_int(type) || btf_is_any_enum(type)))

>
>> +		if (verifier_bug_if(IS_ERR(btf_resolve_size(btf, type, &size)), env,
>> +				    "cannot size return type of subprog %d", subprog))
>> +			return -EFAULT;
>> +		if (ret_regs_cnt(size) > 1) {
>> +			subprog_info(env, subprog)->ret_reg_pair = true;
>> +			env->prog->jit_required = 1;
>> +		}
>> +	}
>> +
>> +	return 0;
>>   }
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31995069373