[PATCH 1/1] IPVS: Fix use-after-free issue in ip_vs_unbind_app()

[PATCH 1/1] IPVS: Fix use-after-free issue in ip_vs_unbind_app()

[Slavin Liu]

When exiting a network namespace, in cleanup_net()->ops_undo_list(),
ip_vs_ftp_ops->exit() is called before ip_vs_core_ops->exit_batch().
The ip_vs_app ip_vs_ftp and its incarnations will be freed by unregister_ip_vs_app().
However, there could still be connections bound to ip_vs_ftp's incarnation.
cp->app points to the free'd incarnation, which will be accessed later by
__ip_vs_cleanup_batch()->ip_vs_conn_net_cleanup()->ip_vs_conn_flush()->ip_vs_conn_del()->
ip_vs_conn_expire()->ip_vs_unbind_app(), causing a uaf. This vulnarability can
lead to a local privilege escalation.

Reproduction steps:
1. create a ipvs service on (127.0.0.1:21)
2. create a ipvs destination on the service, to (127.0.0.1:<any>)
3. send a tcp packet to (127.0.0.1:21)
4. exit the network namespace

I think the fix should flush all connection to ftp before unregistration.
The simpler fix is to delete ip_vs_ftp_ops->exit, and defer the unregistration
of ip_vs_ftp to ip_vs_app_net_cleanup(), which will unregister all ip_vs_app.
It's after ip_vs_conn_net_cleanup() so there is no uaf issue. This patch
seems to solve the issue but has't been fully tested yet, and is also not graceful.

Signed-off-by: Slavin Liu <slavin452@gmail.com>
---
 net/netfilter/ipvs/ip_vs_ftp.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index d8a284999544..68def1106681 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -598,22 +598,9 @@ static int __net_init __ip_vs_ftp_init(struct net *net)
 	unregister_ip_vs_app(ipvs, &ip_vs_ftp);
 	return ret;
 }
-/*
- *	netns exit
- */
-static void __ip_vs_ftp_exit(struct net *net)
-{
-	struct netns_ipvs *ipvs = net_ipvs(net);
-
-	if (!ipvs)
-		return;
-
-	unregister_ip_vs_app(ipvs, &ip_vs_ftp);
-}
 
 static struct pernet_operations ip_vs_ftp_ops = {
 	.init = __ip_vs_ftp_init,
-	.exit = __ip_vs_ftp_exit,
 };
 
 static int __init ip_vs_ftp_init(void)
-- 
2.34.1

Re: [PATCH 1/1] IPVS: Fix use-after-free issue in ip_vs_unbind_app()

[Julian Anastasov]

	Hello,

On Mon, 8 Sep 2025, Slavin Liu wrote:

> When exiting a network namespace, in cleanup_net()->ops_undo_list(),
> ip_vs_ftp_ops->exit() is called before ip_vs_core_ops->exit_batch().
> The ip_vs_app ip_vs_ftp and its incarnations will be freed by unregister_ip_vs_app().
> However, there could still be connections bound to ip_vs_ftp's incarnation.
> cp->app points to the free'd incarnation, which will be accessed later by
> __ip_vs_cleanup_batch()->ip_vs_conn_net_cleanup()->ip_vs_conn_flush()->ip_vs_conn_del()->
> ip_vs_conn_expire()->ip_vs_unbind_app(), causing a uaf. This vulnarability can
> lead to a local privilege escalation.
> 
> Reproduction steps:
> 1. create a ipvs service on (127.0.0.1:21)
> 2. create a ipvs destination on the service, to (127.0.0.1:<any>)
> 3. send a tcp packet to (127.0.0.1:21)
> 4. exit the network namespace
> 
> I think the fix should flush all connection to ftp before unregistration.
> The simpler fix is to delete ip_vs_ftp_ops->exit, and defer the unregistration
> of ip_vs_ftp to ip_vs_app_net_cleanup(), which will unregister all ip_vs_app.
> It's after ip_vs_conn_net_cleanup() so there is no uaf issue. This patch
> seems to solve the issue but has't been fully tested yet, and is also not graceful.
> 
> Signed-off-by: Slavin Liu <slavin452@gmail.com>
> ---
>  net/netfilter/ipvs/ip_vs_ftp.c | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
> index d8a284999544..68def1106681 100644
> --- a/net/netfilter/ipvs/ip_vs_ftp.c
> +++ b/net/netfilter/ipvs/ip_vs_ftp.c
> @@ -598,22 +598,9 @@ static int __net_init __ip_vs_ftp_init(struct net *net)
>  	unregister_ip_vs_app(ipvs, &ip_vs_ftp);
>  	return ret;
>  }
> -/*
> - *	netns exit
> - */
> -static void __ip_vs_ftp_exit(struct net *net)
> -{
> -	struct netns_ipvs *ipvs = net_ipvs(net);
> -
> -	if (!ipvs)

	What if we change this 'if' check to:

	if (!ipvs || !ipvs->enable)

	If netns exits, the cleanup order is:

1. exit handlers for pernet device (&ipvs_core_dev_ops) where
	ipvs->enable is set to 0

2. exit handlers for ip_vs_ftp_ops (as last pernet subsys)
	By checking for ipvs->enable, we should not call
	unregister_ip_vs_app() in __ip_vs_ftp_exit() because
	there can be existing conns with valid cp->app

3. exit handlers for pernet subsys (&ipvs_core_ops) where
	ip_vs_app_net_cleanup() unregisters all apps for netns.
	Here the apps will be freed after all conns are gone

	Why we should keep the ftp pernet subsys: because when
there are no conns using the app, the module can be removed and
it must unregister its app from all netns where ipvs->enable will
be 1.

	But note that we have a pending patch that changes
the access to ipvs->enable to use READ_ONCE/WRITE_ONCE:

https://archive.linuxvirtualserver.org/html/lvs-devel/2025-09/msg00000.html

> -		return;
> -
> -	unregister_ip_vs_app(ipvs, &ip_vs_ftp);
> -}
>  
>  static struct pernet_operations ip_vs_ftp_ops = {
>  	.init = __ip_vs_ftp_init,
> -	.exit = __ip_vs_ftp_exit,
>  };
>  
>  static int __init ip_vs_ftp_init(void)
> -- 
> 2.34.1

Regards

--
Julian Anastasov <ja@ssi.bg>

[PATCH v2] ipvs: Check ipvs->enable before unregistration in __ip_vs_ftp_exit()

[Slavin Liu]

On netns cleanup path, before unregistration in __ip_vs_ftp_exit(),
there could still be existing conns with valid cp->app.

Suggested by Julian, this patch fixes this issue by checking ipvs->enable 
to ensure the right order of cleanup:
1. Set ipvs->enable to 0 in ipvs_core_dev_ops->exit_batch()
2. Skip app unregistration in ip_vs_ftp_ops->exit() by 
	checking ipvs->enable
3. Flush all conns in ipvs_core_ops->exit_batch()
4. Unregister all apps in ipvs_core_ops->exit_batch()

Access ipvs->enable by READ_ONCE to avoid concurrency issue.

Suggested-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Slavin Liu <slavin452@gmail.com>
---
 net/netfilter/ipvs/ip_vs_ftp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index d8a284999544..d3e2f7798bf3 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -605,7 +605,7 @@ static void __ip_vs_ftp_exit(struct net *net)
 {
 	struct netns_ipvs *ipvs = net_ipvs(net);
 
-	if (!ipvs)
+	if (!ipvs || !READ_ONCE(ipvs->enable))
 		return;
 
 	unregister_ip_vs_app(ipvs, &ip_vs_ftp);
-- 
2.34.1

Re: [PATCH v2] ipvs: Check ipvs->enable before unregistration in __ip_vs_ftp_exit()

[Julian Anastasov]

	Hello,

On Wed, 10 Sep 2025, Slavin Liu wrote:

> On netns cleanup path, before unregistration in __ip_vs_ftp_exit(),
> there could still be existing conns with valid cp->app.
> 
> Suggested by Julian, this patch fixes this issue by checking ipvs->enable 
> to ensure the right order of cleanup:
> 1. Set ipvs->enable to 0 in ipvs_core_dev_ops->exit_batch()
> 2. Skip app unregistration in ip_vs_ftp_ops->exit() by 
> 	checking ipvs->enable
> 3. Flush all conns in ipvs_core_ops->exit_batch()
> 4. Unregister all apps in ipvs_core_ops->exit_batch()
> 
> Access ipvs->enable by READ_ONCE to avoid concurrency issue.
> 
> Suggested-by: Julian Anastasov <ja@ssi.bg>
> Signed-off-by: Slavin Liu <slavin452@gmail.com>
> ---
>  net/netfilter/ipvs/ip_vs_ftp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
> index d8a284999544..d3e2f7798bf3 100644
> --- a/net/netfilter/ipvs/ip_vs_ftp.c
> +++ b/net/netfilter/ipvs/ip_vs_ftp.c
> @@ -605,7 +605,7 @@ static void __ip_vs_ftp_exit(struct net *net)
>  {
>  	struct netns_ipvs *ipvs = net_ipvs(net);
>  
> -	if (!ipvs)
> +	if (!ipvs || !READ_ONCE(ipvs->enable))

	Ops, I forgot that ipvs->enable is set later when
service is added. May be we have to add some new global flag
for this in ip_vs_ftp.c:

static bool removing;

	We will set it in ip_vs_ftp_exit() before calling
unregister_pernet_subsys() and the above check will become:

	if (!ipvs || !removing)

	Also, we have to add Fixes tag, this looks the
one that starts to remove apps from netns exit handler:

Fixes: 61b1ab4583e2 ("IPVS: netns, add basic init per netns.")

Regards

--
Julian Anastasov <ja@ssi.bg>

[RFC PATCH v3] ipvs: Defer ip_vs_ftp unregister during netns cleanup

[Slavin Liu]

On the netns cleanup path, __ip_vs_ftp_exit() may unregister ip_vs_ftp
before connections with valid cp->app pointers are flushed, leading to a
use-after-free.

Fix this by introducing a global `module_removing` flag, set to true in
ip_vs_ftp_exit() before unregistering the pernet subsystem. In
__ip_vs_ftp_exit(), skip ip_vs_ftp unregister if called during netns
cleanup (when module_removing is false) and defer it to
__ip_vs_cleanup_batch(), which unregisters all apps after all connections
are flushed. If called during module exit, unregister ip_vs_ftp
immediately.

Fixes: 61b1ab4583e2 ("IPVS: netns, add basic init per netns.")
Suggested-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Slavin Liu <slavin452@gmail.com>
---
 net/netfilter/ipvs/ip_vs_ftp.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index d8a284999544..4db58c42ff9a 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -53,6 +53,7 @@ enum {
 	IP_VS_FTP_EPSV,
 };
 
+static bool module_exiting;
 /*
  * List of ports (up to IP_VS_APP_MAX_PORTS) to be handled by helper
  * First port is set to the default port.
@@ -605,7 +606,7 @@ static void __ip_vs_ftp_exit(struct net *net)
 {
 	struct netns_ipvs *ipvs = net_ipvs(net);
 
-	if (!ipvs)
+	if (!ipvs || !module_exiting)
 		return;
 
 	unregister_ip_vs_app(ipvs, &ip_vs_ftp);
@@ -627,7 +628,9 @@ static int __init ip_vs_ftp_init(void)
  */
 static void __exit ip_vs_ftp_exit(void)
 {
+	module_exiting = true;
 	unregister_pernet_subsys(&ip_vs_ftp_ops);
+	module_exiting = false;
 	/* rcu_barrier() is called by netns */
 }
 
-- 
2.34.1

[PATCH v4] ipvs: Defer ip_vs_ftp unregister during netns cleanup

[Slavin Liu]

On the netns cleanup path, __ip_vs_ftp_exit() may unregister ip_vs_ftp
before connections with valid cp->app pointers are flushed, leading to a
use-after-free.

Fix this by introducing a global `exiting_module` flag, set to true in
ip_vs_ftp_exit() before unregistering the pernet subsystem. In
__ip_vs_ftp_exit(), skip ip_vs_ftp unregister if called during netns
cleanup (when module_removing is false) and defer it to
__ip_vs_cleanup_batch(), which unregisters all apps after all connections
are flushed. If called during module exit, unregister ip_vs_ftp
immediately.

Fixes: 61b1ab4583e2 ("IPVS: netns, add basic init per netns.")
Suggested-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Slavin Liu <slavin452@gmail.com>
---
 net/netfilter/ipvs/ip_vs_ftp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index d8a284999544..206c6700e200 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -53,6 +53,7 @@ enum {
 	IP_VS_FTP_EPSV,
 };
 
+static bool exiting_module;
 /*
  * List of ports (up to IP_VS_APP_MAX_PORTS) to be handled by helper
  * First port is set to the default port.
@@ -605,7 +606,7 @@ static void __ip_vs_ftp_exit(struct net *net)
 {
 	struct netns_ipvs *ipvs = net_ipvs(net);
 
-	if (!ipvs)
+	if (!ipvs || !exiting_module)
 		return;
 
 	unregister_ip_vs_app(ipvs, &ip_vs_ftp);
@@ -627,6 +628,7 @@ static int __init ip_vs_ftp_init(void)
  */
 static void __exit ip_vs_ftp_exit(void)
 {
+	exiting_module = true;
 	unregister_pernet_subsys(&ip_vs_ftp_ops);
 	/* rcu_barrier() is called by netns */
 }
-- 
2.34.1

Re: [PATCH v4] ipvs: Defer ip_vs_ftp unregister during netns cleanup

[Julian Anastasov]

	Hello,

On Fri, 12 Sep 2025, Slavin Liu wrote:

> On the netns cleanup path, __ip_vs_ftp_exit() may unregister ip_vs_ftp
> before connections with valid cp->app pointers are flushed, leading to a
> use-after-free.
> 
> Fix this by introducing a global `exiting_module` flag, set to true in
> ip_vs_ftp_exit() before unregistering the pernet subsystem. In
> __ip_vs_ftp_exit(), skip ip_vs_ftp unregister if called during netns
> cleanup (when module_removing is false) and defer it to

	Pablo, can you change here module_removing to exiting_module 
before applying?

> __ip_vs_cleanup_batch(), which unregisters all apps after all connections
> are flushed. If called during module exit, unregister ip_vs_ftp
> immediately.
> 
> Fixes: 61b1ab4583e2 ("IPVS: netns, add basic init per netns.")
> Suggested-by: Julian Anastasov <ja@ssi.bg>
> Signed-off-by: Slavin Liu <slavin452@gmail.com>

	Looks good to me for the nf tree after above text is
changed, thanks!

Signed-off-by: Julian Anastasov <ja@ssi.bg>

> ---
>  net/netfilter/ipvs/ip_vs_ftp.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
> index d8a284999544..206c6700e200 100644
> --- a/net/netfilter/ipvs/ip_vs_ftp.c
> +++ b/net/netfilter/ipvs/ip_vs_ftp.c
> @@ -53,6 +53,7 @@ enum {
>  	IP_VS_FTP_EPSV,
>  };
>  
> +static bool exiting_module;
>  /*
>   * List of ports (up to IP_VS_APP_MAX_PORTS) to be handled by helper
>   * First port is set to the default port.
> @@ -605,7 +606,7 @@ static void __ip_vs_ftp_exit(struct net *net)
>  {
>  	struct netns_ipvs *ipvs = net_ipvs(net);
>  
> -	if (!ipvs)
> +	if (!ipvs || !exiting_module)
>  		return;
>  
>  	unregister_ip_vs_app(ipvs, &ip_vs_ftp);
> @@ -627,6 +628,7 @@ static int __init ip_vs_ftp_init(void)
>   */
>  static void __exit ip_vs_ftp_exit(void)
>  {
> +	exiting_module = true;
>  	unregister_pernet_subsys(&ip_vs_ftp_ops);
>  	/* rcu_barrier() is called by netns */
>  }
> -- 
> 2.34.1

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH v4] ipvs: Defer ip_vs_ftp unregister during netns cleanup

[Florian Westphal]

Julian Anastasov <ja@ssi.bg> wrote:
> > Fixes: 61b1ab4583e2 ("IPVS: netns, add basic init per netns.")
> > Suggested-by: Julian Anastasov <ja@ssi.bg>
> > Signed-off-by: Slavin Liu <slavin452@gmail.com>
> 
> 	Looks good to me for the nf tree after above text is
> changed, thanks!

Thaks Julian for reviewing, i pushed it to nf:testing.