Thread (4 messages) flat view 4 messages, 2 authors, 8d ago

Re: [PATCH nf 0/1] ipvs: avoid stack overflow from recursive connection expiration

From: Julian Anastasov <ja@ssi.bg>
Date: 2026-09-12 17:50:42
Also in: lkml, lvs-devel, netfilter-devel, stable
Subsystem: ipvs, netfilter, networking [general], the rest · Maintainers: Simon Horman, Julian Anastasov, Pablo Neira Ayuso, Florian Westphal, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds

	Hello,

On Sat, 12 Sep 2026, Zihan Xi wrote:
Hi Linux kernel maintainers,

We found and validated an issue triggered through
net/netfilter/ipvs/ip_vs_ftp.c. An unprivileged user can trigger it by
creating a user namespace and a network namespace. We tested the fix with
the same trigger. The change applies to the generic
ip_vs_conn_expire() control-chain cleanup path. For the reported trigger,
it only defers recursive controller expiration; testing showed no change to
other IPVS behavior.

We will provide detailed information about the bug
in this email, along with a PoC to trigger it.

---- details below ----

Bug details:

The trigger entry point is ip_vs_ftp_out() in
net/netfilter/ipvs/ip_vs_ftp.c. It parses an EPSV reply and creates a
wildcard data connection using the advertised port. If that port is 21,
ip_vs_conn_new() binds the new connection to the FTP helper a second time,
because 21 is the helper's control port. The connection has
IP_VS_CONN_F_NO_CPORT, so the next connection from the same client to the
VIP on port 21 matches the wildcard entry instead of creating a new
top-level entry. Repeating EPSV builds a chain of controlled connections.
	Hm, may be we should also avoid such long chains.
Probably in separate patch, for example:
diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index 9e3e005a8263..0622169b5970 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -237,6 +237,17 @@ static int ip_vs_ftp_get_addrport(char *data, char *data_limit,
 	return 1;
 }
 
+static bool is_control_port(u16 port)
+{
+	int i;
+
+	for (i = 0; i < ports_count; i++) {
+		if (ports[i] == port)
+			return true;
+	}
+	return false;
+}
+
 /* Look at outgoing ftp packets to catch the response to a PASV/EPSV command
  * from the server (inside-to-outside).
  * When we see one, we build a connection entry with the client address,
@@ -293,6 +304,9 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
 
 		IP_VS_DBG(7, "PASV response (%pI4:%u) -> %pI4:%u detected\n",
 			  &from.ip, ntohs(port), &cp->caddr.ip, 0);
+		/* Do not redirect data to control ports */
+		if (!port || is_control_port(ntohs(port)))
+			return 0;
 	} else if (cp->app_data == (void *) IP_VS_FTP_EPSV) {
 		data = ip_vs_ftp_data_ptr(skb, ipvsh);
 		data_limit = skb_tail_pointer(skb);
@@ -529,6 +543,9 @@ static int ip_vs_ftp_in(struct ip_vs_app *app, struct ip_vs_conn *cp,
 		return 1;
 	}
 
+	if (!port)
+		return 0;
+
 	/* Passive mode off */
 	cp->app_data = (void *) IP_VS_FTP_ACTIVE;
	The only problem I see with your proposed change is
that we may need 2-3 timer ticks to expire a DATA->CTL->TPL
chain. Or it expires on the same tick?

	Alternative would be to jump to the beginnig of the
function after successful timer_delete() for our cp->control,
i.e. to use loop instead of recursion. I.e. ip_vs_conn_del_put()
can be converted to function that returns bool instead of
calling ip_vs_conn_expire(), so that we can know if to loop.

	If ip_vs_conn_flush() demands faster expiring, a
loop will work faster. What do you think?

Regards

--
Julian Anastasov [off-list ref]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help