Re: [PATCH bpf-next v7 05/10] bpf: Account R2 of register-pair returns in live register analysis

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The verifier fails to properly model execution for BPF subprograms and kfuncs returning >8 bytes (register pairs), specifically by not restoring R2 to the caller and not verifying R2 initialization.
--

--- Patch [5]: [PATCH bpf-next v7 05/10] bpf: Account R2 of register-pair returns in live register analysis ---
commit 1cab80ee79c6b836ae7862851a98688dd786f8f2
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Account R2 of register-pair returns in live register analysis

This commit modifies live register analysis to mark R2 as read at BPF_EXIT
when a subprogram returns a register pair, such as an __int128. This
prevents a callee's R2 from being considered dead and incorrectly pruned
from checkpointed states.

> diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
> index 74fc4b3f80d6e..71f998c6eb888 100644
> --- a/kernel/bpf/liveness.c
> +++ b/kernel/bpf/liveness.c

[ ... ]

> @@ -2191,7 +2193,7 @@ static void compute_insn_live_regs(struct bpf_verifier_env *env,
>  			break;
>  		case BPF_EXIT:
>  			def = 0;
> -			use = r0;
> +			use = ret_reg_pair ? (r0 | r2) : r0;

[Severity: High]
Does the verifier also need to be updated to properly model execution for
BPF subprograms and kfuncs returning > 8 bytes?

While this patch handles the liveness side, it looks like
kernel/bpf/verifier.c only copies or initializes BPF_REG_0 (and not
BPF_REG_2) to the caller's state in several places.

In prepare_func_exit():

		/* return to the caller whatever r0 had in the callee */
		bpf_diag_mod_begin(env, &caller->regs[BPF_REG_0], r0, BPF_DIAG_MOD_WRITE);
		caller->regs[BPF_REG_0] = *r0;

In check_func_call():

		/* All non-void global functions return a 64-bit SCALAR_VALUE. */
		if (!returns_void) {
			mark_reg_unknown(env, caller->regs, BPF_REG_0);
			bpf_diag_mod_end(env);
		}

In check_kfunc_call():

	if (btf_type_is_scalar(t)) {
		mark_reg_unknown(env, regs, BPF_REG_0);

And in check_global_subprog_return_code():

	if (subprog_returns_void(env, subprog))
		return 0;

	return check_global_ret_scalar_reg(env, BPF_REG_0);

If a program attempts to read the upper half (R2) of a 16-byte return value,
will it be rejected by the verifier with an R2 !read_ok error?

Additionally, for global subprograms returning 16-byte types, could the lack
of R2 initialization checks allow uninitialized memory to be passed back to
the kernel?

>  			break;
>  		case BPF_CALL:
>  			def = ALL_CALLER_SAVED_REGS;

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