Re: [PATCH bpf-next v3 06/13] bpf: Reject callbacks returning more than 8 bytes

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] check_global_subprog_return_code() fails to validate BPF_REG_2 for global subprograms returning > 8 bytes, allowing kernel pointer leaks.
- [High] BPF-to-BPF calls returning 16-byte values are broken because prepare_func_exit() only copies R0 to the caller, leaving R2 uninitialized.
- [High] BPF programs cannot use 16-byte returns from kfuncs because check_kfunc_call() fails to initialize BPF_REG_2.
--

--- Patch [6]: [PATCH bpf-next v3 06/13] bpf: Reject callbacks returning more than 8 bytes ---
commit d03b8255604638dcb72d6dda989b79be64af9fa4
Author: Yonghong Song <[email protected]>

bpf: Reject callbacks returning more than 8 bytes

This patch prevents BPF callbacks passed to helpers, kfuncs, or as exception
callbacks from returning more than 8 bytes. Since the callback prototypes
yield a single u64 in R0, returning a 16-byte value would cause the upper
half to be silently dropped. The verifier now rejects such callbacks at
load time in check_ld_imm() and do_check_common().

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -16306,6 +16306,11 @@ static int check_ld_imm(struct bpf_verifier_env *env, struct bpf_insn *insn)
>  			verbose(env, "callback function not static\n");
>  			return -EINVAL;
>  		}
> +		if (bpf_ret_reg_pair(env, subprogno)) {
> +			verbose(env,
> +				"callback function with >8-byte return value is not supported\n");
> +			return -EINVAL;
> +		}
>  
>  		dst_reg->type = PTR_TO_FUNC;
>  		dst_reg->subprogno = subprogno;

[ ... ]

> @@ -18528,6 +18533,12 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
>  				ret = -EINVAL;
>  				goto out;
>  			}
> +			if (bpf_ret_reg_pair(env, subprog)) {
> +				verbose(env,
> +					"exception cb cannot return value larger than 8 bytes\n");
> +				ret = -EINVAL;
> +				goto out;
> +			}
>  
>  			/* Also ensure the callback only has a single scalar argument. */
>  			if (sub->arg_cnt != 1 || sub->args[0].arg_type != ARG_ANYTHING) {


While reviewing the support for 16-byte return values, I noticed a few other
places in kernel/bpf/verifier.c that might need updates:

In kernel/bpf/verifier.c:check_global_subprog_return_code():

> 	return check_global_ret_scalar_reg(env, BPF_REG_0);

[Severity: High]
Does check_global_subprog_return_code() fail to validate BPF_REG_2 for global
subprograms returning more than 8 bytes?

A global subprogram returning a 16-byte value (e.g. __int128) could place a
kernel pointer in BPF_REG_2 and exit without the verifier rejecting it,
bypassing the verifier's pointer leak prevention.

Although currently unexploitable because callers have R2 marked uninitialized
due to another issue, if the caller issue is fixed, or if the kernel reads
the return value directly (e.g. via BPF trampoline), can this missing
validation become an exploitable KASLR bypass or pointer leak?


In kernel/bpf/verifier.c:prepare_func_exit():

> 	/* return to the caller whatever r0 had in the callee */
> 	caller->regs[BPF_REG_0] = *r0;

[Severity: High]
Are BPF-to-BPF calls returning 16-byte values broken here?

This only copies R0 to the caller, leaving R2 uninitialized in the caller's
frame. BPF programs calling static subprograms that return 16-byte values
cannot use the upper 64 bits of the return value, as the verifier will reject
the program with an uninitialized register error.

Additionally, precision backtracking logic added for R2 on static subprog exit
appears to be rendered unreachable dead code because R2 can never be marked
precise by the caller.


In kernel/bpf/verifier.c:check_kfunc_call():

> 	if (btf_type_is_scalar(t)) {
> 		mark_reg_unknown(env, regs, BPF_REG_0);

[Severity: High]
Can BPF programs actually use 16-byte returns from kfuncs?

This handles scalar return types from kfuncs but only initializes R0, leaving
R2 uninitialized.

Since caller-saved registers (including R2) are cleared before the call, any
BPF program attempting to use this new feature will be rejected by the
verifier when attempting to read R2. Should this also mark R2 as initialized
for 16-byte scalar returns?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6