Re: [PATCH bpf-next v3 03/13] bpf: Wire up JIT support for 16-byte kfunc returns

On 8/8/26 12:39 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Uninitialized kernel register state can be leaked to BPF programs when a kfunc returns an aggregate (struct/union) between 9 and 15 bytes in size.
> --
>
> --- Patch [3]: [PATCH bpf-next v3 03/13] bpf: Wire up JIT support for 16-byte kfunc returns ---
> commit f2df1b40e2f3eb74688d20cd37742fe85c667da9
> Author: Yonghong Song <[email protected]>
> Subject: bpf: Wire up JIT support for 16-byte kfunc returns
>
> This patch adds JIT support for returning structures up to 16 bytes from
> kfuncs in the BPF R0:R2 register pair. It implements the necessary RDX to RSI
> move for x86-64 and adds opt-in capabilities for the x86, arm64, and riscv
> architectures.
>
>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>> index 8dddb5d7af21b..01c34114c8502 100644
>> --- a/arch/x86/net/bpf_jit_comp.c
>> +++ b/arch/x86/net/bpf_jit_comp.c
> [ ... ]
>> @@ -2647,6 +2647,22 @@ st:			insn_off = insn->off;
>>   				return -EINVAL;
>>   			if (priv_frame_ptr)
>>   				pop_r9(&prog);
>> +			if (src_reg == BPF_PSEUDO_KFUNC_CALL) {
>> +				const struct btf_func_model *fm;
>> +
>> +				/*
>> +				 * A kfunc returning a >8 byte aggregate hands the
>> +				 * second half back in RDX (the native ABI's second
>> +				 * return reg), but BPF expects it in R0:R2. BPF R0
>> +				 * is RAX (no move needed), while BPF R2 is RSI, so
>> +				 * copy RDX into RSI.
>> +				 */
>> +				fm = bpf_jit_find_kfunc_model(bpf_prog, insn);
>> +				if (!fm)
>> +					return -EFAULT;
>> +				if (fm->ret_size > 8)
>> +					emit_mov_reg(&prog, true, BPF_REG_2, BPF_REG_3);
> [Severity: High]
> In do_jit(), when a kfunc returns an aggregate between 9 and 15 bytes in size,
> could this 64-bit move leak uninitialized kernel register state?
>
> According to the native ABI, the upper bytes of the second return register
> (RDX/BPF_REG_3) might contain padding and be uninitialized.
>
> Since the move is a full 64-bit copy to BPF_REG_2, and the BPF verifier allows
> 64-bit reads of BPF_REG_2 without zero-extending the padded bits, a BPF
> program might save the full register to a map and leak the uninitialized bytes
> to userspace.
>
> Should the JIT mask out the uninitialized upper bytes of BPF_REG_2 when the
> return size is between 9 and 15 bytes?
>
> This same question applies to the arm64 and riscv capabilities enabled in this
> patch, where the native registers (x1 and a1) map directly to BPF_REG_2 and
> could similarly leave the upper bits unmasked.

We should be okay here. The return value will do proper narrowing. See
https://reviews.llvm.org/D131598

>
>> +			}
>>   			break;
>>   		}