diff options
| author | Florian Westphal <fw@strlen.de> | 2018-08-02 15:44:41 -0400 |
|---|---|---|
| committer | Pablo Neira Ayuso <pablo@netfilter.org> | 2018-08-16 13:37:03 -0400 |
| commit | 6a48de0144767f2c6880540c0a4ac6741e3c440b (patch) | |
| tree | f3111dac7df6aa291d8d8b60dcb50f5a508c9d70 | |
| parent | d209df3e7f7002d9099fdb0f6df0f972b4386a63 (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.c | 7 | ||||
| -rw-r--r-- | net/netfilter/nft_chain_filter.c | 12 |
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 | } |
