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

On 8/13/26 1:49 PM, [email protected] wrote:
>> diff --git a/kernel/bpf/backtrack.c b/kernel/bpf/backtrack.c
>> index a2b18a9f1694..0c8e05a7e175 100644
>> --- a/kernel/bpf/backtrack.c
>> +++ b/kernel/bpf/backtrack.c
>> @@ -423,6 +423,16 @@ 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 clear it here rather
>> +				 * than next to R0. Only a subprog that does
>> +				 * return a pair defines R2, so leave the mask
>> +				 * alone otherwise and let the check below catch
>> +				 * an R2 that has no business being set.
>> +				 */
>> +				if (bpf_ret_reg_pair(env, subprog))
>> +					bt_clear_reg(bt, BPF_REG_2);
> [ ... ]
>
>> @@ -506,6 +516,17 @@ 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.
>> +			 * R2 is part of the args mask checked just below, so
>> +			 * clear it here rather than next to R0. The prototype
>> +			 * lookup is only worth doing when R2 is requested at
>> +			 * all; any other call leaves R2 uninitialized, so a
>> +			 * request for it is caught by the check below.
>> +			 */
>> +			if (bt_is_reg_set(bt, BPF_REG_2) &&
>> +			    insn->src_reg == BPF_PSEUDO_KFUNC_CALL &&
>> +			    bpf_kfunc_ret_reg_pair(env, insn))
>> +				bt_clear_reg(bt, BPF_REG_2);
> [ ... ]
>
>> @@ -520,7 +541,41 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
>>   					return -EFAULT;
>>   			}
>>   		} else if (opcode == BPF_EXIT) {
> [ ... ]
>
>> +			r0_precise = from_subprog_call && bt_is_reg_set(bt, BPF_REG_0);
>> +			r2_precise = false;
>> +			if (from_subprog_call && bt_is_reg_set(bt, BPF_REG_2)) {
>> +				call = &env->prog->insnsi[subseq_idx - 1];
>> +				subprog = bpf_find_subprog(env, subseq_idx + call->imm);
>> +				if (subprog < 0)
>> +					return -EFAULT;
>> +				/* Only a callee that does return a pair defines
>> +				 * R2. Leave the mask alone otherwise, so that
>> +				 * the check below still catches an R2 that has
>> +				 * no business being set.
>> +				 */
>> +				r2_precise = bpf_ret_reg_pair(env, subprog);
>> +			}
> [ ... ]
>
>> @@ -533,30 +588,27 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
>>   			if (subseq_idx >= 0 && bpf_calls_callback(env, subseq_idx))
>>   				for (i = BPF_REG_1; i <= BPF_REG_5; i++)
>>   					bt_clear_reg(bt, i);
>> +
>> +			/* a callee returning more than 8 bytes sets R2 as well;
>> +			 * R2 is part of the args mask checked just below, so
>> +			 * clear it here rather than next to R0.
>> +			 */
>> +			if (r2_precise)
>> +				bt_clear_reg(bt, BPF_REG_2);
> This isn't a bug, but the "R2 is part of the args mask checked just
> below, so clear it here rather than next to R0" rationale appears at
> each of the three call boundaries (global subprog, kfunc, and BPF_EXIT).
> Would stating it once - say at the first site, or in a note above the
> BPF_EXIT block - and keeping the other two to a one-liner read more
> easily?

Eduard suggested to remove the condition "r2_precise" for bt_clear_reg(bt, BPF_REG_2)
which should solve the problem.

>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31740414277