netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: pablo@netfilter.org
To: stable@vger.kernel.org
Cc: davem@davemloft.net, netfilter-devel@vger.kernel.org
Subject: [stable-3.7 10/14] netfilter: xt_hashlimit: fix race that results in duplicated entries
Date: Mon, 28 Jan 2013 20:31:29 +0100	[thread overview]
Message-ID: <1359401493-6196-11-git-send-email-pablo@netfilter.org> (raw)
In-Reply-To: <1359401493-6196-1-git-send-email-pablo@netfilter.org>

From: Pablo Neira Ayuso <pablo@netfilter.org>

Two packets may race to create the same entry in the hashtable,
double check if this packet lost race. This double checking only
happens in the path of the packet that creates the hashtable for
first time.

Note that, with this patch, no packet drops occur if the race happens.

Reported-by: Feng Gao <gfree.wind@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/xt_hashlimit.c |   25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 30ad0b62..a9d7af9 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -157,11 +157,22 @@ dsthash_find(const struct xt_hashlimit_htable *ht,
 /* allocate dsthash_ent, initialize dst, put in htable and lock it */
 static struct dsthash_ent *
 dsthash_alloc_init(struct xt_hashlimit_htable *ht,
-		   const struct dsthash_dst *dst)
+		   const struct dsthash_dst *dst, bool *race)
 {
 	struct dsthash_ent *ent;
 
 	spin_lock(&ht->lock);
+
+	/* Two or more packets may race to create the same entry in the
+	 * hashtable, double check if this packet lost race.
+	 */
+	ent = dsthash_find(ht, dst);
+	if (ent != NULL) {
+		spin_unlock(&ht->lock);
+		*race = true;
+		return ent;
+	}
+
 	/* initialize hash with random val at the time we allocate
 	 * the first hashtable entry */
 	if (unlikely(!ht->rnd_initialized)) {
@@ -588,6 +599,7 @@ hashlimit_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	unsigned long now = jiffies;
 	struct dsthash_ent *dh;
 	struct dsthash_dst dst;
+	bool race = false;
 	u32 cost;
 
 	if (hashlimit_init_dst(hinfo, &dst, skb, par->thoff) < 0)
@@ -596,13 +608,18 @@ hashlimit_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	rcu_read_lock_bh();
 	dh = dsthash_find(hinfo, &dst);
 	if (dh == NULL) {
-		dh = dsthash_alloc_init(hinfo, &dst);
+		dh = dsthash_alloc_init(hinfo, &dst, &race);
 		if (dh == NULL) {
 			rcu_read_unlock_bh();
 			goto hotdrop;
+		} else if (race) {
+			/* Already got an entry, update expiration timeout */
+			dh->expires = now + msecs_to_jiffies(hinfo->cfg.expire);
+			rateinfo_recalc(dh, now, hinfo->cfg.mode);
+		} else {
+			dh->expires = jiffies + msecs_to_jiffies(hinfo->cfg.expire);
+			rateinfo_init(dh, hinfo);
 		}
-		dh->expires = jiffies + msecs_to_jiffies(hinfo->cfg.expire);
-		rateinfo_init(dh, hinfo);
 	} else {
 		/* update expiration timeout */
 		dh->expires = now + msecs_to_jiffies(hinfo->cfg.expire);
-- 
1.7.10.4


  parent reply	other threads:[~2013-01-28 19:32 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-28 19:31 [stable-3.7 00/14] netfilter stable fixes for 3.7 pablo
2013-01-28 19:31 ` [stable-3.7 01/14] netfilter: ip[6]t_REJECT: fix wrong transport header pointer in TCP reset pablo
2013-01-28 19:31 ` [stable-3.7 02/14] netfilter: nf_ct_reasm: fix conntrack reassembly expire code pablo
2013-01-28 19:31 ` [stable-3.7 03/14] netfilter: nfnetlink_log: fix mac address for 6in4 tunnels pablo
2013-01-28 19:31 ` [stable-3.7 04/14] netfilter: nfnetlink_log: fix possible compilation issue due to missing include pablo
2013-01-28 19:31 ` [stable-3.7 05/14] netfilter: xt_CT: recover NOTRACK target support pablo
2013-01-28 19:31 ` [stable-3.7 06/14] netfilter: fix missing dependencies for the NOTRACK target pablo
2013-01-28 19:31 ` [stable-3.7 07/14] netfilter: xt_recent: fix namespace destroy path pablo
2013-01-28 19:31 ` [stable-3.7 08/14] netfilter: xt_recent: avoid high order page allocations pablo
2013-01-28 19:31 ` [stable-3.7 09/14] netfilter: xt_hashlimit: fix namespace destroy path pablo
2013-01-28 19:31 ` pablo [this message]
2013-01-28 19:31 ` [stable-3.7 11/14] netfilter: xt_CT: fix unset return value if conntrack zone are disabled pablo
2013-01-28 19:31 ` [stable-3.7 12/14] netfilter: nf_conntrack: fix BUG_ON while removing nf_conntrack with netns pablo
2013-01-28 19:31 ` [stable-3.7 13/14] netfilter: x_tables: print correct hook names for ARP pablo
2013-01-28 19:31 ` [stable-3.7 14/14] netfilter: ctnetlink: fix leak in error path of ctnetlink_create_expect pablo
2013-01-28 20:52 ` [stable-3.7 00/14] netfilter stable fixes for 3.7 David Miller
2013-02-01 12:34 ` Greg KH
2013-02-01 16:25   ` Pablo Neira Ayuso
2013-02-01 16:39     ` Greg KH

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=1359401493-6196-11-git-send-email-pablo@netfilter.org \
    --to=pablo@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=stable@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).