netfilter: ebtables: move to two-stage removal scheme

Like previous patches for x_tables, follow same pattern in ebtables.
We can't reuse xt helpers: ebt_table struct layout is incompatible.

table->ops assignment is now done while still holding the ebt mutex
to make sure we never expose partially-filled table struct.

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 741360219552574f048e1135014ac7b8082273e6..e6f9e343b41f1a1a078f4cfa2ebad6a73e80e4e6 100644 (file)
--- a/net/bridge/netfilter/ebtable_broute.c
+++ b/net/bridge/netfilter/ebtable_broute.c
@@ -128,8 +128,8 @@ static int __init ebtable_broute_init(void)
 
 static void __exit ebtable_broute_fini(void)
 {
-       unregister_pernet_subsys(&broute_net_ops);
        ebt_unregister_template(&broute_table);
+       unregister_pernet_subsys(&broute_net_ops);
 }
 
 module_init(ebtable_broute_init);

diff --git a/net/bridge/netfilter/ebtable_filter.c b/net/bridge/netfilter/ebtable_filter.c
index dacd81b12e62645b8866ac1071261275280a365d..02b6501c15a5e3dc979959f2c58c287cf195fb6d 100644 (file)
--- a/net/bridge/netfilter/ebtable_filter.c
+++ b/net/bridge/netfilter/ebtable_filter.c
@@ -109,8 +109,8 @@ static int __init ebtable_filter_init(void)
 
 static void __exit ebtable_filter_fini(void)
 {
-       unregister_pernet_subsys(&frame_filter_net_ops);
        ebt_unregister_template(&frame_filter);
+       unregister_pernet_subsys(&frame_filter_net_ops);
 }
 
 module_init(ebtable_filter_init);

diff --git a/net/bridge/netfilter/ebtable_nat.c b/net/bridge/netfilter/ebtable_nat.c
index 0f2a8c6118d42ef47904e4dd8dcd7bcab26e3304..9985a82555c41cbbc2a5286f1a31e74d188bf122 100644 (file)
--- a/net/bridge/netfilter/ebtable_nat.c
+++ b/net/bridge/netfilter/ebtable_nat.c
@@ -109,8 +109,8 @@ static int __init ebtable_nat_init(void)
 
 static void __exit ebtable_nat_fini(void)
 {
-       unregister_pernet_subsys(&frame_nat_net_ops);
        ebt_unregister_template(&frame_nat);
+       unregister_pernet_subsys(&frame_nat_net_ops);
 }
 
 module_init(ebtable_nat_init);

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index aea3e19875c69d513e6dc7f3bc36c742e5a6bf6f..3578ffbc14aee35af3d28b6a17c40e31bb11fbde 100644 (file)
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -42,6 +42,7 @@
 
 struct ebt_pernet {
        struct list_head tables;
+       struct list_head dead_tables;
 };
 
 struct ebt_template {
@@ -1162,11 +1163,6 @@ free_newinfo:
 
 static void __ebt_unregister_table(struct net *net, struct ebt_table *table)
 {
-       mutex_lock(&ebt_mutex);
-       list_del(&table->list);
-       mutex_unlock(&ebt_mutex);
-       audit_log_nfcfg(table->name, AF_BRIDGE, table->private->nentries,
-                       AUDIT_XT_OP_UNREGISTER, GFP_KERNEL);
        EBT_ENTRY_ITERATE(table->private->entries, table->private->entries_size,
                          ebt_cleanup_entry, net, NULL);
        if (table->private->nentries)
@@ -1267,13 +1263,15 @@ int ebt_register_table(struct net *net, const struct ebt_table *input_table,
        for (i = 0; i < num_ops; i++)
                ops[i].priv = table;
 
-       list_add(&table->list, &ebt_net->tables);
-       mutex_unlock(&ebt_mutex);
-
        table->ops = ops;
        ret = nf_register_net_hooks(net, ops, num_ops);
-       if (ret)
+       if (ret) {
+               synchronize_rcu();
                __ebt_unregister_table(net, table);
+       } else {
+               list_add(&table->list, &ebt_net->tables);
+       }
+       mutex_unlock(&ebt_mutex);
 
        audit_log_nfcfg(repl->name, AF_BRIDGE, repl->nentries,
                        AUDIT_XT_OP_REGISTER, GFP_KERNEL);
@@ -1339,7 +1337,7 @@ void ebt_unregister_template(const struct ebt_table *t)
 }
 EXPORT_SYMBOL(ebt_unregister_template);
 
-static struct ebt_table *__ebt_find_table(struct net *net, const char *name)
+void ebt_unregister_table_pre_exit(struct net *net, const char *name)
 {
        struct ebt_pernet *ebt_net = net_generic(net, ebt_pernet_id);
        struct ebt_table *t;
@@ -1348,30 +1346,36 @@ static struct ebt_table *__ebt_find_table(struct net *net, const char *name)
 
        list_for_each_entry(t, &ebt_net->tables, list) {
                if (strcmp(t->name, name) == 0) {
+                       list_move(&t->list, &ebt_net->dead_tables);
                        mutex_unlock(&ebt_mutex);
-                       return t;
+                       nf_unregister_net_hooks(net, t->ops, hweight32(t->valid_hooks));
+                       return;
                }
        }
 
        mutex_unlock(&ebt_mutex);
-       return NULL;
-}
-
-void ebt_unregister_table_pre_exit(struct net *net, const char *name)
-{
-       struct ebt_table *table = __ebt_find_table(net, name);
-
-       if (table)
-               nf_unregister_net_hooks(net, table->ops, hweight32(table->valid_hooks));
 }
 EXPORT_SYMBOL(ebt_unregister_table_pre_exit);
 
 void ebt_unregister_table(struct net *net, const char *name)
 {
-       struct ebt_table *table = __ebt_find_table(net, name);
+       struct ebt_pernet *ebt_net = net_generic(net, ebt_pernet_id);
+       struct ebt_table *t;
 
-       if (table)
-               __ebt_unregister_table(net, table);
+       mutex_lock(&ebt_mutex);
+
+       list_for_each_entry(t, &ebt_net->dead_tables, list) {
+               if (strcmp(t->name, name) == 0) {
+                       list_del(&t->list);
+                       audit_log_nfcfg(t->name, AF_BRIDGE, t->private->nentries,
+                                       AUDIT_XT_OP_UNREGISTER, GFP_KERNEL);
+                       __ebt_unregister_table(net, t);
+                       mutex_unlock(&ebt_mutex);
+                       return;
+               }
+       }
+
+       mutex_unlock(&ebt_mutex);
 }
 
 /* userspace just supplied us with counters */
@@ -2556,11 +2560,21 @@ static int __net_init ebt_pernet_init(struct net *net)
        struct ebt_pernet *ebt_net = net_generic(net, ebt_pernet_id);
 
        INIT_LIST_HEAD(&ebt_net->tables);
+       INIT_LIST_HEAD(&ebt_net->dead_tables);
        return 0;
 }
 
+static void __net_exit ebt_pernet_exit(struct net *net)
+{
+       struct ebt_pernet *ebt_net = net_generic(net, ebt_pernet_id);
+
+       WARN_ON_ONCE(!list_empty(&ebt_net->tables));
+       WARN_ON_ONCE(!list_empty(&ebt_net->dead_tables));
+}
+
 static struct pernet_operations ebt_net_ops = {
        .init = ebt_pernet_init,
+       .exit = ebt_pernet_exit,
        .id   = &ebt_pernet_id,
        .size = sizeof(struct ebt_pernet),
 };