Re: [PATCH bpf-next v6 04/10] bpf: Handle R2 as a return register in precision backtracking

On 8/16/26 10:17 PM, [email protected] wrote:
>> bpf: Handle R2 as a return register in precision backtracking
>>
>> Precision backtracking treats only R0 as a return register at a
>> call/return boundary, so once the verifier starts modeling R2 that way,
>> marking the second half of such a return precise would trip the
>> "unexpected regs" checks in backtrack_insn() and reject a valid
>> program.
>>
>> Marking the upper half precise, for example by branching on it after a
>> call to a static subprogram, walks backtracking into the callee and
>> reaches its BPF_EXIT with R2 still set in the mask. Handle R2 like R0
>> in boundaries where a call defines the return registers.
>>
>> R2 differs from R0 in that it is an argument register as well, so it is
>> part of the BPF_REGMASK_ARGS check and has to be cleared before that check
>> rather than next to R0. Clear it unconditionally, rather than only where
>> the callee or the kfunc really does return a pair. That gives up the
>> "unexpected regs" assertion for R2, and in exchange keeps backtracking
>> free of any BTF lookup. Nothing is lost: a callee that does not return
>> a pair leaves the caller's R2 uninitialized, so the main verification
>> pass has already rejected any program that reads it, and backtracking
>> is never asked for its precision.
>>
>> At BPF_EXIT the return registers are sampled before the callback path
>> clears R1-R5. That clear does not touch R0, but it does cover R2, and
>> running it first would drop a pair return whenever the instruction
>> following the call happens to be one that invokes a callback.
>>
>> Suggested-by: Eduard Zingerman <[email protected]>
>> Signed-off-by: Yonghong Song <[email protected]>
> This looks like a bug fix for 16-byte return support. Should this
> include:
>
>    Fixes: 162762023c59 ("bpf: Add helpers to describe the R0:R2 return register pair")

This Fixes tag is not needed, esp. it is in the same series.

>
>> diff --git a/kernel/bpf/backtrack.c b/kernel/bpf/backtrack.c
>> index a2b18a9f1694..653db80bcc47 100644
>> --- a/kernel/bpf/backtrack.c
>> +++ b/kernel/bpf/backtrack.c
> [ ... ]
>
>> @@ -520,7 +524,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 from_subprog_call, r0_precise, r2_precise;
>> +
>> +			/* 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]);
>> +
>> +			/* Sample the return registers before the callback
>> +			 * handling below clears R1-R5.
>> +			 */
>> +			r0_precise = from_subprog_call && bt_is_reg_set(bt, BPF_REG_0);
>> +			r2_precise = from_subprog_call && bt_is_reg_set(bt, BPF_REG_2);
> A subsystem pattern flags this as potentially concerning: the multi-line
> comment above starts its text on the opening /* line:
>
> 	/* Sample the return registers before the callback
> 	 * handling below clears R1-R5.
> 	 */
>
> The BPF subsystem guide suggests putting /* on its own line for
> multi-line comments under kernel/bpf/:
>
> 	/*
> 	 * Sample the return registers before the callback
> 	 * handling below clears R1-R5.
> 	 */
>
> Every existing multi-line comment in backtrack.c uses the older form,
> including the comment block that this same hunk moves, so matching the
> file's dominant style is a plausible reason to write it this way. Does
> the newer form matter here?

I think it is okay. The new comment format is from the old one.

>
> [ ... ]
>
> ---
> 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/31995069373