Re: [PATCH bpf-next 01/18] bpf: Support stack arguments for bpf functions

[Yonghong Song <yonghong.song@linux.dev>]

On 5/2/26 6:03 PM, Alexei Starovoitov wrote:
> On Fri Apr 24, 2026 at 10:14 AM PDT, Yonghong Song wrote:
>> @@ -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 */
>>   	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 */
> ...
>
>> @@ -739,10 +759,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 stack_arg_depth; /* incoming + max outgoing */
>> +	u16 max_out_stack_arg_depth;
>>   };
> I asked before but if there was an answer it got lost in all emails.
> So will ask again: why duplicate incoming_stack_arg_depth
> and stack_arg_depth in two places? One should be enough.

We can remove 'incoming_stack_arg_depth' in bpf_subprog_info since
in bpf_subprog_info we have 'arg_cnt' field.

I am not sure whether we can remove stack_arg_depth.
stack_arg_depth is to accumulate the incoming + max_outgoing since
the subprog may have multiple functions inside.
At this verification point, we accumulate such infomation at bpf_subprog_info.
At this point, we only have main program, so we cannot copy
subprog info stack_arg_depth to subprog. The subprog allocations
are in jit_subprogs(). Did I miss anything?

>
> And another question:
> max_out_stack_arg_depth is computed only to error like this in bpf_fixup_call_args().
>
> +               u16 outgoing = subprog->stack_arg_depth - subprog->incoming_stack_arg_depth;
> +
> +               if (subprog->max_out_stack_arg_depth > outgoing) {
> +                       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, outgoing);
> +                       return -EINVAL;
>
> why bother? What will go wrong if it's not there?

This is related to jit. For example, we have the following x86 jit stack layout:

       high address
       +-------------------------+
       | incoming stack arg N    |  [rbp + 16 + (N-7)*8]  (from caller)
       | ...                     |
       | incoming stack arg 7    |  [rbp + 16]
       +-------------------------+
       | return address          |  [rbp + 8]
       | saved rbp               |  [rbp]
       +-------------------------+
       | BPF program stack       |  (round_up(stack_depth, 8) bytes)
       +-------------------------+
       | callee-saved regs       |  (r12, rbx, r13, r14, r15 as needed)
       +-------------------------+
       | outgoing arg M          |  [rsp + (M-7)*8]
       | ...                     |
       | outgoing arg 7          |  [rsp]
       +-------------------------+  rsp
       low address

Let us say M = 8, so we will do
     *(r11 - 8) = X1
          is translated to r9 = X1
     *(r11 - 16) = X2
          is to store X2 to slot 'outgoing arg 7'
     *(r11 - 24) = X3
          is to store X3 to slot 'outgoing arg 8'

Let us we have another *(r11 - 32) = X4 in the code, it
will to store the X4 in 'callee-saved regs' area and this
will corrupt the callee-saved regs.
That is why we should reject it in verifier.