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

On 8/8/26 12:45 PM, [email protected] wrote:
> 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?

This patch is a preparation patch, in the next commit, proper verifier support
(with returning R2) will be implemented.