Re: [PATCH bpf-next v1 4/4] 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 memory read in `check_kfunc_call` when handling kfunc calls in signed BPF programs.
--

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

bpf: Unify helper and kfunc call argument meta
    
Helper and kfunc argument checking carried two separate meta structs: the
verifier-local struct bpf_call_arg_meta and bpf_kfunc_call_arg_meta.
Merge them into a single struct bpf_call_arg_meta. This is groundwork for
sharing argument checking between helpers and kfuncs.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index fcd9715eff017..5ccb00a8c84ce 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -12972,7 +12953,7 @@ 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;

[Severity: High]
This is a pre-existing issue, but does this lead to reading an uninitialized
pointer if bpf_fetch_kfunc_arg_meta() returns early?

In check_kfunc_call() in kernel/bpf/verifier.c, the local variable meta is
declared without initialization and passed to bpf_fetch_kfunc_arg_meta().
If the call to fetch_kfunc_meta() inside it returns an error like -EACCES,
it returns early before initializing meta:

kernel/bpf/verifier.c:bpf_fetch_kfunc_arg_meta() {
	struct bpf_kfunc_meta kfunc;
	int err;

	err = fetch_kfunc_meta(env, func_id, offset, &kfunc);
	if (err)
		return err;
    ...
}

Back in check_kfunc_call(), the error path evaluates meta.func_name:

	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);

If the stack memory is uninitialized, could meta.func_name contain a garbage
pointer that leads to a crash or an info leak when dereferenced by vsnprintf
during the verbose() call?

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