Skip to contentSkip to conversation

netfilter: ipset: pernet ops must be unregistered last

PATCH Applied Florian Westphal 3 participants 3 messages Sep 26, 2017

Subsystems (2)
Networking netfilter
lore
Share & tools
lore

Florian Westphal

author maintainer
#0 lore raw

PATCH v2 1/1 netfilter: ipset: pernet ops must be unregistered last

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 <[email protected]>
Signed-off-by Florian Westphal <[email protected]>

---
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(-)
net/netfilter/ipset/ip_set_core.c +13 -9
index e495b5e484b1..a7f049ff3049 100644
@@ -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");
 }
Signature · 1 line
2.13.5
Acked-by on v2 1/1
#1 lore raw
Quoted 6 lines · 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.

Quoted 6 lines

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 <[email protected]>
Signed-off-by Florian Westphal <[email protected]>
Acked-by Jozsef Kadlecsik <[email protected]>

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

Best regards,
Jozsef

Quoted 68 lines

---
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(-)
net/netfilter/ipset/ip_set_core.c +13 -9
index e495b5e484b1..a7f049ff3049 100644
@@ -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");
 }
Signature · 6 lines
2.13.5

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

-
E-mail : [email protected], [email protected] 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
maintainer Applied
#2 lore raw
Quoted 11 lines · Jozsef Kadlecsik
Quoted 9 lines · 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.

Quoted 7 lines
Quoted 3 lines
Fixes 1785e8f473082 ("netfiler: ipset: Add net namespace for ipset")
Reported-by Li Shuang <[email protected]>
Signed-off-by Florian Westphal <[email protected]>
Acked-by Jozsef Kadlecsik <[email protected]>

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

Applied, thanks Jozsef.