Re: [PATCH bpf-next v5 03/11] 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] The verifier fails to initialize BPF_REG_2 for 16-byte kfunc returns, and completely fails to initialize both BPF_REG_0 and BPF_REG_2 if the kfunc returns a 16-byte struct.
--

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

bpf: Wire up JIT support for 16-byte kfunc returns

This commit adds JIT support for 16-byte kfunc returns by placing the
second half of the return value in the BPF R2 register. It introduces
architecture-specific capabilities and enables support for x86-64, arm64,
and riscv64. The verifier is also updated to reject unsupported 16-byte
returns.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 57d14480ded29..161d77791bc62 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2854,6 +2854,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: High]
Does the verifier need updates in check_kfunc_call() to track the initialization
of BPF_REG_2 for these larger returns?

When evaluating the kfunc return type in check_kfunc_call(), the verifier
currently appears to miss marking BPF_REG_2 as initialized for 16-byte scalar
returns:

kernel/bpf/verifier.c:check_kfunc_call() {
    if (btf_type_is_scalar(t)) {
        mark_reg_unknown(env, regs, BPF_REG_0);
        ...
    } else if (btf_type_is_ptr(t)) {
        ...
    } else if (btf_type_is_void(t)) {
        ...
    }
}

Furthermore, if the kfunc returns a 16-byte struct, the code falls through
completely without initializing either BPF_REG_0 or BPF_REG_2.

Since all CALLER_SAVED_REGS are cleared to NOT_INIT earlier during the call,
won't this cause the verifier to incorrectly reject valid BPF programs that
attempt to read these return values?

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