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

[Eduard Zingerman <eddyz87@gmail.com>]

On Thu, 2026-03-05 at 14:38 -0500, Emil Tsalapatis wrote:

[...]

> > > @@ -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?

Actually, I was thinking about `frame->in_async_callback_fn` checks in
check_return_code(). But on a second thought these are unrelated.
Please skip my comment above.

[...]

> > > @@ -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.

So, the tail_call_reachable flag is transferred to subprog[*].tail_call_reachable
property. In check_subprogs() there is direct `subprog[cur_subprog].tail_call_reachable = true`
if tail call exists in the subprogram. Looks like behavior does not really change
with this patch.

> > >  				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;

[...]