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

On 8/18/26 11:17 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Extension programs (BPF_PROG_TYPE_EXT) are mishandled because `bpf_compute_subprog_ret_regs` skips subprog 0, failing to set up `ret_reg_pair`.
> --
>
> --- Patch [2]: [PATCH bpf-next v7 02/10] bpf: Add helpers to describe the R0:R2 return register pair ---
> commit f6c1f4f14c6e8bda9545fe1b82f42648b4f3c475
> Author: Yonghong Song <[email protected]>
>
> bpf: Add helpers to describe the R0:R2 return register pair
>
> This commit adds bpf_ret_reg_pair() to determine if a BPF subprogram returns
> a value in the R0:R2 register pair. It implements
> bpf_compute_subprog_ret_regs() to compute this flag based on the BTF
> prototype of each subprogram, intentionally skipping the main program.
>
> Link: https://github.com/llvm/llvm-project/pull/190894
> Link: https://github.com/llvm/llvm-project/pull/206876
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index d8bf897460eae..2b5e88fdda07e 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
> [ ... ]
>
>> +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++) {
> [Severity: High]
> Does this code mishandle extension programs (BPF_PROG_TYPE_EXT)?
>
> The comment mentions that bpf_check_attach_target() refuses to replace a
> function returning more than 8 bytes. However, for extension programs,
> prog->expected_attach_type is 0, so attach_uses_trampoline_retval(0) evaluates
> to false, bypassing the size check.
>
> If an extension program successfully attaches and replaces a subprogram
> returning an __int128 (16 bytes), skipping subprog 0 here means ret_reg_pair
> won't be set. Could this result in the verifier failing to enforce the
> initialization of R2, potentially leaking uninitialized register contents to
> the caller?

We should be okay. For freplace program, in later commit
   bpf: Add verifier support for 16-byte returns in R0:R2
freplace will get rejected if kernel function returns
R0:R2 values. If the freplace is for original subprog 0,
the R2 (if to-be-returned in freplace) may cause incorrect
result but it is user's problem.

>
>> +		type = subprog_ret_type(env, subprog);
>> +		if (!type || btf_type_is_void(type))
>> +			continue;
>> +		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;
>> +}