Skip to content

Commit c03d278

Browse files
committed
netfilter: nf_tables: wait for rcu grace period on net_device removal
8c873e2 ("netfilter: core: free hooks with call_rcu") removed synchronize_net() call when unregistering basechain hook, however, net_device removal event handler for the NFPROTO_NETDEV was not updated to wait for RCU grace period. Note that 835b803 ("netfilter: nf_tables_netdev: unregister hooks on net_device removal") does not remove basechain rules on device removal, I was hinted to remove rules on net_device removal later, see 5ebe0b0 ("netfilter: nf_tables: destroy basechain and rules on netdevice removal"). Although NETDEV_UNREGISTER event is guaranteed to be handled after synchronize_net() call, this path needs to wait for rcu grace period via rcu callback to release basechain hooks if netns is alive because an ongoing netlink dump could be in progress (sockets hold a reference on the netns). Note that nf_tables_pre_exit_net() unregisters and releases basechain hooks but it is possible to see NETDEV_UNREGISTER at a later stage in the netns exit path, eg. veth peer device in another netns: cleanup_net() default_device_exit_batch() unregister_netdevice_many_notify() notifier_call_chain() nf_tables_netdev_event() __nft_release_basechain() In this particular case, same rule of thumb applies: if netns is alive, then wait for rcu grace period because netlink dump in the other netns could be in progress. Otherwise, if the other netns is going away then no netlink dump can be in progress and basechain hooks can be released inmediately. While at it, turn WARN_ON() into WARN_ON_ONCE() for the basechain validation, which should not ever happen. Fixes: 835b803 ("netfilter: nf_tables_netdev: unregister hooks on net_device removal") Signed-off-by: Pablo Neira Ayuso <[email protected]>
1 parent 50ae879 commit c03d278

File tree

2 files changed

+38
-7
lines changed

2 files changed

+38
-7
lines changed

include/net/netfilter/nf_tables.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,6 +1103,7 @@ struct nft_rule_blob {
11031103
* @name: name of the chain
11041104
* @udlen: user data length
11051105
* @udata: user data in the chain
1106+
* @rcu_head: rcu head for deferred release
11061107
* @blob_next: rule blob pointer to the next in the chain
11071108
*/
11081109
struct nft_chain {
@@ -1120,6 +1121,7 @@ struct nft_chain {
11201121
char *name;
11211122
u16 udlen;
11221123
u8 *udata;
1124+
struct rcu_head rcu_head;
11231125

11241126
/* Only used during control plane commit phase: */
11251127
struct nft_rule_blob *blob_next;
@@ -1263,6 +1265,7 @@ static inline void nft_use_inc_restore(u32 *use)
12631265
* @sets: sets in the table
12641266
* @objects: stateful objects in the table
12651267
* @flowtables: flow tables in the table
1268+
* @net: netnamespace this table belongs to
12661269
* @hgenerator: handle generator state
12671270
* @handle: table handle
12681271
* @use: number of chain references to this table
@@ -1282,6 +1285,7 @@ struct nft_table {
12821285
struct list_head sets;
12831286
struct list_head objects;
12841287
struct list_head flowtables;
1288+
possible_net_t net;
12851289
u64 hgenerator;
12861290
u64 handle;
12871291
u32 use;

net/netfilter/nf_tables_api.c

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1495,6 +1495,7 @@ static int nf_tables_newtable(struct sk_buff *skb, const struct nfnl_info *info,
14951495
INIT_LIST_HEAD(&table->sets);
14961496
INIT_LIST_HEAD(&table->objects);
14971497
INIT_LIST_HEAD(&table->flowtables);
1498+
write_pnet(&table->net, net);
14981499
table->family = family;
14991500
table->flags = flags;
15001501
table->handle = ++nft_net->table_handle;
@@ -11430,22 +11431,48 @@ int nft_data_dump(struct sk_buff *skb, int attr, const struct nft_data *data,
1143011431
}
1143111432
EXPORT_SYMBOL_GPL(nft_data_dump);
1143211433

11433-
int __nft_release_basechain(struct nft_ctx *ctx)
11434+
static void __nft_release_basechain_now(struct nft_ctx *ctx)
1143411435
{
1143511436
struct nft_rule *rule, *nr;
1143611437

11437-
if (WARN_ON(!nft_is_base_chain(ctx->chain)))
11438-
return 0;
11439-
11440-
nf_tables_unregister_hook(ctx->net, ctx->chain->table, ctx->chain);
1144111438
list_for_each_entry_safe(rule, nr, &ctx->chain->rules, list) {
1144211439
list_del(&rule->list);
11443-
nft_use_dec(&ctx->chain->use);
1144411440
nf_tables_rule_release(ctx, rule);
1144511441
}
11442+
nf_tables_chain_destroy(ctx->chain);
11443+
}
11444+
11445+
static void nft_release_basechain_rcu(struct rcu_head *head)
11446+
{
11447+
struct nft_chain *chain = container_of(head, struct nft_chain, rcu_head);
11448+
struct nft_ctx ctx = {
11449+
.family = chain->table->family,
11450+
.chain = chain,
11451+
.net = read_pnet(&chain->table->net),
11452+
};
11453+
11454+
__nft_release_basechain_now(&ctx);
11455+
put_net(ctx.net);
11456+
}
11457+
11458+
int __nft_release_basechain(struct nft_ctx *ctx)
11459+
{
11460+
struct nft_rule *rule;
11461+
11462+
if (WARN_ON_ONCE(!nft_is_base_chain(ctx->chain)))
11463+
return 0;
11464+
11465+
nf_tables_unregister_hook(ctx->net, ctx->chain->table, ctx->chain);
11466+
list_for_each_entry(rule, &ctx->chain->rules, list)
11467+
nft_use_dec(&ctx->chain->use);
11468+
1144611469
nft_chain_del(ctx->chain);
1144711470
nft_use_dec(&ctx->table->use);
11448-
nf_tables_chain_destroy(ctx->chain);
11471+
11472+
if (maybe_get_net(ctx->net))
11473+
call_rcu(&ctx->chain->rcu_head, nft_release_basechain_rcu);
11474+
else
11475+
__nft_release_basechain_now(ctx);
1144911476

1145011477
return 0;
1145111478
}

0 commit comments

Comments
 (0)