netfilter: nf_tables: imbalance in flowtable binding

All these cases cause imbalance between BIND and UNBIND calls:

- Delete an interface from a flowtable with multiple interfaces

- Add a (device to a) flowtable with --check flag

- Delete a netns containing a flowtable

- In an interactive nft session, create a table with owner flag and
  flowtable inside, then quit.

Fix it by calling FLOW_BLOCK_UNBIND when unregistering hooks, then
remove late FLOW_BLOCK_UNBIND call when destroying flowtable.

Fixes: ff4bf2f42a40 ("netfilter: nf_tables: add nft_unregister_flowtable_hook()")
Reported-by: Phil Sutter <phil@nwl.cc>
Tested-by: Phil Sutter <phil@nwl.cc>
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 0b9f1e8dfe49b8e15365e5a5a1fa8ef751ac9877..c4af283356e741cc9ed473639060030775e0d942 100644 (file)
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -8822,6 +8822,7 @@ static void nft_unregister_flowtable_hook(struct net *net,
 }
 
 static void __nft_unregister_flowtable_net_hooks(struct net *net,
+                                                struct nft_flowtable *flowtable,
                                                 struct list_head *hook_list,
                                                 bool release_netdev)
 {
@@ -8829,6 +8830,8 @@ static void __nft_unregister_flowtable_net_hooks(struct net *net,
 
        list_for_each_entry_safe(hook, next, hook_list, list) {
                nf_unregister_net_hook(net, &hook->ops);
+               flowtable->data.type->setup(&flowtable->data, hook->ops.dev,
+                                           FLOW_BLOCK_UNBIND);
                if (release_netdev) {
                        list_del(&hook->list);
                        kfree_rcu(hook, rcu);
@@ -8837,9 +8840,10 @@ static void __nft_unregister_flowtable_net_hooks(struct net *net,
 }
 
 static void nft_unregister_flowtable_net_hooks(struct net *net,
+                                              struct nft_flowtable *flowtable,
                                               struct list_head *hook_list)
 {
-       __nft_unregister_flowtable_net_hooks(net, hook_list, false);
+       __nft_unregister_flowtable_net_hooks(net, flowtable, hook_list, false);
 }
 
 static int nft_register_flowtable_net_hooks(struct net *net,
@@ -9481,8 +9485,6 @@ static void nf_tables_flowtable_destroy(struct nft_flowtable *flowtable)
 
        flowtable->data.type->free(&flowtable->data);
        list_for_each_entry_safe(hook, next, &flowtable->hook_list, list) {
-               flowtable->data.type->setup(&flowtable->data, hook->ops.dev,
-                                           FLOW_BLOCK_UNBIND);
                list_del_rcu(&hook->list);
                kfree_rcu(hook, rcu);
        }
@@ -10870,6 +10872,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
                                                           &nft_trans_flowtable_hooks(trans),
                                                           trans->msg_type);
                                nft_unregister_flowtable_net_hooks(net,
+                                                                  nft_trans_flowtable(trans),
                                                                   &nft_trans_flowtable_hooks(trans));
                        } else {
                                list_del_rcu(&nft_trans_flowtable(trans)->list);
@@ -10878,6 +10881,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
                                                           NULL,
                                                           trans->msg_type);
                                nft_unregister_flowtable_net_hooks(net,
+                                               nft_trans_flowtable(trans),
                                                &nft_trans_flowtable(trans)->hook_list);
                        }
                        break;
@@ -11140,11 +11144,13 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
                case NFT_MSG_NEWFLOWTABLE:
                        if (nft_trans_flowtable_update(trans)) {
                                nft_unregister_flowtable_net_hooks(net,
+                                               nft_trans_flowtable(trans),
                                                &nft_trans_flowtable_hooks(trans));
                        } else {
                                nft_use_dec_restore(&table->use);
                                list_del_rcu(&nft_trans_flowtable(trans)->list);
                                nft_unregister_flowtable_net_hooks(net,
+                                               nft_trans_flowtable(trans),
                                                &nft_trans_flowtable(trans)->hook_list);
                        }
                        break;
@@ -11737,7 +11743,8 @@ static void __nft_release_hook(struct net *net, struct nft_table *table)
        list_for_each_entry(chain, &table->chains, list)
                __nf_tables_unregister_hook(net, table, chain, true);
        list_for_each_entry(flowtable, &table->flowtables, list)
-               __nft_unregister_flowtable_net_hooks(net, &flowtable->hook_list,
+               __nft_unregister_flowtable_net_hooks(net, flowtable,
+                                                    &flowtable->hook_list,
                                                     true);
 }