Skip to content

Commit e6c32a6

Browse files
ummakynesgregkh
authored andcommitted
netfilter: nf_tables: wait for rcu grace period on net_device removal
commit c03d278fdf35e73dd0ec543b9b556876b9d9a8dc upstream. 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]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent bbd6819 commit e6c32a6

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
@@ -1045,6 +1045,7 @@ struct nft_rule_blob {
10451045
* @use: number of jump references to this chain
10461046
* @flags: bitmask of enum nft_chain_flags
10471047
* @name: name of the chain
1048+
* @rcu_head: rcu head for deferred release
10481049
*/
10491050
struct nft_chain {
10501051
struct nft_rule_blob __rcu *blob_gen_0;
@@ -1061,6 +1062,7 @@ struct nft_chain {
10611062
char *name;
10621063
u16 udlen;
10631064
u8 *udata;
1065+
struct rcu_head rcu_head;
10641066

10651067
/* Only used during control plane commit phase: */
10661068
struct nft_rule_blob *blob_next;
@@ -1203,6 +1205,7 @@ static inline void nft_use_inc_restore(u32 *use)
12031205
* @sets: sets in the table
12041206
* @objects: stateful objects in the table
12051207
* @flowtables: flow tables in the table
1208+
* @net: netnamespace this table belongs to
12061209
* @hgenerator: handle generator state
12071210
* @handle: table handle
12081211
* @use: number of chain references to this table
@@ -1218,6 +1221,7 @@ struct nft_table {
12181221
struct list_head sets;
12191222
struct list_head objects;
12201223
struct list_head flowtables;
1224+
possible_net_t net;
12211225
u64 hgenerator;
12221226
u64 handle;
12231227
u32 use;

net/netfilter/nf_tables_api.c

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1413,6 +1413,7 @@ static int nf_tables_newtable(struct sk_buff *skb, const struct nfnl_info *info,
14131413
INIT_LIST_HEAD(&table->sets);
14141414
INIT_LIST_HEAD(&table->objects);
14151415
INIT_LIST_HEAD(&table->flowtables);
1416+
write_pnet(&table->net, net);
14161417
table->family = family;
14171418
table->flags = flags;
14181419
table->handle = ++nft_net->table_handle;
@@ -10662,22 +10663,48 @@ int nft_data_dump(struct sk_buff *skb, int attr, const struct nft_data *data,
1066210663
}
1066310664
EXPORT_SYMBOL_GPL(nft_data_dump);
1066410665

10665-
int __nft_release_basechain(struct nft_ctx *ctx)
10666+
static void __nft_release_basechain_now(struct nft_ctx *ctx)
1066610667
{
1066710668
struct nft_rule *rule, *nr;
1066810669

10669-
if (WARN_ON(!nft_is_base_chain(ctx->chain)))
10670-
return 0;
10671-
10672-
nf_tables_unregister_hook(ctx->net, ctx->chain->table, ctx->chain);
1067310670
list_for_each_entry_safe(rule, nr, &ctx->chain->rules, list) {
1067410671
list_del(&rule->list);
10675-
nft_use_dec(&ctx->chain->use);
1067610672
nf_tables_rule_release(ctx, rule);
1067710673
}
10674+
nf_tables_chain_destroy(ctx->chain);
10675+
}
10676+
10677+
static void nft_release_basechain_rcu(struct rcu_head *head)
10678+
{
10679+
struct nft_chain *chain = container_of(head, struct nft_chain, rcu_head);
10680+
struct nft_ctx ctx = {
10681+
.family = chain->table->family,
10682+
.chain = chain,
10683+
.net = read_pnet(&chain->table->net),
10684+
};
10685+
10686+
__nft_release_basechain_now(&ctx);
10687+
put_net(ctx.net);
10688+
}
10689+
10690+
int __nft_release_basechain(struct nft_ctx *ctx)
10691+
{
10692+
struct nft_rule *rule;
10693+
10694+
if (WARN_ON_ONCE(!nft_is_base_chain(ctx->chain)))
10695+
return 0;
10696+
10697+
nf_tables_unregister_hook(ctx->net, ctx->chain->table, ctx->chain);
10698+
list_for_each_entry(rule, &ctx->chain->rules, list)
10699+
nft_use_dec(&ctx->chain->use);
10700+
1067810701
nft_chain_del(ctx->chain);
1067910702
nft_use_dec(&ctx->table->use);
10680-
nf_tables_chain_destroy(ctx->chain);
10703+
10704+
if (maybe_get_net(ctx->net))
10705+
call_rcu(&ctx->chain->rcu_head, nft_release_basechain_rcu);
10706+
else
10707+
__nft_release_basechain_now(ctx);
1068110708

1068210709
return 0;
1068310710
}

0 commit comments

Comments
 (0)