Re: [PATCH bpf-next 08/12] bpf: Enable 16-byte aggregate return types

[Yonghong Song <yonghong.song@linux.dev>]

On 7/8/26 1:28 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Leaking uninitialized kernel memory via kfunc R2 return register for structs of 9-15 bytes.
> --
>
> --- Patch [8]: [PATCH bpf-next 08/12] bpf: Enable 16-byte aggregate return types ---
> commit 5b4b6cb320ef1b1f82ac557b6f90945992d8a0c5
> Author: Yonghong Song <yonghong.song@linux.dev>
> Subject: bpf: Enable 16-byte aggregate return types
>
> This patch relaxes btf_distill_func_proto() and btf_validate_return_type()
> to allow kfuncs to return by-value structs or unions up to 16 bytes.
> This enables the <=16 byte aggregate return values end to end.
>
>> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
>> index dff5c0d916412..bab82d1c8cb98 100644
>> --- a/kernel/bpf/btf.c
>> +++ b/kernel/bpf/btf.c
>> @@ -7572,7 +7572,12 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
>>   		return -EINVAL;
>>   	}
>>   	ret = __get_type_size(btf, func->type, &t);
>> -	if (ret < 0 || btf_type_is_struct(t)) {
>> +	/*
>> +	 * __get_type_size() already restricts a non-negative ret to void, a
>> +	 * pointer, an int, an enum or a struct/union, so only the size is checked
>> +	 * here.
>> +	 */
>> +	if (ret < 0 || ret > 16) {
> [Severity: High]
> Can this leak uninitialized kernel memory for structs between 9 and 15 bytes?
>
> The x86_64 ABI leaves the unused upper bytes of the %rdx register undefined
> when returning a struct that is not a multiple of 8 bytes.
>
> Looking at mark_kfunc_ret_reg_size() in kernel/bpf/verifier.c, if the struct
> size is greater than 8:
>
> 	if (size > 8) {
> 		mark_btf_func_reg_size(env, BPF_REG_0, 8);
> 		mark_reg_unknown(env, regs, BPF_REG_2);
> 		regs[BPF_REG_2].subreg_def = DEF_NOT_SUBREG;
> 	}
>
> The verifier explicitly marks BPF_REG_2 as fully initialized, bypassing subreg
> zero-extension.
>
> And in the x86 JIT do_jit() in arch/x86/net/bpf_jit_comp.c, it moves the full
> 64-bit register:
>
> 				fm = bpf_jit_find_kfunc_model(bpf_prog, insn);
> 				if (fm && fm->ret_size > 8)
> 					emit_mov_reg(&prog, true, BPF_REG_2, BPF_REG_3);
>
> Could a BPF program read the undefined bytes from R2, resulting in an
> information leak?

We should be fine here. See llvm patch https://reviews.llvm.org/D131598
which specifically tries to address this issue. The following is an example
for R2:

$ cat t.c
struct foo { long a; short b; };
struct foo bar(void);

int tar(void) {
   struct foo t = bar();
   return t.a + t.b;
}
$ clang --target=bpf -O2 -S t.c
$ cat t.s
         .file   "t.c"
         .text
         .globl  tar                             # -- Begin function tar
         .p2align        3
         .type   tar,@function
tar:                                    # @tar
# %bb.0:
         call bar
         w2 <<= 16
         w2 s>>= 16
         w2 += w0
         w0 = w2
         exit
.Lfunc_end0:
         .size   tar, .Lfunc_end0-tar
                                         # -- End function
         .addrsig

The caller will do proper narrowing immediately after call return.

>
>>   		bpf_log(log,
>>   			"The function %s return type %s is unsupported.\n",
>>   			tname, btf_type_str(t));