Re: [PATCH bpf-next v6 05/10] bpf: Account R2 of register-pair returns in live register analysis

On 8/16/26 9:43 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Critical] When a BPF program with 16-byte returns falls back to the interpreter (e.g., if JIT is disabled), the interpreter silently discards the upper 8 bytes (R2) of the return value, causing runtime data corruption.
> - [High] The verifier fails to propagate or initialize BPF_REG_2 back to the caller for subprograms and kfuncs returning 16-byte values, rendering the feature unusable.
> - [High] The verifier fails to validate the safety of BPF_REG_2 at the exit of global subprograms returning 16-byte values, allowing potential pointer leaks or returning uninitialized memory.
> --
>
> --- Patch [5]: [PATCH bpf-next v6 05/10] bpf: Account R2 of register-pair returns in live register analysis ---
> commit 1e84baf022beac38fd11fa1a392f1c5622154586
> Author: Yonghong Song <[email protected]>
>
> bpf: Account R2 of register-pair returns in live register analysis
>
> This commit updates the live register analysis to account for subprograms
> returning a 16-byte register pair. It modifies compute_insn_live_regs()
> to mark BPF_REG_2 as read during BPF_EXIT when a subprogram returns a
> register pair.
>
>> diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
>> index 74fc4b3f80d6e..71f998c6eb888 100644
>> --- a/kernel/bpf/liveness.c
>> +++ b/kernel/bpf/liveness.c
> [ ... ]
>> @@ -2191,7 +2193,7 @@ static void compute_insn_live_regs(struct bpf_verifier_env *env,
>>   			break;
>>   		case BPF_EXIT:
>>   			def = 0;
>> -			use = r0;
>> +			use = ret_reg_pair ? (r0 | r2) : r0;
>>   			break;
> [Severity: High]
> How does the verifier propagate BPF_REG_2 back to the caller for subprograms
> and kfuncs returning 16-byte values?
>
> In kernel/bpf/verifier.c:prepare_func_exit(), only BPF_REG_0 is copied from
> the callee's state to the caller's state:
>
> kernel/bpf/verifier.c:prepare_func_exit() {
> 	...
> 	/* return to the caller whatever r0 had in the callee */
> 	bpf_diag_mod_begin(env, &caller->regs[BPF_REG_0], r0, BPF_DIAG_MOD_WRITE);
> 	caller->regs[BPF_REG_0] = *r0;
> 	bpf_diag_mod_end(env);
> 	...
> }
>
> Similarly, when returning from a non-void global subprogram or a kfunc, only
> BPF_REG_0 is initialized as a scalar. For instance, in check_func_call():
>
> kernel/bpf/verifier.c:check_func_call() {
> 	...
> 	/* All non-void global functions return a 64-bit SCALAR_VALUE. */
> 	if (!returns_void) {
> 		mark_reg_unknown(env, caller->regs, BPF_REG_0);
> 		bpf_diag_mod_end(env);
> 	}
> 	...
> }
>
> And in check_kfunc_call():
>
> kernel/bpf/verifier.c:check_kfunc_call() {
> 	...
> 	if (btf_type_is_scalar(t)) {
> 		mark_reg_unknown(env, regs, BPF_REG_0);
> 	...
> }
>
> Since clear_caller_saved_regs() marks R1-R5 as NOT_INIT prior to the call,
> would this leave the caller's BPF_REG_2 uninitialized, causing the verifier
> to reject legitimate reads of the upper 8 bytes of the return value?

This should be okay. The BPF_REG_2 is introduced in later patch
   bpf: Add verifier support for 16-byte returns in R0:R2

>
> [Severity: High]
> Is the safety of BPF_REG_2 validated at the exit of global subprograms
> returning 16-byte values?
>
> In kernel/bpf/verifier.c:check_global_subprog_return_code(), the return code
> validation is only performed for BPF_REG_0:
>
> kernel/bpf/verifier.c:check_global_subprog_return_code() {
> 	...
> 	return check_global_ret_scalar_reg(env, BPF_REG_0);
> }
>
> Could this omit BPF_REG_2 for register-pair returns, potentially allowing a
> global subprogram to return an unreferenced pointer, a map value pointer, or
> uninitialized memory without being caught by the verifier?

The same as above. The BPF_REG_2 is introduced in later patch
   bpf: Add verifier support for 16-byte returns in R0:R2


>
> [Severity: Critical]
> What happens if a BPF program with 16-byte returns falls back to the
> interpreter?
>
> If JIT is not requested, bpf_compute_subprog_ret_regs() exits early without
> setting env->prog->jit_required = 1:
>
> kernel/bpf/verifier.c:bpf_compute_subprog_ret_regs() {
> 	...
> 	if (!env->prog->jit_requested || bpf_prog_is_offloaded(env->prog->aux))
> 		return 0;
> 	...
> }
>
> This permits the program to fall back to the interpreter. However, the
> interpreter handles subprogram calls via JMP_CALL_ARGS, which only propagates
> BPF_R0:
>
> kernel/bpf/core.c:___bpf_prog_run() {
> 	...
> 	JMP_CALL_ARGS:
> 		BPF_R0 = interpreters_args[insn->off](BPF_R1, BPF_R2, BPF_R3,
> 						      BPF_R4, BPF_R5,
> 						      insn + insn->imm + 1);
> 		CONT;
> 	...
> }
>
> Since the PROG_NAME_ARGS macro generates functions that return a single u64,
> does the interpreter silently discard the upper 8 bytes (R2) of the return
> value, causing data corruption at runtime?
>
>>   		case BPF_CALL:
>>   			def = ALL_CALLER_SAVED_REGS;

In such case (jit not requested), verifier will fail. For example,
suppose the subprog will return R0:R2, but in interpreter mode, only
R0 is returned. R2 is not initialized. In caller, when trying to
return R2, but R2 is not initialized, so verifier will fail.