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

["Alexei Starovoitov" <alexei.starovoitov@gmail.com>]

On Sun Apr 19, 2026 at 9:33 AM PDT, Yonghong Song wrote:
> Currently BPF functions (subprogs) are limited to 5 register arguments.
> With [1], the compiler can emit code that passes additional arguments
> via a dedicated stack area through bpf register BPF_REG_PARAMS (r11),
> introduced in the previous patch.
>
> The compiler uses positive r11 offsets for incoming (callee-side) args
> and negative r11 offsets for outgoing (caller-side) args, following the
> x86_64/arm64 calling convention direction. There is an 8-byte gap at
> offset 0 separating the two regions:
>   Incoming (callee reads):   r11+8 (arg6), r11+16 (arg7), ...
>   Outgoing (caller writes):  r11-8 (arg6), r11-16 (arg7), ...

This part looks clean now.

> A per-state bitmask out_stack_arg_mask tracks which outgoing stack arg
> slots have been written on the current path. Each bit corresponds to
> an outgoing slot index (bit 0 = r11-8 = arg6, bit 1 = r11-16 = arg7,
> etc.). At a call site, the verifier checks that all slots required by
> the callee have their corresponding mask bits set. This enables
> precise per-path tracking: if one branch of a conditional writes arg6
> but another does not, the mask correctly reflects the difference and
> the verifier rejects the uninitialized path. The mask is included in
> stack_arg_safe() so that states with different sets of initialized
> slots are not incorrectly pruned together.

But this part I don't understand.
why do you need this bitmask?
Even when they're written out of order stack_arg_depth is all you need.
Then compare old->stack_arg_regs vs cur->stack_arg_regs.
If one is not written its state will be NOT_INIT.
so
  *(u64 *)(r11 - 16) = r7;
  // without *(u64 *)(r11 - 8) = r6;
  call bar1;                // arg6 = r6, arg7 = r7

will fail the verification.

> @@ -1669,6 +1669,8 @@ struct bpf_prog_aux {
>  	u32 max_pkt_offset;
>  	u32 max_tp_access;
>  	u32 stack_depth;
> +	u16 incoming_stack_arg_depth;
> +	u16 stack_arg_depth; /* both incoming and max outgoing of stack arguments */

these two are ok. I'm assuming you don't want JIT to recompute them.

>  	u32 id;
>  	u32 func_cnt; /* used by non-func prog as the number of func progs */
>  	u32 real_func_cnt; /* includes hidden progs, only used for JIT and freeing progs */
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 9fbbddc40d21..bb6d8cab3a35 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -372,6 +372,11 @@ struct bpf_func_state {
>  	 * `stack`. allocated_stack is always a multiple of BPF_REG_SIZE.
>  	 */
>  	int allocated_stack;
> +
> +	u16 stack_arg_depth; /* Size of incoming + max outgoing stack args in bytes. */
> +	u16 incoming_stack_arg_depth; /* Size of incoming stack args in bytes. */

but incoming_stack_arg_depth looks odd.

Callee should be accessing caller's stack_arg_depth and
caller's stack_arg_regs.

> +	u16 out_stack_arg_mask; /* Bitmask of outgoing stack arg slots that have been written. */
> +	struct bpf_reg_state *stack_arg_regs; /* On-stack arguments */
>  };
>  
>  #define MAX_CALL_FRAMES 8
> @@ -508,6 +513,17 @@ struct bpf_verifier_state {
>  	     iter < frame->allocated_stack / BPF_REG_SIZE;		\
>  	     iter++, reg = bpf_get_spilled_reg(iter, frame, mask))
>  
> +#define bpf_get_spilled_stack_arg(slot, frame, mask)                   \
> +	((((slot) < frame->stack_arg_depth / BPF_REG_SIZE) &&           \
> +	  (frame->stack_arg_regs[slot].type != NOT_INIT))               \
> +	 ? &frame->stack_arg_regs[slot] : NULL)
> +
> +/* Iterate over 'frame', setting 'reg' to either NULL or a spilled stack arg. */
> +#define bpf_for_each_spilled_stack_arg(iter, frame, reg, mask)         \
> +	for (iter = 0, reg = bpf_get_spilled_stack_arg(iter, frame, mask); \
> +	     iter < frame->stack_arg_depth / BPF_REG_SIZE;              \
> +	     iter++, reg = bpf_get_spilled_stack_arg(iter, frame, mask))
> +
>  #define bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, __mask, __expr)   \
>  	({                                                               \
>  		struct bpf_verifier_state *___vstate = __vst;            \
> @@ -525,6 +541,11 @@ struct bpf_verifier_state {
>  					continue;                        \
>  				(void)(__expr);                          \
>  			}                                                \
> +			bpf_for_each_spilled_stack_arg(___j, __state, __reg, __mask) { \
> +				if (!__reg)                              \
> +					continue;                        \
> +				(void)(__expr);                          \
> +			}						 \
>  		}                                                        \
>  	})
>  
> @@ -739,10 +760,13 @@ struct bpf_subprog_info {
>  	bool keep_fastcall_stack: 1;
>  	bool changes_pkt_data: 1;
>  	bool might_sleep: 1;
> -	u8 arg_cnt:3;
> +	u8 arg_cnt:4;
>  
>  	enum priv_stack_mode priv_stack_mode;
> -	struct bpf_subprog_arg_info args[MAX_BPF_FUNC_REG_ARGS];
> +	struct bpf_subprog_arg_info args[MAX_BPF_FUNC_ARGS];
> +	u16 incoming_stack_arg_depth;
> +	u16 outgoing_stack_arg_depth;
> +	u16 max_out_stack_arg_depth;

but you already have them in prog_aux?! another copy in bpf_subprog_info?!
Remove one of them. JIT only need one set.

>  };
>  
>  struct bpf_verifier_env;
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index a62d78581207..c5f3aa05d5a3 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7887,13 +7887,19 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
>  	}
>  	args = (const struct btf_param *)(t + 1);
>  	nargs = btf_type_vlen(t);
> -	if (nargs > MAX_BPF_FUNC_REG_ARGS) {
> -		if (!is_global)
> -			return -EINVAL;
> -		bpf_log(log, "Global function %s() with %d > %d args. Buggy compiler.\n",
> +	if (nargs > MAX_BPF_FUNC_ARGS) {
> +		bpf_log(log, "Function %s() with %d > %d args not supported.\n",
> +			tname, nargs, MAX_BPF_FUNC_ARGS);
> +		return -EINVAL;
> +	}
> +	if (is_global && nargs > MAX_BPF_FUNC_REG_ARGS) {
> +		bpf_log(log, "Global function %s() with %d > %d args not supported.\n",
>  			tname, nargs, MAX_BPF_FUNC_REG_ARGS);
>  		return -EINVAL;
>  	}
> +	if (nargs > MAX_BPF_FUNC_REG_ARGS)
> +		sub->incoming_stack_arg_depth = (nargs - MAX_BPF_FUNC_REG_ARGS) * BPF_REG_SIZE;
> +
>  	/* check that function is void or returns int, exception cb also requires this */
>  	t = btf_type_by_id(btf, t->type);
>  	while (btf_type_is_modifier(t))
> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index fba9e8c00878..c4e0224ad2f2 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
> @@ -1123,6 +1123,9 @@ static int jit_subprogs(struct bpf_verifier_env *env)
>  
>  		func[i]->aux->name[0] = 'F';
>  		func[i]->aux->stack_depth = env->subprog_info[i].stack_depth;
> +		func[i]->aux->incoming_stack_arg_depth = env->subprog_info[i].incoming_stack_arg_depth;
> +		func[i]->aux->stack_arg_depth = env->subprog_info[i].incoming_stack_arg_depth +
> +						env->subprog_info[i].outgoing_stack_arg_depth;
>  		if (env->subprog_info[i].priv_stack_mode == PRIV_STACK_ADAPTIVE)
>  			func[i]->aux->jits_use_priv_stack = true;
>  
> @@ -1301,8 +1304,10 @@ int bpf_jit_subprogs(struct bpf_verifier_env *env)
>  	struct bpf_insn_aux_data *orig_insn_aux;
>  	u32 *orig_subprog_starts;
>  
> -	if (env->subprog_cnt <= 1)
> +	if (env->subprog_cnt <= 1) {
> +		env->prog->aux->stack_arg_depth = env->subprog_info[0].outgoing_stack_arg_depth;
>  		return 0;
> +	}
>  
>  	prog = orig_prog = env->prog;
>  	if (bpf_prog_need_blind(prog)) {
> @@ -1378,9 +1383,20 @@ int bpf_fixup_call_args(struct bpf_verifier_env *env)
>  	struct bpf_prog *prog = env->prog;
>  	struct bpf_insn *insn = prog->insnsi;
>  	bool has_kfunc_call = bpf_prog_has_kfunc_call(prog);
> -	int i, depth;
> +	int depth;
>  #endif
> -	int err = 0;
> +	int i, err = 0;
> +
> +	for (i = 0; i < env->subprog_cnt; i++) {
> +		struct bpf_subprog_info *subprog = &env->subprog_info[i];
> +
> +		if (subprog->max_out_stack_arg_depth > subprog->outgoing_stack_arg_depth) {
> +			verbose(env,
> +				"func#%d writes stack arg slot at depth %u, but calls only require %u bytes\n",
> +				i, subprog->max_out_stack_arg_depth, subprog->outgoing_stack_arg_depth);
> +			return -EINVAL;
> +		}
> +	}
>  
>  	if (env->prog->jit_requested &&
>  	    !bpf_prog_is_offloaded(env->prog->aux)) {
> diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
> index 8478d2c6ed5b..235841d23fe3 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;
> +
> +	if (old->out_stack_arg_mask != cur->out_stack_arg_mask)
> +		return false;

shouldn't be neccessary.

> +
> +	nslots = old->stack_arg_depth / BPF_REG_SIZE;
> +	for (i = 0; i < nslots; i++) {
> +		struct bpf_reg_state *old_arg = &old->stack_arg_regs[i];
> +		struct bpf_reg_state *cur_arg = &cur->stack_arg_regs[i];
> +
> +		if (old_arg->type == NOT_INIT && cur_arg->type == NOT_INIT)
> +			continue;
> +
> +		if (exact == EXACT && old_arg->type != cur_arg->type)
> +			return false;
> +
> +		if (!regsafe(env, old_arg, cur_arg, idmap, exact))
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
>  static bool refsafe(struct bpf_verifier_state *old, struct bpf_verifier_state *cur,
>  		    struct bpf_idmap *idmap)
>  {
> @@ -929,6 +967,9 @@ static bool func_states_equal(struct bpf_verifier_env *env, struct bpf_func_stat
>  	if (!stacksafe(env, old, cur, &env->idmap_scratch, exact))
>  		return false;
>  
> +	if (!stack_arg_safe(env, old, cur, &env->idmap_scratch, exact))
> +		return false;
> +
>  	return true;
>  }
>  
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 6aa4dc161a56..78c9322870a5 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -1340,6 +1340,20 @@ static int copy_stack_state(struct bpf_func_state *dst, const struct bpf_func_st
>  		return -ENOMEM;
>  
>  	dst->allocated_stack = src->allocated_stack;
> +
> +	/* copy stack args state */
> +	n = src->stack_arg_depth / BPF_REG_SIZE;
> +	if (n) {
> +		dst->stack_arg_regs = copy_array(dst->stack_arg_regs, src->stack_arg_regs, n,
> +					     sizeof(struct bpf_reg_state),
> +					     GFP_KERNEL_ACCOUNT);

copy is unnecessary.

> @@ -4220,6 +4254,13 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
>  					}
>  					if (type == STACK_INVALID && env->allow_uninit_stack)
>  						continue;
> +					/*
> +					 * Cross-frame reads may hit slots poisoned by dead code elimination.
> +					 * Static liveness can't track indirect references through pointers,
> +					 * so allow the read conservatively.
> +					 */
> +					if (type == STACK_POISON && reg_state != state)
> +						continue;

wait what? Is this a real issue? Are you saying you have a test prog
that passes FP derived pointers in arg 6, 7 and arg_tracking cannot detect it?
Then it should be added properly in arg_tracking.
This hack to allow reading poisoned slots is not ok.
This is a serious issue.

pw-bot: cr