Re: [PATCH bpf-next v1 22/22] bpf: Check helper and kfunc arguments in one path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Out-of-bounds reads on `meta->fn->arg_type` and `meta->fn->arg_btf_id` for kfuncs with more than 5 arguments, allowing bypass of eBPF verifier safety checks.
--

commit a1895fafaa5219b7b972d20a932c550f7862cec8
Author: Amery Hung <ameryhung@gmail.com>

bpf: Check helper and kfunc arguments in one path

This commit unifies the argument checking logic for BPF helpers and kfuncs
by introducing check_func_args(). It moves kfunc-specific argument handling
into the shared check_func_arg() path to avoid duplicating the argument
iteration and switch logic.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 6c5d7e544f844..54656fb31bbe7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -8808,6 +8840,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
>  			  struct bpf_call_arg_meta *meta,
>  			  int insn_idx)
>  {
> +	const struct btf_param *btf_arg = meta->btf ? &btf_params(meta->func_proto)[arg] : NULL;
>  	const struct bpf_func_proto *fn = meta->fn;
>  	struct bpf_func_state *caller = cur_func(env);
>  	struct bpf_reg_state *regs = cur_regs(env);
>  	argno_t argno = argno_from_arg(arg + 1);
>  	struct bpf_reg_state *reg = get_func_arg_reg(caller, regs, arg);
>  	enum bpf_arg_type arg_type = fn->arg_type[arg];

[Severity: Critical]
Can this read out of bounds when processing kfuncs with more than 5
arguments? The fn->arg_type array is sized for MAX_BPF_FUNC_ARGS (5), but
kfuncs can have more arguments. For stack arguments (arg >= 5), this appears
to read past the end of the array.

[ ... ]
> @@ -8935,6 +8980,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
>  	case ARG_PTR_TO_BTF_ID_SOCK_COMMON:
>  	{
>  		const u32 *arg_btf_id = fn->arg_btf_id[arg];

[Severity: Critical]
Does this also read out of bounds for kfuncs with more than 5 arguments?
Similar to arg_type, the arg_btf_id array in struct bpf_func_proto is limited
to MAX_BPF_FUNC_ARGS. If a garbage pointer is dereferenced here, could it
crash the verifier?

[ ... ]
> @@ -9056,6 +9389,37 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
>  	return err;
>  }
>  
> +static int check_func_args(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
> +			   int insn_idx)
> +{
> +	struct bpf_func_state *caller = cur_func(env);
> +	const struct btf_param *args = NULL;
> +	u32 arg, nargs = MAX_BPF_FUNC_REG_ARGS;
> +	int err;
> +
> +	if (meta->btf) {
> +		args = btf_params(meta->func_proto);
> +		nargs = btf_type_vlen(meta->func_proto);
> +	}
> +
> +	if (nargs > MAX_BPF_FUNC_REG_ARGS) {
> +		err = check_outgoing_stack_args(env, caller, nargs, meta->func_name,
> +						meta->btf, args);
> +		if (err)
> +			return err;
> +	}
> +
> +	for (arg = 0; arg < nargs; arg++) {
> +		if (meta->fn->arg_type[arg] == ARG_UNUSED)
> +			break;

[Severity: Critical]
Could this loop early-exit and bypass verifier checks for stack arguments?

Since nargs can exceed 5 for kfuncs, the out-of-bounds read on
meta->fn->arg_type[arg] might evaluate to ARG_UNUSED (0). If this happens,
the loop breaks and skips the safety checks for all remaining stack
arguments, potentially allowing unverified memory access.

> +		err = check_func_arg(env, arg, meta, insn_idx);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}
> +

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