Re: [PATCH bpf-next v3 1/2] bpf: Only enforce 8 frame call stack limit for all-static stacks

["Emil Tsalapatis" <emil@etsalapatis.com>]

On Tue Mar 3, 2026 at 8:01 PM EST, Eduard Zingerman wrote:
> On Mon, 2026-03-02 at 23:31 -0500, Emil Tsalapatis wrote:
>> The BPF verifier currently enforces a call stack depth of 8 frames,
>> regardless of the actual stack space consumption of those frames. The
>> limit is necessary for static call stacks, because the bookkeeping data
>> structures used by the verifier when stepping into static functions
>> during verification only support 8 stack frames. However, this
>> limitation only matters for static stack frames: Global subprogs are
>> verified by themselves and do not require limiting the call depth.
>> 
>> Relax this limitation to only apply to static stack frames. Verification
>> now only fails when there is a sequence of 8 calls to non-global
>> subprogs. Calling into a global subprog resets the counter. This allows
>> deeper call stacks, provided all frames still fit in the stack.
>> 
>> The change does not increase the maximum size of the call stack, only
>> the maximum number of frames we can place in it.
>> 
>> Also change the progs/test_global_func3.c selftest to use static
>> functions, since with the new patch it would otherwise unexpectedly
>> pass verification.
>> 
>> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
>> ---
>>  include/linux/bpf_verifier.h                  |  6 +++
>>  kernel/bpf/verifier.c                         | 43 ++++++++++++-------
>>  .../selftests/bpf/progs/test_global_func3.c   | 18 ++++----
>>  3 files changed, 42 insertions(+), 25 deletions(-)
>> 
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index c1e30096ea7b..39a54e631bcd 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
>> @@ -650,6 +650,8 @@ enum priv_stack_mode {
>>  	PRIV_STACK_ADAPTIVE,
>>  };
>>  
>> +struct bpf_subprog_info;
>> +
>>  struct bpf_subprog_info {
>>  	/* 'start' has to be the first field otherwise find_subprog() won't work */
>>  	u32 start; /* insn idx of function entry point */
>> @@ -677,6 +679,10 @@ struct bpf_subprog_info {
>>  
>>  	enum priv_stack_mode priv_stack_mode;
>>  	struct bpf_subprog_arg_info args[MAX_BPF_FUNC_REG_ARGS];
>> +
>
> Nit: please add a comment here saying the below are temporary values
>      used in check_max_stack_depth_subprog() (tbh, I'd move those to a
>      separate array in env but maybe that's an overkill).
>
>> +	int ret_insn;
>> +	int frame;
>> +	int cidx;
>
> Nit: please rename to `caller` and add a comment.
>
>>  };

Ack (and to all other nits).

>>  
>>  struct bpf_verifier_env;
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 1153a828ce8d..d362ddd47d71 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -6652,9 +6652,11 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
>>  	struct bpf_insn *insn = env->prog->insnsi;
>>  	int depth = 0, frame = 0, i, subprog_end, subprog_depth;
>>  	bool tail_call_reachable = false;
>> -	int ret_insn[MAX_CALL_FRAMES];
>> -	int ret_prog[MAX_CALL_FRAMES];
>> -	int j;
>> +	int total;
>> +	int tmp;
>> +
>> +	/* no caller idx */
>> +	subprog[idx].cidx = -1;
>>  
>>  	i = subprog[idx].start;
>>  	if (!priv_stack_supported)
>> @@ -6706,8 +6708,11 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
>>  	} else {
>>  		depth += subprog_depth;
>>  		if (depth > MAX_BPF_STACK) {
>> +			for (total = 1; subprog[idx].cidx >= 0 ; total++)
>> +				idx = subprog[idx].cidx;
>> +
>>  			verbose(env, "combined stack size of %d calls is %d. Too large\n",
>> -				frame + 1, depth);
>> +				total, depth);
>>  			return -EACCES;
>>  		}
>>  	}
>> @@ -6723,8 +6728,8 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
>>  				continue;
>>  			if (subprog[idx].is_cb)
>>  				err = true;
>
> Below the old version of the loop does not include current frame, but
> the new version *does* include current frame. Looks like the above
> check can be removed.
>

Makes sense.

>> -			for (int c = 0; c < frame && !err; c++) {
>> -				if (subprog[ret_prog[c]].is_cb) {
>> +			for (tmp = idx; tmp >= 0 && !err; tmp = subprog[tmp].cidx) {
>> +				if (subprog[tmp].is_cb) {
>>  					err = true;
>>  					break;
>>  				}
>
> This code checks that bpf_throw() cannot be called from callbacks.
> The `is_cb` is set in push_callback_call() both for sync and async
> callbacks, it is also set for exception callback separately.
> Meaning that check_return_code() can be simplified further.
>

Do you mean we can simplify the check for choosing between
check_return_code and check_global_subprog_return_code to just check
is_cb instead of checking both is_async_cb and is_exception_cb?

>> @@ -6740,8 +6745,9 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
>>  		if (!bpf_pseudo_call(insn + i) && !bpf_pseudo_func(insn + i))
>>  			continue;
>>  		/* remember insn and function to return to */
>> -		ret_insn[frame] = i + 1;
>> -		ret_prog[frame] = idx;
>> +
>> +		subprog[idx].frame = frame;
>> +		subprog[idx].ret_insn = i + 1;
>
> Nit: move this down to `subprog[sidx].cidx = idx;`, so that the whole
>      "frame" setup is done in one place?
>
>>  
>>  		/* find the callee */
>>  		next_insn = i + insn[i].imm + 1;
>> @@ -6762,6 +6768,9 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
>>  			}
>>  		}
>>  		i = next_insn;
>> +
>> +		/* caller idx */
>> +		subprog[sidx].cidx = idx;
>>  		idx = sidx;
>>  		if (!priv_stack_supported)
>>  			subprog[idx].priv_stack_mode = NO_PRIV_STACK;
>> @@ -6769,7 +6778,7 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
>>  		if (subprog[idx].has_tail_call)
>>  			tail_call_reachable = true;
>>  
>> -		frame++;
>> +		frame = subprog_is_global(env, idx) ? 0 : frame + 1;
>>  		if (frame >= MAX_CALL_FRAMES) {
>>  			verbose(env, "the call stack of %d frames is too deep !\n",
>>  				frame);
>> @@ -6783,12 +6792,12 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
>>  	 * tail call counter throughout bpf2bpf calls combined with tailcalls
>>  	 */
>>  	if (tail_call_reachable)
>
> Unrelated to current patch, but looks like an issue.
> `tail_call_reachable` is not reset anywhere in
> check_max_stack_depth_subprog(). Once it flips to true all calls
> visited afterwards trigger spine to be marked.
>

You're right, this is never unset. I can send a followup patchset for
this along with the followups for the check_return_code patch. I believe
we can change this to eagerly set the subprog's tail_call_reachable flag
directly instead of using a stack variable. 

>> -		for (j = 0; j < frame; j++) {
>> -			if (subprog[ret_prog[j]].is_exception_cb) {
>> +		for (tmp = idx; tmp >= 0; tmp = subprog[tmp].cidx) {
>> +			if (subprog[tmp].is_exception_cb) {
>
> As with previous such loop, the new code seem to include current frame.
> Does this change anything?
>

I think it is ok, though since as you pointed out tail_call_reachable doesn't 
currently seem to work correctly I can only assume: We set tail_call_reachable 
if any of the subprogs called by the current one make a tail call. If
that's the case, the current frame should also count as tail_call_reachable.

It all depends on where tail_call_reachable is supposed to be set and unset. I 
think it's supposed to flow from callee to caller, and if that's the
case we should hoist up the whole loop to where we initially set the
stack variable.

>>  				verbose(env, "cannot tail call within exception cb\n");
>>  				return -EINVAL;
>>  			}
>> -			subprog[ret_prog[j]].tail_call_reachable = true;
>> +			subprog[tmp].tail_call_reachable = true;
>>  		}
>>  	if (subprog[0].tail_call_reachable)
>>  		env->prog->aux->tail_call_reachable = true;
>> @@ -6796,13 +6805,15 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
>>  	/* end of for() loop means the last insn of the 'subprog'
>>  	 * was reached. Doesn't matter whether it was JA or EXIT
>>  	 */
>> -	if (frame == 0)
>> +	if (frame == 0 && subprog[idx].cidx < 0)
>>  		return 0;
>>  	if (subprog[idx].priv_stack_mode != PRIV_STACK_ADAPTIVE)
>>  		depth -= round_up_stack_depth(env, subprog[idx].stack_depth);
>> -	frame--;
>> -	i = ret_insn[frame];
>> -	idx = ret_prog[frame];
>> +
>> +	idx = subprog[idx].cidx;
>> +	frame = subprog[idx].frame;
>> +	i = subprog[idx].ret_insn;
>> +
>>  	goto continue_func;
>>  }
>>  
>
> [...]