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

Alexei Starovoitov
2026-04-21T00:37:33Z

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