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

On 8/12/26 2:16 PM, Eduard Zingerman wrote:
> 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.

Okay, will remove the above 'if' condition.

>
>>   				/* 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.

Will remove verifier_bug_if().

>
>>   			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.

Yes, will remove verifier_bug_if().

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