Re: [PATCH bpf-next v6 07/17] bpf: Support stack arguments for bpf functions

[sashiko-bot@kernel.org]

Sashiko AI review found 3 potential issue(s):
- [Critical] Missing precision tracking for stack arguments (`BPF_REG_PARAMS` / `r11`) leads to incorrect verifier state pruning and arbitrary out-of-bounds memory access.
- [Medium] `BPF_PROG_TYPE_EXT` (freplace) programs are incorrectly rejected when targeting static subprograms with more than 5 arguments.
- [Low] `stack_arg_safe()` strictly compares `stack_arg_depth`, preventing safe paths from being pruned and leading to state explosion.
--

commit 0c58d1bb907c4a6a387f1afd22baae9d43e95a55
Author: Yonghong Song <yonghong.song@linux.dev>
Subject: bpf: Support stack arguments for bpf functions

This commit introduces support for passing more than 5 arguments to BPF
subprograms by using a dedicated stack area accessed via register r11.
It tracks these stack arguments separately from the regular stack to ensure
proper verification and pruning.

Link: https://github.com/llvm/llvm-project/pull/189060

> diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
> index 8478d2c6ed5b6..235841d23fe3f 100644
> --- a/kernel/bpf/states.c
> +++ b/kernel/bpf/states.c
[ ... ]
> @@ -838,6 +838,44 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
>  	return true;
>  }
>  
> +/*
> + * Compare stack arg slots between old and current states.
> + * Outgoing stack args are path-local state and must agree for pruning.
> + */
> +static bool stack_arg_safe(struct bpf_verifier_env *env, struct bpf_func_state *old,
> +			   struct bpf_func_state *cur, struct bpf_idmap *idmap,
> +			   enum exact_level exact)
> +{
> +	int i, nslots;
> +
> +	if (old->incoming_stack_arg_depth != cur->incoming_stack_arg_depth)
> +		return false;
> +
> +	/* Compare both incoming and outgoing stack arg slots. */
> +	if (old->stack_arg_depth != cur->stack_arg_depth)
> +		return false;

Does this strict comparison of stack_arg_depth prevent safe paths from
being pruned?

Since stack_arg_depth tracks the allocated capacity rather than the
active outgoing arguments tracked by out_stack_arg_mask, two branches
that call subprograms with different stack argument requirements might
end up with permanently different stack_arg_depth values.

Could this unnecessarily increase verification complexity by failing to
prune functionally equivalent states?

[ ... ]
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 6aa4dc161a563..78c9322870a51 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> +static int check_stack_arg_write(struct bpf_verifier_env *env, struct bpf_func_state *state,
> +				 int off, int value_regno)
> +{
> +	int max_stack_arg_regs = MAX_BPF_FUNC_ARGS - MAX_BPF_FUNC_REG_ARGS;
[ ... ]
> +	state->out_stack_arg_mask |= BIT(idx);
> +	return 0;
> +}
> +
> +/*
> + * Read a value from the incoming stack arg area.
> + * off is a positive offset from r11 (e.g. +8 for arg6, +16 for arg7).
> + * Callers ensure off > 0, 8-byte aligned, and size is BPF_REG_SIZE.
> + */
> +static int check_stack_arg_read(struct bpf_verifier_env *env, struct bpf_func_state *state,
> +				int off, int dst_regno)
> +{
[ ... ]
> +	if (is_spillable_regtype(arg->type))
> +		copy_register_state(&cur->regs[dst_regno], arg);
> +	else
> +		mark_reg_unknown(env, cur->regs, dst_regno);
> +	return 0;
> +}

Should check_stack_arg_read() and check_stack_arg_write() call
bpf_push_jmp_history() with the INSN_F_STACK_ACCESS flag?

When the callee uses a stack argument in a context requiring precision,
backtracking is initiated. Because the INSN_F_STACK_ACCESS flag is
missing, backtrack_insn() might silently return 0. This clears the
destination register without propagating the precision requirement to
the stack slot.

Could this cause the verifier to incorrectly prune execution paths
containing different, potentially unsafe scalar values, allowing
programs to bypass verifier bounds checks and perform arbitrary
out-of-bounds accesses? Note that the subsequent patch that tracks
r11 registers in const_fold and liveness does not address this
backtracking logic.

[ ... ]
> @@ -18808,7 +19097,7 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
>  				goto out;
>  			}
>  		}
> -		for (i = BPF_REG_1; i <= sub->arg_cnt; i++) {
> +		for (i = BPF_REG_1; i <= min_t(u32, sub->arg_cnt, MAX_BPF_FUNC_REG_ARGS); i++) {
>  			arg = &sub->args[i - BPF_REG_1];
>  			reg = &regs[i];

For freplace programs targeting static subprograms with more than 5
arguments, do we need to initialize state->frame[0]->incoming_stack_arg_depth
here or allocate stack_arg_regs?

Currently it seems to only initialize arguments up to MAX_BPF_FUNC_REG_ARGS.
If incoming_stack_arg_depth remains 0, an EXT program attempting to read
its 6th argument via BPF_REG_PARAMS would fail verification in
check_stack_arg_read() with -EACCES.

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