[PATCH nf v2] netfilter: ipset: pernet ops must be unregistered last

[PATCH nf v2] netfilter: ipset: pernet ops must be unregistered last

[Florian Westphal]

Removing the ipset module leaves a small window where one cpu performs
module removal while another runs a command like 'ipset flush'.

ipset uses net_generic(), unregistering the pernet ops frees this
storage area.

Fix it by first removing the user-visible api handlers and the pernet
ops last.

Fixes: 1785e8f473082 ("netfiler: ipset: Add net namespace for ipset")
Reported-by: Li Shuang <shuali@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 Change since v2:
  - unregister setsockopt first (i.e., use reverse order)

 net/netfilter/ipset/ip_set_core.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index e495b5e484b1..a7f049ff3049 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -2072,25 +2072,28 @@ static struct pernet_operations ip_set_net_ops = {
 static int __init
 ip_set_init(void)
 {
-	int ret = nfnetlink_subsys_register(&ip_set_netlink_subsys);
+	int ret = register_pernet_subsys(&ip_set_net_ops);
 
+	if (ret) {
+		pr_err("ip_set: cannot register pernet_subsys.\n");
+		return ret;
+	}
+
+	ret = nfnetlink_subsys_register(&ip_set_netlink_subsys);
 	if (ret != 0) {
 		pr_err("ip_set: cannot register with nfnetlink.\n");
+		unregister_pernet_subsys(&ip_set_net_ops);
 		return ret;
 	}
+
 	ret = nf_register_sockopt(&so_set);
 	if (ret != 0) {
 		pr_err("SO_SET registry failed: %d\n", ret);
 		nfnetlink_subsys_unregister(&ip_set_netlink_subsys);
+		unregister_pernet_subsys(&ip_set_net_ops);
 		return ret;
 	}
-	ret = register_pernet_subsys(&ip_set_net_ops);
-	if (ret) {
-		pr_err("ip_set: cannot register pernet_subsys.\n");
-		nf_unregister_sockopt(&so_set);
-		nfnetlink_subsys_unregister(&ip_set_netlink_subsys);
-		return ret;
-	}
+
 	pr_info("ip_set: protocol %u\n", IPSET_PROTOCOL);
 	return 0;
 }
@@ -2098,9 +2101,10 @@ ip_set_init(void)
 static void __exit
 ip_set_fini(void)
 {
-	unregister_pernet_subsys(&ip_set_net_ops);
 	nf_unregister_sockopt(&so_set);
 	nfnetlink_subsys_unregister(&ip_set_netlink_subsys);
+
+	unregister_pernet_subsys(&ip_set_net_ops);
 	pr_debug("these are the famous last words\n");
 }
 
-- 
2.13.5

Re: [PATCH nf v2] netfilter: ipset: pernet ops must be unregistered last

[Jozsef Kadlecsik]

On Tue, 26 Sep 2017, Florian Westphal wrote:

> Removing the ipset module leaves a small window where one cpu performs
> module removal while another runs a command like 'ipset flush'.
> 
> ipset uses net_generic(), unregistering the pernet ops frees this
> storage area.
> 
> Fix it by first removing the user-visible api handlers and the pernet
> ops last.
> 
> Fixes: 1785e8f473082 ("netfiler: ipset: Add net namespace for ipset")
> Reported-by: Li Shuang <shuali@redhat.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>

Acked-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>

Pablo, could you apply the patch in the nf tree?

Best regards,
Jozsef

> ---
>  Change since v2:
>   - unregister setsockopt first (i.e., use reverse order)
> 
>  net/netfilter/ipset/ip_set_core.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
> index e495b5e484b1..a7f049ff3049 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -2072,25 +2072,28 @@ static struct pernet_operations ip_set_net_ops = {
>  static int __init
>  ip_set_init(void)
>  {
> -	int ret = nfnetlink_subsys_register(&ip_set_netlink_subsys);
> +	int ret = register_pernet_subsys(&ip_set_net_ops);
>  
> +	if (ret) {
> +		pr_err("ip_set: cannot register pernet_subsys.\n");
> +		return ret;
> +	}
> +
> +	ret = nfnetlink_subsys_register(&ip_set_netlink_subsys);
>  	if (ret != 0) {
>  		pr_err("ip_set: cannot register with nfnetlink.\n");
> +		unregister_pernet_subsys(&ip_set_net_ops);
>  		return ret;
>  	}
> +
>  	ret = nf_register_sockopt(&so_set);
>  	if (ret != 0) {
>  		pr_err("SO_SET registry failed: %d\n", ret);
>  		nfnetlink_subsys_unregister(&ip_set_netlink_subsys);
> +		unregister_pernet_subsys(&ip_set_net_ops);
>  		return ret;
>  	}
> -	ret = register_pernet_subsys(&ip_set_net_ops);
> -	if (ret) {
> -		pr_err("ip_set: cannot register pernet_subsys.\n");
> -		nf_unregister_sockopt(&so_set);
> -		nfnetlink_subsys_unregister(&ip_set_netlink_subsys);
> -		return ret;
> -	}
> +
>  	pr_info("ip_set: protocol %u\n", IPSET_PROTOCOL);
>  	return 0;
>  }
> @@ -2098,9 +2101,10 @@ ip_set_init(void)
>  static void __exit
>  ip_set_fini(void)
>  {
> -	unregister_pernet_subsys(&ip_set_net_ops);
>  	nf_unregister_sockopt(&so_set);
>  	nfnetlink_subsys_unregister(&ip_set_netlink_subsys);
> +
> +	unregister_pernet_subsys(&ip_set_net_ops);
>  	pr_debug("these are the famous last words\n");
>  }
>  
> -- 
> 2.13.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH nf v2] netfilter: ipset: pernet ops must be unregistered last

[Pablo Neira Ayuso]

On Tue, Sep 26, 2017 at 07:48:45PM +0200, Jozsef Kadlecsik wrote:
> On Tue, 26 Sep 2017, Florian Westphal wrote:
> 
> > Removing the ipset module leaves a small window where one cpu performs
> > module removal while another runs a command like 'ipset flush'.
> > 
> > ipset uses net_generic(), unregistering the pernet ops frees this
> > storage area.
> > 
> > Fix it by first removing the user-visible api handlers and the pernet
> > ops last.
> > 
> > Fixes: 1785e8f473082 ("netfiler: ipset: Add net namespace for ipset")
> > Reported-by: Li Shuang <shuali@redhat.com>
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> 
> Acked-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
> 
> Pablo, could you apply the patch in the nf tree?

Applied, thanks Jozsef.