Re: [PATCH net v2 1/2] ipvs: avoid stack overflow from recursive connection expiration

[Julian Anastasov <ja@ssi.bg>]

	Hello,

On Fri, 18 Sep 2026, Julian Anastasov wrote:

> On Thu, 17 Sep 2026, Zihan Xi wrote:
> 
> > When a controlled IPVS connection expires, its controller may be expired
> > synchronously if it has no remaining controlled connections. A chain of
> > controlled connections can then cause recursive calls to
> > ip_vs_conn_expire() and exhaust the kernel stack during namespace cleanup.
> > 
> > Make ip_vs_conn_del_put() report whether it deleted the controller timer.
> > When it succeeds, continue expiration with the controller instead of
> > calling ip_vs_conn_expire() recursively. This keeps chain cleanup
> > synchronous while using one stack frame for the whole chain.
> > 
> > Fixes: f9200a52eedf ("ipvs: avoid expiring many connections from timer")
> > Cc: stable@vger.kernel.org
> > Reported-by: Vega <vega@nebusec.ai>
> > Assisted-by: LLM
> > Co-developed-by: Luxing Yin <root@tr0jan.top>
> > Signed-off-by: Luxing Yin <root@tr0jan.top>
> > Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
> 
> 	Patch looks good to me for the nf tree, thanks!
> 
> Acked-by: Julian Anastasov <ja@ssi.bg>

	In fact, Sashiko detects problem with connections
that are deleted and traffic that can restart the timer and
its callback deleteing the connection:

https://sashiko.dev/#/patchset/cover.1789435989.git.zihanx%40nebusec.ai

	Events are in this order:

CPU 1                   CPU 2
timer_delete,
refcnt is 1
                        find conn, get refcnt
                        mod_timer, put refcnt => 1

                        run timer callback
			and expire the conn,
			refcnt=0

touching cp->control is
safe under RCU,
but we mod_timer
with refcnt=0

	The problem is that the timer callback runs without
conn reference and not under RCU lock. OTOH, we delete the
connection only under RCU read lock and can take measures
if the callback removed the connection before us.

	Dropping conns only via timer callback is something we try
to avoid, we have to rethink this change.

pw-bot: changes-requested

> 	Next time use "nf"/"nf-next" tag for the IPVS patches.
> 
> > ---
> > changes in v2:
> >   - Use a repeat path for controller cleanup so expiration stays
> >     synchronous without recursive calls or extra timer ticks.
> >   - v1 Link:
> >     https://lore.kernel.org/all/cover.1789110326.git.zihanx@nebusec.ai/
> > 
> >  net/netfilter/ipvs/ip_vs_conn.c | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> > index 6fa3e1dc534c3..c7b88ce1765dc 100644
> > --- a/net/netfilter/ipvs/ip_vs_conn.c
> > +++ b/net/netfilter/ipvs/ip_vs_conn.c
> > @@ -1331,17 +1331,18 @@ static void ip_vs_conn_del(struct ip_vs_conn *cp)
> >  }
> >  
> >  /* Try to delete connection while holding reference */
> > -static void ip_vs_conn_del_put(struct ip_vs_conn *cp)
> > +static bool ip_vs_conn_del_put(struct ip_vs_conn *cp)
> >  {
> >  	if (timer_delete(&cp->timer)) {
> >  		/* Drop cp->control chain too */
> >  		if (cp->control)
> >  			cp->timeout = 0;
> >  		__ip_vs_conn_put(cp);
> > -		ip_vs_conn_expire(&cp->timer);
> > -	} else {
> > -		__ip_vs_conn_put(cp);
> > +		return true;
> >  	}
> > +
> > +	__ip_vs_conn_put(cp);
> > +	return false;
> >  }
> >  
> >  static void ip_vs_conn_expire(struct timer_list *t)
> > @@ -1349,6 +1350,7 @@ static void ip_vs_conn_expire(struct timer_list *t)
> >  	struct ip_vs_conn *cp = timer_container_of(cp, t, timer);
> >  	struct netns_ipvs *ipvs = cp->ipvs;
> >  
> > +repeat:
> >  	/*
> >  	 *	do I control anybody?
> >  	 */
> > @@ -1358,6 +1360,7 @@ static void ip_vs_conn_expire(struct timer_list *t)
> >  	/* Unlink conn if not referenced anymore */
> >  	if (likely(ip_vs_conn_unlink(cp))) {
> >  		struct ip_vs_conn *ct = cp->control;
> > +		bool next = false;
> >  
> >  		/* delete the timer if it is activated by other users */
> >  		timer_delete(&cp->timer);
> > @@ -1372,7 +1375,7 @@ static void ip_vs_conn_expire(struct timer_list *t)
> >  			    (!(ct->flags & IP_VS_CONN_F_TEMPLATE) ||
> >  			     !(ct->state & IP_VS_CTPL_S_ASSURED))) {
> >  				IP_VS_DBG(4, "drop controlling connection\n");
> > -				ip_vs_conn_del_put(ct);
> > +				next = ip_vs_conn_del_put(ct);
> >  			} else if (has_ref) {
> >  				__ip_vs_conn_put(ct);
> >  			}
> > @@ -1402,6 +1405,10 @@ static void ip_vs_conn_expire(struct timer_list *t)
> >  		else
> >  			call_rcu(&cp->rcu_head, ip_vs_conn_rcu_free);
> >  		atomic_dec(&ipvs->conn_count);
> > +		if (next) {
> > +			cp = ct;
> > +			goto repeat;
> > +		}
> >  		return;
> >  	}
> >  
> > -- 
> > 2.43.0

Regards

--
Julian Anastasov <ja@ssi.bg>