Re: [PATCH bpf-next v6 03/10] 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:
- [Medium] The BPF verifier fails to initialize BPF_REG_2 for 16-byte scalar kfunc returns, breaking the feature.
--

--- Patch [3]: [PATCH bpf-next v6 03/10] bpf: Wire up JIT support for 16-byte kfunc returns ---
commit 70077e00d2d0c46ef0daa85eb2612ed7fbf545ef
Author: Yonghong Song <[email protected]>
bpf: Wire up JIT support for 16-byte kfunc returns

This commit wires up JIT support for 16-byte kfunc returns in BPF, such as
large structs or 128-bit integers. It adds support to the x86-64, arm64, and
riscv64 JITs to return the second half of the value in BPF_REG_2, and adds
verifier checks to reject larger returns on unsupported architectures.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index f1f1268d29c66..54aca6c305061 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -2920,6 +2920,18 @@ int bpf_add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, u16 offset)
>  	err = btf_distill_func_proto(&env->log, kfunc.btf, kfunc.proto, kfunc.name, &func_model);
>  	if (err)
>  		return err;
> +	if (func_model.ret_size > 8) {
> +		if (kfunc.flags && (*kfunc.flags & KF_FASTCALL)) {
> +			verbose(env, "kfunc %s with >8-byte return is not supported with KF_FASTCALL\n",
> +				kfunc.name);
> +			return -EOPNOTSUPP;
> +		}
> +		if (!bpf_jit_supports_kfunc_ret_reg_pair()) {
> +			verbose(env, "kfunc %s with >8-byte return is not supported by JIT\n",
> +				kfunc.name);
> +			return -EOPNOTSUPP;
> +		}
> +	}

[Severity: Medium]
While this patch adds JIT support and basic verifier checks for >8-byte
returns, does the verifier correctly initialize BPF_REG_2 for 16-byte scalar
returns?

Looking at check_kfunc_call() in kernel/bpf/verifier.c, it seems only
BPF_REG_0 is marked as initialized for scalar returns:

    if (btf_type_is_scalar(t)) {
        mark_reg_unknown(env, regs, BPF_REG_0);
        if (meta.btf == btf_vmlinux &&
            (meta.func_id == special_kfunc_list[KF_bpf_res_spin_lock] || ...

Since BPF caller-saved registers (including BPF_REG_2) are marked as NOT_INIT
earlier in check_kfunc_call(), a BPF program attempting to access the upper
64 bits of a 16-byte scalar (like an __int128) in BPF_REG_2 would be rejected
with a verifier read error.

Should check_kfunc_call() also initialize BPF_REG_2 when the return size is
greater than 8 bytes?

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