Skip to content

Commit e427033

Browse files
committed
netfilter: ctnetlink: fix refcount leak on table dump
JIRA: https://issues.redhat.com/browse/RHEL-115630 Upstream Status: commit de788b2 commit de788b2 Author: Florian Westphal <fw@strlen.de> Date: Fri Aug 1 17:25:08 2025 +0200 netfilter: ctnetlink: fix refcount leak on table dump There is a reference count leak in ctnetlink_dump_table(): if (res < 0) { nf_conntrack_get(&ct->ct_general); // HERE cb->args[1] = (unsigned long)ct; ... While its very unlikely, its possible that ct == last. If this happens, then the refcount of ct was already incremented. This 2nd increment is never undone. This prevents the conntrack object from being released, which in turn keeps prevents cnet->count from dropping back to 0. This will then block the netns dismantle (or conntrack rmmod) as nf_conntrack_cleanup_net_list() will wait forever. This can be reproduced by running conntrack_resize.sh selftest in a loop. It takes ~20 minutes for me on a preemptible kernel on average before I see a runaway kworker spinning in nf_conntrack_cleanup_net_list. One fix would to change this to: if (res < 0) { if (ct != last) nf_conntrack_get(&ct->ct_general); But this reference counting isn't needed in the first place. We can just store a cookie value instead. A followup patch will do the same for ctnetlink_exp_dump_table, it looks to me as if this has the same problem and like ctnetlink_dump_table, we only need a 'skip hint', not the actual object so we can apply the same cookie strategy there as well. Fixes: d205dc4 ("[NETFILTER]: ctnetlink: fix deadlock in table dumping") Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Florian Westphal <fwestpha@redhat.com>
1 parent b5177ff commit e427033

File tree

1 file changed

+13
-11
lines changed

1 file changed

+13
-11
lines changed

net/netfilter/nf_conntrack_netlink.c

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -862,8 +862,6 @@ ctnetlink_conntrack_event(unsigned int events, const struct nf_ct_event *item)
862862

863863
static int ctnetlink_done(struct netlink_callback *cb)
864864
{
865-
if (cb->args[1])
866-
nf_ct_put((struct nf_conn *)cb->args[1]);
867865
kfree(cb->data);
868866
return 0;
869867
}
@@ -1178,19 +1176,26 @@ static int ctnetlink_filter_match(struct nf_conn *ct, void *data)
11781176
return 0;
11791177
}
11801178

1179+
static unsigned long ctnetlink_get_id(const struct nf_conn *ct)
1180+
{
1181+
unsigned long id = nf_ct_get_id(ct);
1182+
1183+
return id ? id : 1;
1184+
}
1185+
11811186
static int
11821187
ctnetlink_dump_table(struct sk_buff *skb, struct netlink_callback *cb)
11831188
{
11841189
unsigned int flags = cb->data ? NLM_F_DUMP_FILTERED : 0;
11851190
struct net *net = sock_net(skb->sk);
1186-
struct nf_conn *ct, *last;
1191+
unsigned long last_id = cb->args[1];
11871192
struct nf_conntrack_tuple_hash *h;
11881193
struct hlist_nulls_node *n;
11891194
struct nf_conn *nf_ct_evict[8];
1195+
struct nf_conn *ct;
11901196
int res, i;
11911197
spinlock_t *lockp;
11921198

1193-
last = (struct nf_conn *)cb->args[1];
11941199
i = 0;
11951200

11961201
local_bh_disable();
@@ -1227,7 +1232,7 @@ ctnetlink_dump_table(struct sk_buff *skb, struct netlink_callback *cb)
12271232
continue;
12281233

12291234
if (cb->args[1]) {
1230-
if (ct != last)
1235+
if (ctnetlink_get_id(ct) != last_id)
12311236
continue;
12321237
cb->args[1] = 0;
12331238
}
@@ -1240,8 +1245,7 @@ ctnetlink_dump_table(struct sk_buff *skb, struct netlink_callback *cb)
12401245
NFNL_MSG_TYPE(cb->nlh->nlmsg_type),
12411246
ct, true, flags);
12421247
if (res < 0) {
1243-
nf_conntrack_get(&ct->ct_general);
1244-
cb->args[1] = (unsigned long)ct;
1248+
cb->args[1] = ctnetlink_get_id(ct);
12451249
spin_unlock(lockp);
12461250
goto out;
12471251
}
@@ -1254,12 +1258,10 @@ ctnetlink_dump_table(struct sk_buff *skb, struct netlink_callback *cb)
12541258
}
12551259
out:
12561260
local_bh_enable();
1257-
if (last) {
1261+
if (last_id) {
12581262
/* nf ct hash resize happened, now clear the leftover. */
1259-
if ((struct nf_conn *)cb->args[1] == last)
1263+
if (cb->args[1] == last_id)
12601264
cb->args[1] = 0;
1261-
1262-
nf_ct_put(last);
12631265
}
12641266

12651267
while (i) {

0 commit comments

Comments
 (0)