aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorFlorian Westphal <fw@strlen.de>2018-08-02 15:44:41 -0400
committerPablo Neira Ayuso <pablo@netfilter.org>2018-08-16 13:37:03 -0400
commit6a48de0144767f2c6880540c0a4ac6741e3c440b (patch)
treef3111dac7df6aa291d8d8b60dcb50f5a508c9d70
parentd209df3e7f7002d9099fdb0f6df0f972b4386a63 (diff)
netfilter: nf_tables: don't prevent event handler from device cleanup on netns exit
When a netnsamespace exits, the nf_tables pernet_ops will remove all rules. However, there is one caveat: Base chains that register ingress hooks will cause use-after-free: device is already gone at that point. The device event handlers prevent this from happening: netns exit synthesizes unregister events for all devices. However, an improper fix for a race condition made the notifiers a no-op in case they get called from netns exit path, so revert that part. This is safe now as the previous patch fixed nf_tables pernet ops and device notifier initialisation ordering. Fixes: 0a2cf5ee432c2 ("netfilter: nf_tables: close race between netns exit and rmmod") Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
-rw-r--r--net/netfilter/nf_tables_api.c7
-rw-r--r--net/netfilter/nft_chain_filter.c12
2 files changed, 9 insertions, 10 deletions
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 80636cc59686..1dca5683f59f 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -5925,10 +5925,7 @@ static int nf_tables_flowtable_event(struct notifier_block *this,
5925 if (event != NETDEV_UNREGISTER) 5925 if (event != NETDEV_UNREGISTER)
5926 return 0; 5926 return 0;
5927 5927
5928 net = maybe_get_net(dev_net(dev)); 5928 net = dev_net(dev);
5929 if (!net)
5930 return 0;
5931
5932 mutex_lock(&net->nft.commit_mutex); 5929 mutex_lock(&net->nft.commit_mutex);
5933 list_for_each_entry(table, &net->nft.tables, list) { 5930 list_for_each_entry(table, &net->nft.tables, list) {
5934 list_for_each_entry(flowtable, &table->flowtables, list) { 5931 list_for_each_entry(flowtable, &table->flowtables, list) {
@@ -5936,7 +5933,7 @@ static int nf_tables_flowtable_event(struct notifier_block *this,
5936 } 5933 }
5937 } 5934 }
5938 mutex_unlock(&net->nft.commit_mutex); 5935 mutex_unlock(&net->nft.commit_mutex);
5939 put_net(net); 5936
5940 return NOTIFY_DONE; 5937 return NOTIFY_DONE;
5941} 5938}
5942 5939
diff --git a/net/netfilter/nft_chain_filter.c b/net/netfilter/nft_chain_filter.c
index 9d07b277b9ee..3fd540b2c6ba 100644
--- a/net/netfilter/nft_chain_filter.c
+++ b/net/netfilter/nft_chain_filter.c
@@ -293,6 +293,13 @@ static void nft_netdev_event(unsigned long event, struct net_device *dev,
293 if (strcmp(basechain->dev_name, dev->name) != 0) 293 if (strcmp(basechain->dev_name, dev->name) != 0)
294 return; 294 return;
295 295
296 /* UNREGISTER events are also happpening on netns exit.
297 *
298 * Altough nf_tables core releases all tables/chains, only
299 * this event handler provides guarantee that
300 * basechain.ops->dev is still accessible, so we cannot
301 * skip exiting net namespaces.
302 */
296 __nft_release_basechain(ctx); 303 __nft_release_basechain(ctx);
297 break; 304 break;
298 case NETDEV_CHANGENAME: 305 case NETDEV_CHANGENAME:
@@ -318,10 +325,6 @@ static int nf_tables_netdev_event(struct notifier_block *this,
318 event != NETDEV_CHANGENAME) 325 event != NETDEV_CHANGENAME)
319 return NOTIFY_DONE; 326 return NOTIFY_DONE;
320 327
321 ctx.net = maybe_get_net(ctx.net);
322 if (!ctx.net)
323 return NOTIFY_DONE;
324
325 mutex_lock(&ctx.net->nft.commit_mutex); 328 mutex_lock(&ctx.net->nft.commit_mutex);
326 list_for_each_entry(table, &ctx.net->nft.tables, list) { 329 list_for_each_entry(table, &ctx.net->nft.tables, list) {
327 if (table->family != NFPROTO_NETDEV) 330 if (table->family != NFPROTO_NETDEV)
@@ -338,7 +341,6 @@ static int nf_tables_netdev_event(struct notifier_block *this,
338 } 341 }
339 } 342 }
340 mutex_unlock(&ctx.net->nft.commit_mutex); 343 mutex_unlock(&ctx.net->nft.commit_mutex);
341 put_net(ctx.net);
342 344
343 return NOTIFY_DONE; 345 return NOTIFY_DONE;
344} 346}