netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick McHardy <kaber@trash.net>
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, Patrick McHardy <kaber@trash.net>,
	netfilter-devel@vger.kernel.org
Subject: netfilter 02/07: nf_conntrack: fix confirmation race condition
Date: Mon, 22 Jun 2009 14:53:51 +0200 (MEST)	[thread overview]
Message-ID: <20090622125351.6531.50859.sendpatchset@x2.localnet> (raw)
In-Reply-To: <20090622125349.6531.35515.sendpatchset@x2.localnet>

commit 5c8ec910e789a92229978d8fd1fce7b62e8ac711
Author: Patrick McHardy <kaber@trash.net>
Date:   Mon Jun 22 14:14:16 2009 +0200

    netfilter: nf_conntrack: fix confirmation race condition
    
    New connection tracking entries are inserted into the hash before they
    are fully set up, namely the CONFIRMED bit is not set and the timer not
    started yet. This can theoretically lead to a race with timer, which
    would set the timeout value to a relative value, most likely already in
    the past.
    
    Perform hash insertion as the final step to fix this.
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 5276a2d..b0b06c7 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -425,7 +425,6 @@ __nf_conntrack_confirm(struct sk_buff *skb)
 	/* Remove from unconfirmed list */
 	hlist_nulls_del_rcu(&ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode);
 
-	__nf_conntrack_hash_insert(ct, hash, repl_hash);
 	/* Timer relative to confirmation time, not original
 	   setting time, otherwise we'd get timer wrap in
 	   weird delay cases. */
@@ -433,8 +432,16 @@ __nf_conntrack_confirm(struct sk_buff *skb)
 	add_timer(&ct->timeout);
 	atomic_inc(&ct->ct_general.use);
 	set_bit(IPS_CONFIRMED_BIT, &ct->status);
+
+	/* Since the lookup is lockless, hash insertion must be done after
+	 * starting the timer and setting the CONFIRMED bit. The RCU barriers
+	 * guarantee that no other CPU can find the conntrack before the above
+	 * stores are visible.
+	 */
+	__nf_conntrack_hash_insert(ct, hash, repl_hash);
 	NF_CT_STAT_INC(net, insert);
 	spin_unlock_bh(&nf_conntrack_lock);
+
 	help = nfct_help(ct);
 	if (help && help->helper)
 		nf_conntrack_event_cache(IPCT_HELPER, ct);

  parent reply	other threads:[~2009-06-22 12:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-22 12:53 netfilter 00/07: netfilter fixes Patrick McHardy
2009-06-22 12:53 ` netfilter 01/07: nf_conntrack: death_by_timeout() fix Patrick McHardy
2009-06-22 12:53 ` Patrick McHardy [this message]
2009-06-22 12:53 ` netfilter 03/07: nf_conntrack: fix conntrack lookup race Patrick McHardy
2009-06-22 12:53 ` netfilter 04/07: fix some sparse endianess warnings Patrick McHardy
2009-06-22 12:53 ` netfilter 05/07: nf_log: fix direct userspace memory access in proc handler Patrick McHardy
2009-06-22 12:53 ` netfilter 06/07: xt_quota: fix incomplete initialization Patrick McHardy
2009-06-22 12:53 ` netfilter 07/07: xt_rateest: fix comparison with self Patrick McHardy
2009-06-22 22:56 ` netfilter 00/07: netfilter fixes David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090622125351.6531.50859.sendpatchset@x2.localnet \
    --to=kaber@trash.net \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).