Skip to content

Commit 13ee6ac

Browse files
Stephen Hemmingerummakynes
authored andcommitted
netfilter: fix race in conntrack between dump_table and destroy
The netlink interface to dump the connection tracking table has a race when entries are deleted at the same time. A customer reported a crash and the backtrace showed thatctnetlink_dump_table was running while a conntrack entry was being destroyed. (see https://bugzilla.vyatta.com/show_bug.cgi?id=6402). According to RCU documentation, when using hlist_nulls the reader must handle the case of seeing a deleted entry and not proceed further down the linked list. The old code would continue which caused the scan to walk into the free list. This patch uses locking (rather than RCU) for this operation which is guaranteed safe, and no longer requires getting reference while doing dump operation. Signed-off-by: Stephen Hemminger <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]>
1 parent 83723d6 commit 13ee6ac

File tree

1 file changed

+5
-9
lines changed

1 file changed

+5
-9
lines changed

net/netfilter/nf_conntrack_netlink.c

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -645,25 +645,23 @@ ctnetlink_dump_table(struct sk_buff *skb, struct netlink_callback *cb)
645645
struct nfgenmsg *nfmsg = nlmsg_data(cb->nlh);
646646
u_int8_t l3proto = nfmsg->nfgen_family;
647647

648-
rcu_read_lock();
648+
spin_lock_bh(&nf_conntrack_lock);
649649
last = (struct nf_conn *)cb->args[1];
650650
for (; cb->args[0] < net->ct.htable_size; cb->args[0]++) {
651651
restart:
652-
hlist_nulls_for_each_entry_rcu(h, n, &net->ct.hash[cb->args[0]],
652+
hlist_nulls_for_each_entry(h, n, &net->ct.hash[cb->args[0]],
653653
hnnode) {
654654
if (NF_CT_DIRECTION(h) != IP_CT_DIR_ORIGINAL)
655655
continue;
656656
ct = nf_ct_tuplehash_to_ctrack(h);
657-
if (!atomic_inc_not_zero(&ct->ct_general.use))
658-
continue;
659657
/* Dump entries of a given L3 protocol number.
660658
* If it is not specified, ie. l3proto == 0,
661659
* then dump everything. */
662660
if (l3proto && nf_ct_l3num(ct) != l3proto)
663-
goto releasect;
661+
continue;
664662
if (cb->args[1]) {
665663
if (ct != last)
666-
goto releasect;
664+
continue;
667665
cb->args[1] = 0;
668666
}
669667
if (ctnetlink_fill_info(skb, NETLINK_CB(cb->skb).pid,
@@ -681,16 +679,14 @@ ctnetlink_dump_table(struct sk_buff *skb, struct netlink_callback *cb)
681679
if (acct)
682680
memset(acct, 0, sizeof(struct nf_conn_counter[IP_CT_DIR_MAX]));
683681
}
684-
releasect:
685-
nf_ct_put(ct);
686682
}
687683
if (cb->args[1]) {
688684
cb->args[1] = 0;
689685
goto restart;
690686
}
691687
}
692688
out:
693-
rcu_read_unlock();
689+
spin_unlock_bh(&nf_conntrack_lock);
694690
if (last)
695691
nf_ct_put(last);
696692

0 commit comments

Comments
 (0)