From patchwork Fri Dec 28 11:52:48 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pablo Neira Ayuso X-Patchwork-Id: 208455 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 005E22C00D7 for ; Fri, 28 Dec 2012 22:53:29 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753544Ab2L1Lx0 (ORCPT ); Fri, 28 Dec 2012 06:53:26 -0500 Received: from mail.us.es ([193.147.175.20]:51037 "EHLO mail.us.es" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753552Ab2L1LxU (ORCPT ); Fri, 28 Dec 2012 06:53:20 -0500 Received: (qmail 20705 invoked from network); 28 Dec 2012 12:53:19 +0100 Received: from unknown (HELO us.es) (192.168.2.12) by us.es with SMTP; 28 Dec 2012 12:53:19 +0100 Received: (qmail 14690 invoked by uid 507); 28 Dec 2012 11:53:19 -0000 X-Qmail-Scanner-Diagnostics: from 127.0.0.1 by antivirus2 (envelope-from , uid 501) with qmail-scanner-2.10 (clamdscan: 0.97.6/16162. spamassassin: 3.3.2. Clear:RC:1(127.0.0.1):SA:0(-97.0/7.5):. Processed in 1.810851 secs); 28 Dec 2012 11:53:19 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on antivirus2 X-Spam-Level: X-Spam-Status: No, score=-97.0 required=7.5 tests=BAYES_50, RCVD_IN_BRBL_LASTEXT,RCVD_IN_PBL,RCVD_IN_SORBS_DUL,RDNS_DYNAMIC, USER_IN_WHITELIST autolearn=disabled version=3.3.2 X-Envelope-From: pablo@netfilter.org Received: from unknown (HELO antivirus2) (127.0.0.1) by us.es with SMTP; 28 Dec 2012 11:53:17 -0000 Received: from 192.168.1.13 (192.168.1.13) by antivirus2 (F-Secure/fsigk_smtp/407/antivirus2); Fri, 28 Dec 2012 12:53:17 +0100 (CET) X-Virus-Status: clean(F-Secure/fsigk_smtp/407/antivirus2) Received: (qmail 27774 invoked from network); 28 Dec 2012 12:53:16 +0100 Received: from 240.161.220.87.dynamic.jazztel.es (HELO localhost.localdomain) (pneira@us.es@87.220.161.240) by us.es with SMTP; 28 Dec 2012 12:53:16 +0100 From: pablo@netfilter.org To: netfilter-devel@vger.kernel.org Cc: davem@davemloft.net, netdev@vger.kernel.org Subject: [PATCH 09/12] netfilter: xt_hashlimit: fix race that results in duplicated entries Date: Fri, 28 Dec 2012 12:52:48 +0100 Message-Id: <1356695571-3305-10-git-send-email-pablo@netfilter.org> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1356695571-3305-1-git-send-email-pablo@netfilter.org> References: <1356695571-3305-1-git-send-email-pablo@netfilter.org> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Pablo Neira Ayuso 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 Signed-off-by: Pablo Neira Ayuso --- 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 26a668a..cc430f9 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)) { @@ -585,6 +596,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) @@ -593,13 +605,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);