1. Patchew
  2. linux
  3. View series

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

Slavin Liu posted 1 patch 3 weeks, 2 days ago 
net/netfilter/ipvs/ip_vs_ftp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v2] ipvs: Check ipvs->enable before unregistration in __ip_vs_ftp_exit()
Posted by Slavin Liu 3 weeks, 2 days ago 
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()
Posted by Julian Anastasov 3 weeks, 1 day ago 
	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
Posted by Slavin Liu 3 weeks ago 
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
Posted by Slavin Liu 3 weeks ago 
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
Posted by Julian Anastasov 2 weeks, 5 days ago 
	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
Posted by Florian Westphal 2 weeks, 5 days ago 
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.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.