Re: [PATCH bpf-next 04/12] bpf: Track R2 of register-pair returns in precision backtracking

[Eduard Zingerman <eddyz87@gmail.com>]

On Wed, 2026-07-08 at 13:10 -0700, Yonghong Song wrote:
> A function returning a value larger than 8 bytes (a struct/union, or an
> __int128) uses R2 as a second return register alongside R0. Previous
> precision backtracking only treated R0 as a return register at a
> call/return boundary, so marking the second half of such a return precise
> would trip the "unexpected regs" checks in backtrack_insn() and reject a
> valid program with -EFAULT.
> 
> Handle R2 like R0 in the three boundaries where a call defines the return
> registers:
> 
>  - static subprog exit (BPF_EXIT): R2 is a return register, not a leftover
>    argument; clear it before the args check and propagate its precision
>    into the callee.
>  - global subprog call: a global subprog returning >8 bytes also sets R2;
>    clear it before the args check.
>  - kfunc call (BPF_CALL): a kfunc returning >8 bytes (model ret_size > 8)
>    also sets R2; clear it like R0.
> 
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

> diff --git a/kernel/bpf/backtrack.c b/kernel/bpf/backtrack.c
> index 2e4ae0ef0860..c46c2778bf60 100644
> --- a/kernel/bpf/backtrack.c
> +++ b/kernel/bpf/backtrack.c
> @@ -424,6 +424,8 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
>  				 */
>  				verifier_bug_if(idx + 1 != subseq_idx, env,
>  						"extra insn from subprog");
> +				if (bpf_ret_reg_pair(env, subprog))
> +					bt_clear_reg(bt, BPF_REG_2);

Nit: please move this a few lines below, so that r0 and r2 are handled nearby.

>  				/* r1-r5 are invalidated after subprog call,
>  				 * so for global func call it shouldn't be set
>  				 * anymore
> @@ -507,6 +509,12 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
>  				return -ENOTSUPP;
>  			/* regular helper call sets R0 */
>  			bt_clear_reg(bt, BPF_REG_0);
> +			/* On error the return is negative, R2 stays set, and the
> +			 * check below catches it as an unexpected register.
> +			 */

Nit: I don't understand the above comment (the code is fine). Maybe drop it altogether?

> +			if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL &&
> +			    bpf_get_kfunc_ret_size(env->prog, insn->imm, insn->off) > 8)
> +				bt_clear_reg(bt, BPF_REG_2);
>  			if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) {
>  				/* if backtracking was looking for registers R1-R5
>  				 * they should have been found already.
> @@ -521,7 +529,25 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
>  					return -EFAULT;
>  			}
>  		} else if (opcode == BPF_EXIT) {
> -			bool r0_precise;
> +			bool r0_precise, r2_precise = false;
> +
> +			/* A subprog returning a value larger than 8 bytes (a
> +			 * struct/union or an __int128) uses R2 as a second
> +			 * return register alongside R0. When exiting such a
> +			 * subprog, R2 is a return register rather than a
> +			 * leftover argument, so handle it like R0 below.
> +			 */

Nit: the comment is too verbose.

> +			if (subseq_idx - 1 >= 0 &&
> +			    bpf_pseudo_call(&env->prog->insnsi[subseq_idx - 1])) {
> +				int call_idx = subseq_idx - 1;
> +				int tgt = call_idx + env->prog->insnsi[call_idx].imm + 1;
> +				int subprog = bpf_find_subprog(env, tgt);
> +
> +				if (subprog >= 0 &&
> +				    bpf_ret_reg_pair(env, subprog) &&
> +				    bt_is_reg_set(bt, BPF_REG_2))
> +					r2_precise = true;
> +			}
>  
>  			/* Backtracking to a nested function call, 'idx' is a part of
>  			 * the inner frame 'subseq_idx' is a part of the outer frame.

[...]