shinys000114/linux
1
0
Fork 0
mirror of https://github.com/hardkernel/linux.git synced 2026-06-05 02:21:52 +09:00
Code Issues Packages Projects Releases Wiki Activity

netfilter: nf_tables: do not defer rule destruction via call_rcu

Browse Source 
commit b04df3da1b5c6f6dc7cdccc37941740c078c4043 upstream.

nf_tables_chain_destroy can sleep, it can't be used from call_rcu
callbacks.

Moreover, nf_tables_rule_release() is only safe for error unwinding,
while transaction mutex is held and the to-be-desroyed rule was not
exposed to either dataplane or dumps, as it deactives+frees without
the required synchronize_rcu() in-between.

nft_rule_expr_deactivate() callbacks will change ->use counters
of other chains/sets, see e.g. nft_lookup .deactivate callback, these
must be serialized via transaction mutex.

Also add a few lockdep asserts to make this more explicit.

Calling synchronize_rcu() isn't ideal, but fixing this without is hard
and way more intrusive.  As-is, we can get:

WARNING: .. net/netfilter/nf_tables_api.c:5515 nft_set_destroy+0x..
Workqueue: events nf_tables_trans_destroy_work
RIP: 0010:nft_set_destroy+0x3fe/0x5c0
Call Trace:
 <TASK>
 nf_tables_trans_destroy_work+0x6b7/0xad0
 process_one_work+0x64a/0xce0
 worker_thread+0x613/0x10d0

In case the synchronize_rcu becomes an issue, we can explore alternatives.

One way would be to allocate nft_trans_rule objects + one nft_trans_chain
object, deactivate the rules + the chain and then defer the freeing to the
nft destroy workqueue.  We'd still need to keep the synchronize_rcu path as
a fallback to handle -ENOMEM corner cases though.

Reported-by: syzbot+b26935466701e56cfdc2@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/67478d92.050a0220.253251.0062.GAE@google.com/T/
Fixes: c03d278fdf35 ("netfilter: nf_tables: wait for rcu grace period on net_device removal")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Florian Westphal
2025-05-20 01:34:38 +02:00
committed by Greg Kroah-Hartman
parent e6c32a64d6
commit b0f013bebf
2 changed files with 15 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
include/net/netfilter/nf_tables.h
View File

@@ -1062,7 +1062,6 @@ struct nft_chain {
char *name;
u16 udlen;
u8 *udata;
struct rcu_head rcu_head;
/* Only used during control plane commit phase: */
struct nft_rule_blob *blob_next;
@@ -1205,7 +1204,6 @@ static inline void nft_use_inc_restore(u32 *use)
* @sets: sets in the table
* @objects: stateful objects in the table
* @flowtables: flow tables in the table
* @net: netnamespace this table belongs to
* @hgenerator: handle generator state
* @handle: table handle
* @use: number of chain references to this table
@@ -1221,7 +1219,6 @@ struct nft_table {
struct list_head sets;
struct list_head objects;
struct list_head flowtables;
possible_net_t net;
u64 hgenerator;
u64 handle;
u32 use;

32
net/netfilter/nf_tables_api.c
View File

@@ -1413,7 +1413,6 @@ static int nf_tables_newtable(struct sk_buff *skb, const struct nfnl_info *info,
INIT_LIST_HEAD(&table->sets);
INIT_LIST_HEAD(&table->objects);
INIT_LIST_HEAD(&table->flowtables);
write_pnet(&table->net, net);
table->family = family;
table->flags = flags;
table->handle = ++nft_net->table_handle;
@@ -3511,8 +3510,11 @@ void nf_tables_rule_destroy(const struct nft_ctx *ctx, struct nft_rule *rule)
kfree(rule);
}
/* can only be used if rule is no longer visible to dumps */
static void nf_tables_rule_release(const struct nft_ctx *ctx, struct nft_rule *rule)
{
lockdep_commit_lock_is_held(ctx->net);
nft_rule_expr_deactivate(ctx, rule, NFT_TRANS_RELEASE);
nf_tables_rule_destroy(ctx, rule);
}
@@ -5248,6 +5250,8 @@ void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
struct nft_set_binding *binding,
enum nft_trans_phase phase)
{
lockdep_commit_lock_is_held(ctx->net);
switch (phase) {
case NFT_TRANS_PREPARE_ERROR:
nft_set_trans_unbind(ctx, set);
@@ -10674,19 +10678,6 @@ static void __nft_release_basechain_now(struct nft_ctx *ctx)
nf_tables_chain_destroy(ctx->chain);
}
static void nft_release_basechain_rcu(struct rcu_head *head)
{
struct nft_chain *chain = container_of(head, struct nft_chain, rcu_head);
struct nft_ctx ctx = {
.family = chain->table->family,
.chain = chain,
.net = read_pnet(&chain->table->net),
};
__nft_release_basechain_now(&ctx);
put_net(ctx.net);
}
int __nft_release_basechain(struct nft_ctx *ctx)
{
struct nft_rule *rule;
@@ -10701,11 +10692,18 @@ int __nft_release_basechain(struct nft_ctx *ctx)
nft_chain_del(ctx->chain);
nft_use_dec(&ctx->table->use);
if (maybe_get_net(ctx->net))
call_rcu(&ctx->chain->rcu_head, nft_release_basechain_rcu);
else
if (!maybe_get_net(ctx->net)) {
__nft_release_basechain_now(ctx);
return 0;
}
/* wait for ruleset dumps to complete. Owning chain is no longer in
* lists, so new dumps can't find any of these rules anymore.
*/
synchronize_rcu();
__nft_release_basechain_now(ctx);
put_net(ctx->net);
return 0;
}
EXPORT_SYMBOL_GPL(__nft_release_basechain);