nft_{match/target}_select_ops() find xt modules in the list. if the
module is found, it is just used without any holding module or
increasing reference counts. and the reference count is increased in
nft_{match/target}_init() function.
But, xt modules can be removed anytime if reference count is 0.
CPU0 CPU1
nft_{match/target}_select_ops
nft_{match/target}_destroy
nft_{match/target}_init
The scenario is like this:
1. ->select_ops() found a module in the list and keeps an element of it
without increasing reference count.
2. ->destroy() frees an element of list.
3. nft_{match/target}_init try to use the pointer of element, but this is
already freed.
Test commands:
while :
do
iptables-nft -t nat -I PREROUTING -m string --string ap --algo \
kmp -j MASQUERADE &
nft flush ruleset &
done
Result:
[ 682.340431] BUG: KASAN: use-after-free in nft_target_init+0xb96/0xbe0 [nft_compat]
[ 682.340431] Read of size 4 at addr ffff8881079a73a0 by task iptables-nft/8136
[ 682.340431]
[ 682.340431] CPU: 1 PID: 8136 Comm: iptables-nft Not tainted 4.20.0+ #59
[ 682.373297] Call Trace:
[ 682.373297] dump_stack+0xc9/0x16b
[ 682.373297] ? show_regs_print_info+0x5/0x5
[ 682.373297] ? kmsg_dump_rewind_nolock+0xd9/0xd9
[ 682.402587] ? nft_target_init+0xb96/0xbe0 [nft_compat]
[ 682.402587] print_address_description+0x6a/0x270
[ 682.402587] ? nft_target_init+0xb96/0xbe0 [nft_compat]
[ 682.402587] ? nft_target_init+0xb96/0xbe0 [nft_compat]
[ 682.402587] kasan_report+0x12a/0x16f
[ 682.429087] ? nft_target_init+0xb96/0xbe0 [nft_compat]
[ 682.433653] nft_target_init+0xb96/0xbe0 [nft_compat]
[ 682.433653] ? target_compat_from_user.isra.5+0x80/0x80 [nft_compat]
[ 682.433653] ? nf_tables_newrule+0xe46/0x2570 [nf_tables]
[ 682.433653] ? ksm_do_scan+0x2bc0/0x3300
[ 682.433653] ? hlock_class+0x140/0x140
[ 682.433653] ? nf_tables_newrule+0xe46/0x2570 [nf_tables]
[ 682.465582] ? memcpy+0x39/0x50
[ 682.465582] ? __kmalloc+0x134/0x2e0
[ 682.465582] ? nf_tables_newrule+0xe46/0x2570 [nf_tables]
[ 682.465582] nf_tables_newrule+0x10a4/0x2570 [nf_tables]
[ ... ]
[ 682.828711]
[ 682.828711] Allocated by task 8134:
[ 682.828711] kasan_kmalloc+0xa5/0xd0
[ 682.828711] kmem_cache_alloc_trace+0x117/0x290
[ 682.828711] nft_target_select_ops+0x481/0x8e0 [nft_compat]
[ 682.828711] nf_tables_expr_parse+0x2a1/0x510 [nf_tables]
[ 682.828711] nf_tables_newrule+0xd0c/0x2570 [nf_tables]
[ 682.828711] nfnetlink_rcv_batch+0xd2e/0x1550 [nfnetlink]
[ 682.828711] nfnetlink_rcv+0x2ee/0x350 [nfnetlink]
[ 682.828711] netlink_unicast+0x414/0x5e0
[ 682.828711] netlink_sendmsg+0x7b8/0xcf0
[ 682.937632] ___sys_sendmsg+0x689/0x990
[ 682.937632] __sys_sendmsg+0xd2/0x210
[ 682.937632] do_syscall_64+0x138/0x560
[ 682.937632] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 682.937632]
[ 682.937632] Freed by task 8140:
[ 682.937632] __kasan_slab_free+0x135/0x180
[ 682.937632] kfree+0xdb/0x280
[ 682.937632] rcu_process_callbacks+0x947/0x24d0
[ 682.937632] __do_softirq+0x2a5/0xa3b
[ 682.937632]
Fixes: 0935d5588400 ("netfilter: nf_tables: asynchronous release")
Signed-off-by: Taehee Yoo <ap420073@xxxxxxxxx>
---
net/netfilter/nft_compat.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index 83adabdec900..8f3114871d20 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -29,6 +29,7 @@ struct nft_xt {
struct list_head head;
struct nft_expr_ops ops;
unsigned int refcnt;
+ bool use;
/* Unlike other expressions, ops doesn't have static storage duration.
* nft core assumes they do. We use kfree_rcu so that nft core can
@@ -48,7 +49,7 @@ struct nft_xt_match_priv {
static bool nft_xt_put(struct nft_xt *xt)
{
mutex_lock(&nft_xt_lock);
- if (--xt->refcnt == 0) {
+ if (--xt->refcnt == 0 && !xt->use) {
list_del(&xt->head);
kfree_rcu(xt, rcu_head);
mutex_unlock(&nft_xt_lock);
@@ -280,6 +281,7 @@ nft_target_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
mutex_lock(&nft_xt_lock);
nft_xt = container_of(expr->ops, struct nft_xt, ops);
nft_xt->refcnt++;
+ nft_xt->use = false;
mutex_unlock(&nft_xt_lock);
return 0;
}
@@ -495,6 +497,7 @@ __nft_match_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
mutex_lock(&nft_xt_lock);
nft_xt = container_of(expr->ops, struct nft_xt, ops);
nft_xt->refcnt++;
+ nft_xt->use = false;
mutex_unlock(&nft_xt_lock);
return 0;
}
@@ -780,6 +783,7 @@ nft_match_select_ops(const struct nft_ctx *ctx,
struct xt_match *match = nft_match->ops.data;
if (nft_match_cmp(match, mt_name, rev, family)) {
+ nft_match->use = true;
mutex_unlock(&nft_xt_lock);
return &nft_match->ops;
}
@@ -803,6 +807,7 @@ nft_match_select_ops(const struct nft_ctx *ctx,
}
nft_match->refcnt = 0;
+ nft_match->use = true;
nft_match->ops.type = &nft_match_type;
nft_match->ops.eval = nft_match_eval;
nft_match->ops.init = nft_match_init;
@@ -885,6 +890,7 @@ nft_target_select_ops(const struct nft_ctx *ctx,
continue;
if (nft_target_cmp(target, tg_name, rev, family)) {
+ nft_target->use = true;
mutex_unlock(&nft_xt_lock);
return &nft_target->ops;
}
@@ -913,6 +919,7 @@ nft_target_select_ops(const struct nft_ctx *ctx,
}
nft_target->refcnt = 0;
+ nft_target->use = true;
nft_target->ops.type = &nft_target_type;
nft_target->ops.size = NFT_EXPR_SIZE(XT_ALIGN(target->targetsize));
nft_target->ops.init = nft_target_init;
--
2.17.1
