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

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.

> +			}
>  			break;
>  		}

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