Re: [PATCH bpf-next v2 7/7] bpf: Unify helper and kfunc call argument meta

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Uninitialized stack variable `meta.func_name` dereferenced on error path, leading to kernel panic.
--

--- Patch [7]: [PATCH bpf-next v2 7/7] bpf: Unify helper and kfunc call argument meta ---
commit ca91a3b2fc7adc980912af895a2a5840eb94b51f
Author: Amery Hung <ameryhung@gmail.com>

bpf: Unify helper and kfunc call argument meta
    
This commit merges bpf_call_arg_meta and bpf_kfunc_call_arg_meta into a single
struct. The unused btf_id field is dropped from the helper meta as groundwork
for sharing argument checking logic.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -12953,11 +12953,11 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  	struct bpf_reg_state *regs = cur_regs(env);
>  	const char *func_name, *ptr_type_name;
>  	const struct btf_type *t, *ptr_type;
> -	struct bpf_kfunc_call_arg_meta meta;
> +	struct bpf_call_arg_meta meta;
>  	struct bpf_insn_aux_data *insn_aux;
>  	int err, insn_idx = *insn_idx_p;
>  	const struct btf_param *args;

[ ... ]

>  	/* skip for now, but return error when we find this in fixup_kfunc_call */
>  	if (!insn->imm)
>  		return 0;
>  
>  	err = bpf_fetch_kfunc_arg_meta(env, insn->imm, insn->off, &meta);
>  	if (err == -EACCES && meta.func_name)
>  		verbose(env, "calling kernel function %s is not allowed\n", meta.func_name);

[Severity: High]
This is a pre-existing issue, but does this error path dereference an
uninitialized stack variable?

Looking at bpf_fetch_kfunc_arg_meta() in kernel/bpf/verifier.c, if
fetch_kfunc_meta() returns an error like -EACCES, it returns early without
initializing the struct:

int bpf_fetch_kfunc_arg_meta(...)
{
        ...
        err = fetch_kfunc_meta(env, func_id, offset, &kfunc);
        if (err)
                return err;
        
        memset(meta, 0, sizeof(*meta));
        ...
}

When this early return happens, the meta struct on the stack in
check_kfunc_call() remains uninitialized. The check here for
err == -EACCES && meta.func_name will then evaluate garbage memory for
meta.func_name.

If the uninitialized memory evaluates to true, verbose() will dereference
that garbage pointer, which could lead to a kernel panic. Could we initialize
the meta struct earlier, or zero it in check_kfunc_call() before passing it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715064047.1793790-1-ameryhung@gmail.com?part=7