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

> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -814,6 +814,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;
> @@ -1048,6 +1050,13 @@ static inline struct bpf_subprog_info *subprog_info(struct bpf_verifier_env *env, int subprog)
>  	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;
> +}
> +
> +bool bpf_kfunc_ret_reg_pair(struct bpf_verifier_env *env, struct bpf_insn *insn);
> +
>  struct bpf_call_summary {
>  	u8 num_params;
>  	bool is_void;
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -382,29 +382,62 @@ 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 void 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;
> +
> +	for (subprog = 0; subprog < env->subprog_cnt; subprog++) {

Does this loop need to start at subprog 0?  The main program (subprog 0)
delivers its exit code in R0 alone and never uses the R0:R2 calling
convention, so checking its BTF return type can incorrectly trigger
jit_required even though no R2 propagation is needed.

> +		type = subprog_ret_type(env, subprog);
> +		if (!type || !(btf_type_is_struct(type) || btf_type_is_scalar(type)))
> +			continue;
> +		if (IS_ERR(btf_resolve_size(btf, type, &size)))
> +			continue;
> +		if (ret_regs_cnt(size) > 1) {
> +			subprog_info(env, subprog)->ret_reg_pair = true;
> +			/*
> +			 * The R0:R2 return convention is only implemented in
> +			 * the JIT: the interpreter propagates BPF_R0 alone out
> +			 * of a subprogram, so a caller reading R2 would see a
> +			 * stale value.
> +			 */
> +			env->prog->jit_required = 1;
> +		}
> +	}
> +}

Additionally, should jit_required be set for subprogs whose BTF prototypes
are marked unreliable?  btf_check_subprog_call() can flag a mismatch with
prog->aux->func_info_aux[subprog].unreliable = true, and check_func_call()
only aborts on -EFAULT, so a static callee with an aggregate return can
load today without using the BTF return type. Subprogs subsequently deleted
by bpf_opt_remove_dead_code() (which runs after this pass) also don't need
jit_required set.

This changes behavior for existing programs: on CONFIG_BPF_JIT=n or
net.core.bpf_jit_enable=0, such programs would now fail in
bpf_fixup_call_args() with "program requires BPF JIT compiler but it is
not available", whereas they previously verified and ran on the
interpreter.

[ ... ]

> @@ -20333,6 +20381,9 @@
>  	if (ret < 0)
>  		goto skip_full_check;
>
> +	/* must precede the first bpf_ret_reg_pair() user below */
> +	bpf_compute_subprog_ret_regs(env);
> +
>  	ret = bpf_compute_live_registers(env);
>  	if (ret < 0)
>  		goto skip_full_check;


---
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/31740414277