Commit f9a43007 authored by Pablo Neira Ayuso's avatar Pablo Neira Ayuso

netfilter: nf_tables: double hook unregistration in netns path

__nft_release_hooks() is called from pre_netns exit path which
unregisters the hooks, then the NETDEV_UNREGISTER event is triggered
which unregisters the hooks again.

[  565.221461] WARNING: CPU: 18 PID: 193 at net/netfilter/core.c:495 __nf_unregister_net_hook+0x247/0x270
[...]
[  565.246890] CPU: 18 PID: 193 Comm: kworker/u64:1 Tainted: G            E     5.18.0-rc7+ #27
[  565.253682] Workqueue: netns cleanup_net
[  565.257059] RIP: 0010:__nf_unregister_net_hook+0x247/0x270
[...]
[  565.297120] Call Trace:
[  565.300900]  <TASK>
[  565.304683]  nf_tables_flowtable_event+0x16a/0x220 [nf_tables]
[  565.308518]  raw_notifier_call_chain+0x63/0x80
[  565.312386]  unregister_netdevice_many+0x54f/0xb50

Unregister and destroy netdev hook from netns pre_exit via kfree_rcu
so the NETDEV_UNREGISTER path see unregistered hooks.

Fixes: 767d1216 ("netfilter: nftables: fix possible UAF over chains from packet path in netns")
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent 3923b1e4
...@@ -222,12 +222,18 @@ static int nft_netdev_register_hooks(struct net *net, ...@@ -222,12 +222,18 @@ static int nft_netdev_register_hooks(struct net *net,
} }
static void nft_netdev_unregister_hooks(struct net *net, static void nft_netdev_unregister_hooks(struct net *net,
struct list_head *hook_list) struct list_head *hook_list,
bool release_netdev)
{ {
struct nft_hook *hook; struct nft_hook *hook, *next;
list_for_each_entry(hook, hook_list, list) list_for_each_entry_safe(hook, next, hook_list, list) {
nf_unregister_net_hook(net, &hook->ops); nf_unregister_net_hook(net, &hook->ops);
if (release_netdev) {
list_del(&hook->list);
kfree_rcu(hook, rcu);
}
}
} }
static int nf_tables_register_hook(struct net *net, static int nf_tables_register_hook(struct net *net,
...@@ -253,9 +259,10 @@ static int nf_tables_register_hook(struct net *net, ...@@ -253,9 +259,10 @@ static int nf_tables_register_hook(struct net *net,
return nf_register_net_hook(net, &basechain->ops); return nf_register_net_hook(net, &basechain->ops);
} }
static void nf_tables_unregister_hook(struct net *net, static void __nf_tables_unregister_hook(struct net *net,
const struct nft_table *table, const struct nft_table *table,
struct nft_chain *chain) struct nft_chain *chain,
bool release_netdev)
{ {
struct nft_base_chain *basechain; struct nft_base_chain *basechain;
const struct nf_hook_ops *ops; const struct nf_hook_ops *ops;
...@@ -270,11 +277,19 @@ static void nf_tables_unregister_hook(struct net *net, ...@@ -270,11 +277,19 @@ static void nf_tables_unregister_hook(struct net *net,
return basechain->type->ops_unregister(net, ops); return basechain->type->ops_unregister(net, ops);
if (nft_base_chain_netdev(table->family, basechain->ops.hooknum)) if (nft_base_chain_netdev(table->family, basechain->ops.hooknum))
nft_netdev_unregister_hooks(net, &basechain->hook_list); nft_netdev_unregister_hooks(net, &basechain->hook_list,
release_netdev);
else else
nf_unregister_net_hook(net, &basechain->ops); nf_unregister_net_hook(net, &basechain->ops);
} }
static void nf_tables_unregister_hook(struct net *net,
const struct nft_table *table,
struct nft_chain *chain)
{
return __nf_tables_unregister_hook(net, table, chain, false);
}
static void nft_trans_commit_list_add_tail(struct net *net, struct nft_trans *trans) static void nft_trans_commit_list_add_tail(struct net *net, struct nft_trans *trans)
{ {
struct nftables_pernet *nft_net = nft_pernet(net); struct nftables_pernet *nft_net = nft_pernet(net);
...@@ -7307,13 +7322,25 @@ static void nft_unregister_flowtable_hook(struct net *net, ...@@ -7307,13 +7322,25 @@ static void nft_unregister_flowtable_hook(struct net *net,
FLOW_BLOCK_UNBIND); FLOW_BLOCK_UNBIND);
} }
static void nft_unregister_flowtable_net_hooks(struct net *net, static void __nft_unregister_flowtable_net_hooks(struct net *net,
struct list_head *hook_list) struct list_head *hook_list,
bool release_netdev)
{ {
struct nft_hook *hook; struct nft_hook *hook, *next;
list_for_each_entry(hook, hook_list, list) list_for_each_entry_safe(hook, next, hook_list, list) {
nf_unregister_net_hook(net, &hook->ops); nf_unregister_net_hook(net, &hook->ops);
if (release_netdev) {
list_del(&hook->list);
kfree_rcu(hook);
}
}
}
static void nft_unregister_flowtable_net_hooks(struct net *net,
struct list_head *hook_list)
{
__nft_unregister_flowtable_net_hooks(net, hook_list, false);
} }
static int nft_register_flowtable_net_hooks(struct net *net, static int nft_register_flowtable_net_hooks(struct net *net,
...@@ -9755,9 +9782,10 @@ static void __nft_release_hook(struct net *net, struct nft_table *table) ...@@ -9755,9 +9782,10 @@ static void __nft_release_hook(struct net *net, struct nft_table *table)
struct nft_chain *chain; struct nft_chain *chain;
list_for_each_entry(chain, &table->chains, list) list_for_each_entry(chain, &table->chains, list)
nf_tables_unregister_hook(net, table, chain); __nf_tables_unregister_hook(net, table, chain, true);
list_for_each_entry(flowtable, &table->flowtables, list) list_for_each_entry(flowtable, &table->flowtables, list)
nft_unregister_flowtable_net_hooks(net, &flowtable->hook_list); __nft_unregister_flowtable_net_hooks(net, &flowtable->hook_list,
true);
} }
static void __nft_release_hooks(struct net *net) static void __nft_release_hooks(struct net *net)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment