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

[sashiko-bot@kernel.org]

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 <yonghong.song@linux.dev>

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?

> +		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;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819055239.3293449-1-yonghong.song@linux.dev?part=2