netfilter: nf_tables: use list_del_rcu for netlink hooks

nft_netdev_unregister_hooks and __nft_unregister_flowtable_net_hooks need
to use list_del_rcu(), this list can be walked by concurrent dumpers.

Add a new helper and use it consistently.

Fixes: f9a43007d3f7 ("netfilter: nf_tables: double hook unregistration in netns path")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 8537b94653d374f9a82e7e377650a30578e62749..07e1512457650fbbd50f204e28561fc1c5c69138 100644 (file)
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -374,6 +374,12 @@ static void nft_netdev_hook_free_rcu(struct nft_hook *hook)
        call_rcu(&hook->rcu, __nft_netdev_hook_free_rcu);
 }
 
+static void nft_netdev_hook_unlink_free_rcu(struct nft_hook *hook)
+{
+       list_del_rcu(&hook->list);
+       nft_netdev_hook_free_rcu(hook);
+}
+
 static void nft_netdev_unregister_hooks(struct net *net,
                                        struct list_head *hook_list,
                                        bool release_netdev)
@@ -384,10 +390,8 @@ static void nft_netdev_unregister_hooks(struct net *net,
        list_for_each_entry_safe(hook, next, hook_list, list) {
                list_for_each_entry(ops, &hook->ops_list, list)
                        nf_unregister_net_hook(net, ops);
-               if (release_netdev) {
-                       list_del(&hook->list);
-                       nft_netdev_hook_free_rcu(hook);
-               }
+               if (release_netdev)
+                       nft_netdev_hook_unlink_free_rcu(hook);
        }
 }
 
@@ -2271,10 +2275,8 @@ void nf_tables_chain_destroy(struct nft_chain *chain)
 
                if (nft_base_chain_netdev(table->family, basechain->ops.hooknum)) {
                        list_for_each_entry_safe(hook, next,
-                                                &basechain->hook_list, list) {
-                               list_del_rcu(&hook->list);
-                               nft_netdev_hook_free_rcu(hook);
-                       }
+                                                &basechain->hook_list, list)
+                               nft_netdev_hook_unlink_free_rcu(hook);
                }
                module_put(basechain->type->owner);
                if (rcu_access_pointer(basechain->stats)) {
@@ -2974,6 +2976,7 @@ err_hooks:
                                list_for_each_entry(ops, &h->ops_list, list)
                                        nf_unregister_net_hook(ctx->net, ops);
                        }
+                       /* hook.list is on stack, no need for list_del_rcu() */
                        list_del(&h->list);
                        nft_netdev_hook_free_rcu(h);
                }
@@ -8852,10 +8855,8 @@ static void __nft_unregister_flowtable_net_hooks(struct net *net,
        list_for_each_entry_safe(hook, next, hook_list, list) {
                list_for_each_entry(ops, &hook->ops_list, list)
                        nft_unregister_flowtable_ops(net, flowtable, ops);
-               if (release_netdev) {
-                       list_del(&hook->list);
-                       nft_netdev_hook_free_rcu(hook);
-               }
+               if (release_netdev)
+                       nft_netdev_hook_unlink_free_rcu(hook);
        }
 }
 
@@ -8926,8 +8927,7 @@ err_unregister_net_hooks:
 
                        nft_unregister_flowtable_ops(net, flowtable, ops);
                }
-               list_del_rcu(&hook->list);
-               nft_netdev_hook_free_rcu(hook);
+               nft_netdev_hook_unlink_free_rcu(hook);
        }
 
        return err;
@@ -8937,10 +8937,8 @@ static void nft_hooks_destroy(struct list_head *hook_list)
 {
        struct nft_hook *hook, *next;
 
-       list_for_each_entry_safe(hook, next, hook_list, list) {
-               list_del_rcu(&hook->list);
-               nft_netdev_hook_free_rcu(hook);
-       }
+       list_for_each_entry_safe(hook, next, hook_list, list)
+               nft_netdev_hook_unlink_free_rcu(hook);
 }
 
 static int nft_flowtable_update(struct nft_ctx *ctx, const struct nlmsghdr *nlh,
@@ -9028,8 +9026,7 @@ err_flowtable_update_hook:
                                nft_unregister_flowtable_ops(ctx->net,
                                                             flowtable, ops);
                }
-               list_del_rcu(&hook->list);
-               nft_netdev_hook_free_rcu(hook);
+               nft_netdev_hook_unlink_free_rcu(hook);
        }
 
        return err;
@@ -9535,13 +9532,8 @@ err:
 
 static void nf_tables_flowtable_destroy(struct nft_flowtable *flowtable)
 {
-       struct nft_hook *hook, *next;
-
        flowtable->data.type->free(&flowtable->data);
-       list_for_each_entry_safe(hook, next, &flowtable->hook_list, list) {
-               list_del_rcu(&hook->list);
-               nft_netdev_hook_free_rcu(hook);
-       }
+       nft_hooks_destroy(&flowtable->hook_list);
        kfree(flowtable->name);
        module_put(flowtable->data.type->owner);
        kfree(flowtable);