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

        Hello,

On Sat, 12 Sep 2026, Zihan Xi wrote:

quoted

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]