>From c454ca5a96f5b6f815fe29cc2c91c92d719d7b95 Mon Sep 17 00:00:00 2001 From: bill bonaparte Date: Mon, 3 Nov 2014 17:13:51 +0800 Subject: [PATCH] netfilter: nf_conntrack: fix a race in __nf_conntrack_confirm against nf_ct_get_next_corpse After we remove central spinlock nf_conntrack_lock, we get the race against nf_ct_get_next_corpse again, to get rid of this race, we should remove the conntrack from the unconfirmed-list (which is per-cpu list) firstly, then check if it is dying. --- net/netfilter/nf_conntrack_core.c | 28 ++++++++++++++++------------ 1 files changed, 16 insertions(+), 12 deletions(-) diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c index 5016a69..5d54a18 100644 --- a/net/netfilter/nf_conntrack_core.c +++ b/net/netfilter/nf_conntrack_core.c @@ -589,6 +589,22 @@ __nf_conntrack_confirm(struct sk_buff *skb) zone = nf_ct_zone(ct); local_bh_disable(); + + /* We have to check the DYING flag after unlink the conntrack + to prevent a race against nf_ct_get_next_corpse() possibly + called from user context, else we insert an already 'dead' hash, + blocking further use of that particular connection -JM */ + nf_ct_del_from_dying_or_unconfirmed_list(ct); + + if (unlikely(nf_ct_is_dying(ct))) { + /* let's follow the rules that the conntrack must + alternatively at the unconfirmed-list or dying-list + when it is abort to be destoryed + */ + nf_ct_add_to_dying_list(ct); + local_bh_enable(); + return NF_ACCEPT; + } do { sequence = read_seqcount_begin(&net->ct.generation); @@ -611,16 +627,6 @@ __nf_conntrack_confirm(struct sk_buff *skb) */ NF_CT_ASSERT(!nf_ct_is_confirmed(ct)); pr_debug("Confirming conntrack %p\n", ct); - /* We have to check the DYING flag inside the lock to prevent - a race against nf_ct_get_next_corpse() possibly called from - user context, else we insert an already 'dead' hash, blocking - further use of that particular connection -JM */ - - if (unlikely(nf_ct_is_dying(ct))) { - nf_conntrack_double_unlock(hash, reply_hash); - local_bh_enable(); - return NF_ACCEPT; - } /* See if there's one in the list already, including reverse: NAT could have grabbed it without realizing, since we're @@ -636,8 +642,6 @@ __nf_conntrack_confirm(struct sk_buff *skb) zone == nf_ct_zone(nf_ct_tuplehash_to_ctrack(h))) goto out; - nf_ct_del_from_dying_or_unconfirmed_list(ct); - /* Timer relative to confirmation time, not original setting time, otherwise we'd get timer wrap in weird delay cases. */ -- 1.7.5.4