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

On 8/8/26 12:29 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 comment style.
> --
>
> --- Patch [4]: [PATCH bpf-next v3 04/13] bpf: Track R2 of register-pair returns in precision backtracking ---
> commit 031a41deb76f72f449ddf5239bbf769a2cbe7e5c
> Author: Yonghong Song <[email protected]>
>
> bpf: Track R2 of register-pair returns in precision backtracking
>
> This commit updates the BPF verifier's precision backtracking to correctly
> handle R2 for functions returning values larger than 8 bytes. It ensures
> R2 is cleared from or added to the register mask across function call
> boundaries to prevent unexpected register mask verification failures.
>
>> diff --git a/kernel/bpf/backtrack.c b/kernel/bpf/backtrack.c
>> index 40bd04421a991..9b112bf427a57 100644
>> --- a/kernel/bpf/backtrack.c
>> +++ b/kernel/bpf/backtrack.c
> [ ... ]
>> @@ -425,6 +425,14 @@ 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.
>> +				 */
> [Severity: Low]
> This isn't a bug, but does this multi-line comment follow the preferred
> BPF subsystem style? The guidelines specify that the opening /* should be
> on its own line rather than sharing a line with the text.
>
>> +				if (bt_is_reg_set(bt, BPF_REG_2) &&
>> +				    bpf_ret_reg_pair(env, subprog))
>> +					bt_clear_reg(bt, BPF_REG_2);
> [ ... ]
>> @@ -522,7 +543,29 @@ 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
> [Severity: Low]
> This is also not a bug, but since this comment is being heavily modified,
> could it be updated to place the opening /* on a separate line to match
> the modern BPF comment style?
>
>> +			 * case, we need to propagate the precision of the
>> +			 * return registers, if necessary. In the latter we
>> +			 * never do that.
>> +			 */

Comment format issue. Will fix in the next revision.