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

On Mon, 2026-08-10 at 17:09 -0700, Yonghong Song wrote:

...

> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 79680f3b4c74..911d57ce2488 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -1447,6 +1447,8 @@ int bpf_jmp_offset(struct bpf_insn *insn);
>  struct bpf_iarray *bpf_insn_successors(struct bpf_verifier_env *env, u32 idx);
>  void bpf_fmt_stack_mask(char *buf, ssize_t buf_sz, u64 stack_mask);
>  bool bpf_subprog_is_global(const struct bpf_verifier_env *env, int subprog);
> +int bpf_get_kfunc_ret_size(const struct bpf_prog *prog, u32 func_id,
> +			   u16 btf_fd_idx, u8 *ret_size);
>  
>  int bpf_find_subprog(struct bpf_verifier_env *env, int off);
>  bool bpf_is_throw_kfunc(struct bpf_insn *insn);
> diff --git a/kernel/bpf/backtrack.c b/kernel/bpf/backtrack.c
> index 40bd04421a99..fc8ecad6f01b 100644
> --- a/kernel/bpf/backtrack.c
> +++ b/kernel/bpf/backtrack.c
> @@ -425,6 +425,15 @@ 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");
> +				/*
> +				 * a global subprog returning more than 8 bytes
> +				 * sets R2 as well. R2 is part of the args mask
> +				 * checked just below, so it has to be cleared
> +				 * here rather than next to R0.
> +				 */
> +				if (bt_is_reg_set(bt, BPF_REG_2) &&
> +				    bpf_ret_reg_pair(env, subprog))
> +					bt_clear_reg(bt, BPF_REG_2);

Nit: Tbh, I don't see a need in the bpf_ret_reg_pair() check.
     I'd call bt_clear_reg(r2) unconditionally.
     Don't find verifier_bug() justification all that compelling.

>  				/* r1-r5 are invalidated after subprog call,
>  				 * so for global func call it shouldn't be set
>  				 * anymore
> @@ -508,6 +517,19 @@ 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);
> +			/* a kfunc returning more than 8 bytes also sets R2 */
> +			if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL &&
> +			    bt_is_reg_set(bt, BPF_REG_2)) {
> +				u8 ret_size;
> +				int err;
> +
> +				err = bpf_get_kfunc_ret_size(env->prog, insn->imm, insn->off,
> +							     &ret_size);
> +				if (verifier_bug_if(err, env, "no kfunc desc for insn %d", idx))
> +					return -EFAULT;
> +				if (ret_size > 8)
> +					bt_clear_reg(bt, BPF_REG_2);
> +			}

And same here, too much code for bug detection that fired once or
twice in my memory.

>  			if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) {
>  				/* if backtracking was looking for registers R1-R5
>  				 * they should have been found already.
> @@ -522,7 +544,30 @@ 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 from_subprog_call, r0_precise, r2_precise = false;
> +
> +			/*
> +			 * BPF_EXIT in subprog or callback always returns
> +			 * right after the call instruction, so by checking
> +			 * whether the instruction at subseq_idx-1 is subprog
> +			 * call or not we can distinguish actual exit from
> +			 * *subprog* from exit from *callback*. In the former
> +			 * case, we need to propagate the precision of the
> +			 * return registers, if necessary. In the latter we
> +			 * never do that.
> +			 */
> +			from_subprog_call = subseq_idx - 1 >= 0 &&
> +					    bpf_pseudo_call(&env->prog->insnsi[subseq_idx - 1]);
> +			if (from_subprog_call && bt_is_reg_set(bt, BPF_REG_2)) {
> +				struct bpf_subprog_info *callee;
> +
> +				/* 'idx' is the exit insn, so it is in the callee */
> +				callee = bpf_find_containing_subprog(env, idx);
> +				if (verifier_bug_if(!callee, env,
> +						    "no subprog contains exit insn %d", idx))
> +					return -EFAULT;
> +				r2_precise = bpf_ret_reg_pair(env, callee - env->subprog_info);
> +			}

And here, if the verifier_bug check is ignored r0/r2 can be handled together.

>  
>  			/* Backtracking to a nested function call, 'idx' is a part of
>  			 * the inner frame 'subseq_idx' is a part of the outer frame.

...