Re: [PATCH bpf-next v4 05/25] bpf: Support stack arguments for bpf functions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Bypass of caller stack argument initialization checks allows NULL pointer dereferences and out-of-bounds heap reads, leading to verifier crashes or privilege escalation.
--

commit 64c850bac22b15ca90af1ba361a4d764104b6ffb
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Support stack arguments for bpf functions

This commit allows the compiler to pass additional arguments via a dedicated
stack area through the BPF_REG_PARAMS (r11) register. It tracks outgoing stack
arguments and introduces bounds checks for stack arguments in BPF subprograms.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 3bafb7ad2ba71..69965d2c5fdd5 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -4050,6 +4080,103 @@ static int check_stack_write(struct bpf_verifier_env *env,
>  	return err;
>  }
>  
> +/*
> + * Read a value from the incoming stack arg area.
> + * off is a positive offset from r11 (e.g. +8 for arg6, +16 for arg7).
> + */
> +static int check_stack_arg_read(struct bpf_verifier_env *env, struct bpf_func_state *state,
> +				int off, int dst_regno)
> +{
> +	struct bpf_subprog_info *subprog = &env->subprog_info[state->subprogno];
> +	struct bpf_verifier_state *vstate = env->cur_state;
> +	int spi = off / BPF_REG_SIZE - 1;
> +	struct bpf_func_state *caller, *cur;
> +	struct bpf_reg_state *arg;
> +
> +	if (state->no_stack_arg_load) {
> +		verbose(env, "r11 load must be before any r11 store or call insn\n");
> +		return -EINVAL;
> +	}
> +
> +	if (spi + 1 > bpf_in_stack_arg_cnt(subprog)) {
> +		verbose(env, "invalid read from stack arg off %d depth %d\n",
> +			off, bpf_in_stack_arg_cnt(subprog) * BPF_REG_SIZE);
> +		return -EACCES;
> +	}
> +
> +	caller = vstate->frame[vstate->curframe - 1];
> +	arg = &caller->stack_arg_regs[spi];

Could this lead to an out-of-bounds read or a NULL pointer dereference?

If check_outgoing_stack_args() is skipped (e.g. due to an error in
btf_prepare_func_args()), caller->stack_arg_regs might be NULL or smaller
than spi. Since check_func_call() or push_callback_call() can ignore
btf_check_func_arg_match() failures for static subprograms or callbacks,
the callee can still execute.

If sub->arg_cnt was updated before the failure, bpf_in_stack_arg_cnt(subprog)
would be greater than 0, allowing this check to pass and potentially accessing
unallocated memory.

[ ... ]

> @@ -8922,6 +9078,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
>  				    struct bpf_reg_state *regs)
>  {
>  	struct bpf_subprog_info *sub = subprog_info(env, subprog);
> +	struct bpf_func_state *caller = cur_func(env);
>  	struct bpf_verifier_log *log = &env->log;
>  	u32 i;
>  	int ret;
> @@ -8930,13 +9087,16 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
>  	if (ret)
>  		return ret;
>  
> +	ret = check_outgoing_stack_args(env, caller, sub->arg_cnt);
> +	if (ret)
> +		return ret;
> +

Is it possible for btf_prepare_func_args() to return an error after modifying
sub->arg_cnt?

For instance, if a BTF tag is unsupported in a static subprogram,
btf_prepare_func_args() will return an error after already assigning
sub->arg_cnt. This skips check_outgoing_stack_args(), which is responsible
for verifying that the caller actually allocated and initialized
caller->stack_arg_regs. This could lead to the unverified memory access
mentioned above.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513044949.2382019-1-yonghong.song@linux.dev?part=5