The nft_compat module manages match/target lists which are
nft_{match/target}_list.
These lists are used in several functions and these functions can be
executed concurrently because the rule destroying function is called
by workqueue.
So, in order to protect these lists, a lock should be used.
A new nft_xt_lock is added and this protects both lists and the refcnt
of the struct nft_xt.
Test commands:
while :
do
iptables-nft -I FORWARD -m string --string ap --algo kmp &
nft flush ruleset &
done
Result:
[ 488.337545] kernel BUG at lib/list_debug.c:50!
[ 488.346922] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[ 488.347881] CPU: 0 PID: 183 Comm: kworker/0:2 Tainted: G B 4.20.0+ #59
[ 488.347881] Workqueue: events nf_tables_trans_destroy_work [nf_tables]
[ 488.347881] RIP: 0010:__list_del_entry_valid+0xd8/0x150
[ 488.347881] Code: 39 48 83 c4 08 b8 01 00 00 00 5b 5d c3 48 89 ea 48 c7 c7 00 d4 5b 93 e8 0f 55 41 ff 0f 0b 48 c7 c7 60 d4 5b 93 e8 01 55 41 ff <0f> 0b 48 c7 c7 c0 d4 5b 93 e8 f3 54 41 ff 0f 0b 48 c7 c7 20 d5 5b
[ 488.347881] RSP: 0018:ffff8881122073b0 EFLAGS: 00010286
[ 488.409630] RAX: 000000000000004e RBX: dead000000000200 RCX: 0000000000000000
[ 488.409630] RDX: 000000000000004e RSI: 0000000000000008 RDI: ffffed1022440e6c
[ 488.409630] RBP: ffff88810b800ee8 R08: ffffed10235fee91 R09: ffffed10235fee91
[ 488.409630] R10: 00000000dc1f97e1 R11: ffffed10235fee90 R12: ffff88810ef2a890
[ 488.409630] R13: dffffc0000000000 R14: ffff8881120c7a40 R15: 0000000000000002
[ 488.409630] FS: 0000000000000000(0000) GS:ffff88811ae00000(0000) knlGS:0000000000000000
[ 488.462588] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 488.462588] CR2: 00007fa67de0f000 CR3: 0000000108dd6000 CR4: 00000000001006f0
[ 488.473606] Call Trace:
[ 488.473606] nft_xt_put.part.7+0x63/0x200 [nft_compat]
[ 488.481996] ? nft_match_init+0x10/0x10 [nft_compat]
[ 488.481996] ? __kasan_slab_free+0x14a/0x180
[ 488.481996] ? __nft_match_destroy.isra.8+0x1c6/0x360 [nft_compat]
[ 488.502312] __nft_match_destroy.isra.8+0x23d/0x360 [nft_compat]
[ 488.502312] ? nft_target_destroy+0x2e0/0x2e0 [nft_compat]
[ 488.502312] nf_tables_expr_destroy+0x61/0x100 [nf_tables]
[ 488.502312] nf_tables_rule_destroy+0xd8/0x170 [nf_tables]
[ 488.502312] nf_tables_trans_destroy_work+0x5ac/0x840 [nf_tables]
[ 488.502312] ? sched_clock_cpu+0x126/0x170
Fixes: 0935d5588400 ("netfilter: nf_tables: asynchronous release")
Signed-off-by: Taehee Yoo <ap420073@xxxxxxxxx>
---
net/netfilter/nft_compat.c | 28 ++++++++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index 7334e0b80a5e..83adabdec900 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -23,6 +23,8 @@
#include <linux/netfilter_arp/arp_tables.h>
#include <net/netfilter/nf_tables.h>
+static DEFINE_MUTEX(nft_xt_lock);
+
struct nft_xt {
struct list_head head;
struct nft_expr_ops ops;
@@ -45,12 +47,15 @@ struct nft_xt_match_priv {
static bool nft_xt_put(struct nft_xt *xt)
{
+ mutex_lock(&nft_xt_lock);
if (--xt->refcnt == 0) {
list_del(&xt->head);
kfree_rcu(xt, rcu_head);
+ mutex_unlock(&nft_xt_lock);
return true;
}
+ mutex_unlock(&nft_xt_lock);
return false;
}
@@ -272,8 +277,10 @@ nft_target_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
if (!target->target)
return -EINVAL;
+ mutex_lock(&nft_xt_lock);
nft_xt = container_of(expr->ops, struct nft_xt, ops);
nft_xt->refcnt++;
+ mutex_unlock(&nft_xt_lock);
return 0;
}
@@ -485,8 +492,10 @@ __nft_match_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
if (ret < 0)
return ret;
+ mutex_lock(&nft_xt_lock);
nft_xt = container_of(expr->ops, struct nft_xt, ops);
nft_xt->refcnt++;
+ mutex_unlock(&nft_xt_lock);
return 0;
}
@@ -766,12 +775,16 @@ nft_match_select_ops(const struct nft_ctx *ctx,
family = ctx->family;
/* Re-use the existing match if it's already loaded. */
+ mutex_lock(&nft_xt_lock);
list_for_each_entry(nft_match, &nft_match_list, head) {
struct xt_match *match = nft_match->ops.data;
- if (nft_match_cmp(match, mt_name, rev, family))
+ if (nft_match_cmp(match, mt_name, rev, family)) {
+ mutex_unlock(&nft_xt_lock);
return &nft_match->ops;
+ }
}
+ mutex_unlock(&nft_xt_lock);
match = xt_request_find_match(family, mt_name, rev);
if (IS_ERR(match))
@@ -810,7 +823,9 @@ nft_match_select_ops(const struct nft_ctx *ctx,
nft_match->ops.size = matchsize;
+ mutex_lock(&nft_xt_lock);
list_add(&nft_match->head, &nft_match_list);
+ mutex_unlock(&nft_xt_lock);
return &nft_match->ops;
err:
@@ -862,15 +877,19 @@ nft_target_select_ops(const struct nft_ctx *ctx,
return ERR_PTR(-EINVAL);
/* Re-use the existing target if it's already loaded. */
+ mutex_lock(&nft_xt_lock);
list_for_each_entry(nft_target, &nft_target_list, head) {
struct xt_target *target = nft_target->ops.data;
if (!target->target)
continue;
- if (nft_target_cmp(target, tg_name, rev, family))
+ if (nft_target_cmp(target, tg_name, rev, family)) {
+ mutex_unlock(&nft_xt_lock);
return &nft_target->ops;
+ }
}
+ mutex_unlock(&nft_xt_lock);
target = xt_request_find_target(family, tg_name, rev);
if (IS_ERR(target))
@@ -907,7 +926,9 @@ nft_target_select_ops(const struct nft_ctx *ctx,
else
nft_target->ops.eval = nft_target_eval_xt;
+ mutex_lock(&nft_xt_lock);
list_add(&nft_target->head, &nft_target_list);
+ mutex_unlock(&nft_xt_lock);
return &nft_target->ops;
err:
@@ -961,6 +982,7 @@ static void __exit nft_compat_module_exit(void)
* In this case, the lists contain 0-refcount entries that still
* hold module reference.
*/
+ mutex_lock(&nft_xt_lock);
list_for_each_entry_safe(xt, next, &nft_target_list, head) {
struct xt_target *target = xt->ops.data;
@@ -975,9 +997,11 @@ static void __exit nft_compat_module_exit(void)
if (WARN_ON_ONCE(xt->refcnt))
continue;
+
module_put(match->me);
kfree(xt);
}
+ mutex_unlock(&nft_xt_lock);
nfnetlink_subsys_unregister(&nfnl_compat_subsys);
nft_unregister_expr(&nft_target_type);
nft_unregister_expr(&nft_match_type);
--
2.17.1
