netfilter: ebtables: close dangling table module init race

sashiko reported for a related patch:
 In modules like iptable_raw.c, [..], if register_pernet_subsys() fails,
 the rollback might call kfree(rawtable_ops) before [..]
 During this window, could a concurrent userspace process find the globally
 visible template, trigger table_init(), [..]

The table init functions must always register the template last.

Otherwise, set/getsockopt can instantiate a table in a namespace
while the required pernet ops (contain the destructor) isn't available.
This change is also required in x_tables, handled in followup change.

Fixes: 87663c39f898 ("netfilter: ebtables: do not hook tables by default")
Reviewed-by: Tristan Madani <tristan@talencesecurity.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/net/bridge/netfilter/ebtable_broute.c b/net/bridge/netfilter/ebtable_broute.c
index e6f9e343b41f1a1a078f4cfa2ebad6a73e80e4e6..f05c79f215ea0622f48e6c8230d3c89af1fb3aeb 100644 (file)
--- a/net/bridge/netfilter/ebtable_broute.c
+++ b/net/bridge/netfilter/ebtable_broute.c
@@ -112,18 +112,16 @@ static struct pernet_operations broute_net_ops = {
 
 static int __init ebtable_broute_init(void)
 {
-       int ret = ebt_register_template(&broute_table, broute_table_init);
+       int ret = register_pernet_subsys(&broute_net_ops);
 
        if (ret)
                return ret;
 
-       ret = register_pernet_subsys(&broute_net_ops);
-       if (ret) {
-               ebt_unregister_template(&broute_table);
-               return ret;
-       }
+       ret = ebt_register_template(&broute_table, broute_table_init);
+       if (ret)
+               unregister_pernet_subsys(&broute_net_ops);
 
-       return 0;
+       return ret;
 }
 
 static void __exit ebtable_broute_fini(void)

diff --git a/net/bridge/netfilter/ebtable_filter.c b/net/bridge/netfilter/ebtable_filter.c
index 02b6501c15a5e3dc979959f2c58c287cf195fb6d..0fc03b07e62aebf978c898d11adffc27b30e99e1 100644 (file)
--- a/net/bridge/netfilter/ebtable_filter.c
+++ b/net/bridge/netfilter/ebtable_filter.c
@@ -93,18 +93,16 @@ static struct pernet_operations frame_filter_net_ops = {
 
 static int __init ebtable_filter_init(void)
 {
-       int ret = ebt_register_template(&frame_filter, frame_filter_table_init);
+       int ret = register_pernet_subsys(&frame_filter_net_ops);
 
        if (ret)
                return ret;
 
-       ret = register_pernet_subsys(&frame_filter_net_ops);
-       if (ret) {
-               ebt_unregister_template(&frame_filter);
-               return ret;
-       }
+       ret = ebt_register_template(&frame_filter, frame_filter_table_init);
+       if (ret)
+               unregister_pernet_subsys(&frame_filter_net_ops);
 
-       return 0;
+       return ret;
 }
 
 static void __exit ebtable_filter_fini(void)

diff --git a/net/bridge/netfilter/ebtable_nat.c b/net/bridge/netfilter/ebtable_nat.c
index 9985a82555c41cbbc2a5286f1a31e74d188bf122..8a10375d890992c14e59104585442d8075b633eb 100644 (file)
--- a/net/bridge/netfilter/ebtable_nat.c
+++ b/net/bridge/netfilter/ebtable_nat.c
@@ -93,16 +93,14 @@ static struct pernet_operations frame_nat_net_ops = {
 
 static int __init ebtable_nat_init(void)
 {
-       int ret = ebt_register_template(&frame_nat, frame_nat_table_init);
+       int ret = register_pernet_subsys(&frame_nat_net_ops);
 
        if (ret)
                return ret;
 
-       ret = register_pernet_subsys(&frame_nat_net_ops);
-       if (ret) {
-               ebt_unregister_template(&frame_nat);
-               return ret;
-       }
+       ret = ebt_register_template(&frame_nat, frame_nat_table_init);
+       if (ret)
+               unregister_pernet_subsys(&frame_nat_net_ops);
 
        return ret;
 }