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

[sashiko-bot@kernel.org]

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 <yonghong.song@linux.dev>

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?

[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?

[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;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817042141.2286086-1-yonghong.song@linux.dev?part=5