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

On 8/18/26 11:03 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Multi-line comments do not follow the required BPF subsystem style.
> --
>
> --- Patch [4]: [PATCH bpf-next v7 04/10] bpf: Handle R2 as a return register in precision backtracking ---
> commit 827448f59c95b01086f128d7ec2894a154de5092
> Author: Yonghong Song <[email protected]>
>
> bpf: Handle R2 as a return register in precision backtracking
>
> This commit updates precision backtracking to treat R2 as a return register
> alongside R0 at call/return boundaries. It clears R2 unconditionally when
> evaluating subprograms or kfuncs, and samples it before callback paths clear
> R1-R5.
>
>> diff --git a/kernel/bpf/backtrack.c b/kernel/bpf/backtrack.c
>> index a2b18a9f1694c..653db80bcc47b 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
> [Severity: Low]
> This isn't a bug, but does this multi-line comment follow the preferred BPF
> subsystem style? The opening /* is usually required to be on its own line.
>
>> +			 * 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
> [Severity: Low]
> This isn't a bug either, but the same formatting question applies here. Should
> the opening /* be on its own line for multi-line comments?

We should be okay here.

>
>> +			 * 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);