From patchwork Thu Jan 3 00:07:50 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alakesh Haloi X-Patchwork-Id: 1020117 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=amazon.com header.i=@amazon.com header.b="J7c7AwCG"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 43VStC51CKz9s55 for ; Thu, 3 Jan 2019 11:08:03 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726213AbfACAIC (ORCPT ); Wed, 2 Jan 2019 19:08:02 -0500 Received: from smtp-fw-6002.amazon.com ([52.95.49.90]:11131 "EHLO smtp-fw-6002.amazon.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725985AbfACAIC (ORCPT ); Wed, 2 Jan 2019 19:08:02 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazon201209; t=1546474081; x=1578010081; h=date:from:to:cc:subject:message-id:mime-version; bh=wMwfjztqtllnujWba/xtWuzibZOD2JYqYuzoDtsM8Kk=; b=J7c7AwCGq5/KZrjBdRX/WyX1LtyF9rk3E/a3G90h46ivjpdeKHDknRu/ 9+2EnhjieArZ9pHZ/H9dMCcI5ymXMR8Cq63GubNZuS4Bzf61nEceJYkGS 27EVne71Lkn+wuPaMSemhbLonbNpZaObru0rvwIgye034ba7vwlOK6UOW 8=; X-IronPort-AV: E=Sophos;i="5.56,253,1539648000"; d="scan'208";a="380030026" Received: from iad6-co-svc-p1-lb1-vlan3.amazon.com (HELO email-inbound-relay-2a-90c42d1d.us-west-2.amazon.com) ([10.124.125.6]) by smtp-border-fw-out-6002.iad6.amazon.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 03 Jan 2019 00:07:59 +0000 Received: from EX13MTAUWC001.ant.amazon.com (pdx1-ws-svc-p6-lb9-vlan2.pdx.amazon.com [10.236.137.194]) by email-inbound-relay-2a-90c42d1d.us-west-2.amazon.com (8.14.7/8.14.7) with ESMTP id x0307ve7090424 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Thu, 3 Jan 2019 00:07:57 GMT Received: from EX13D17UWC003.ant.amazon.com (10.43.162.206) by EX13MTAUWC001.ant.amazon.com (10.43.162.135) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Thu, 3 Jan 2019 00:07:56 +0000 Received: from dev-dsk-alakeshh-2c-f8a3e6e0.us-west-2.amazon.com (10.43.161.99) by EX13D17UWC003.ant.amazon.com (10.43.162.206) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Thu, 3 Jan 2019 00:07:56 +0000 Date: Thu, 3 Jan 2019 00:07:50 +0000 From: Alakesh Haloi To: CC: Pablo Neira Ayuso , Jozsef Kadlecsik , Florian Westphal , "David S. Miller" , Dmitry Andrianov , Justin Pettit , "Yi-Hung Wei" , Subject: [PATCH v2] netfilter: xt_connlimit: fix race in connection counting Message-ID: <20190103000742.GA108711@dev-dsk-alakeshh-2c-f8a3e6e0.us-west-2.amazon.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-Originating-IP: [10.43.161.99] X-ClientProxiedBy: EX13D23UWC004.ant.amazon.com (10.43.162.219) To EX13D17UWC003.ant.amazon.com (10.43.162.206) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org An iptable rule like the following on a multicore systems will result in accepting more connections than set in the rule. iptables -A INPUT -p tcp -m tcp --syn --dport 7777 -m connlimit \ --connlimit-above 2000 --connlimit-mask 0 -j DROP In check_hlist function, connections that are found in saved connections but not in netfilter conntrack are deleted, assuming that those connections do not exist anymore. But for multi core systems, there exists a small time window, when a connection has been added to the xt_connlimit maintained rb-tree but has not yet made to netfilter conntrack table. This causes concurrent connections to return incorrect counts and go over limit set in iptable rule. The fix has been partially backported from the above mentioned upstream commit. Introduce timestamp and the owning cpu. Signed-off-by: Alakesh Haloi Cc: Pablo Neira Ayuso Cc: Jozsef Kadlecsik Cc: Florian Westphal Cc: "David S. Miller" Cc: stable@vger.kernel.org # v4.15 and before Cc: netdev@vger.kernel.org Cc: Dmitry Andrianov Cc: Justin Pettit Cc: Yi-Hung Wei --- net/netfilter/xt_connlimit.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/net/netfilter/xt_connlimit.c b/net/netfilter/xt_connlimit.c index ffa8eec..e7b092b 100644 --- a/net/netfilter/xt_connlimit.c +++ b/net/netfilter/xt_connlimit.c @@ -47,6 +47,8 @@ struct xt_connlimit_conn { struct hlist_node node; struct nf_conntrack_tuple tuple; union nf_inet_addr addr; + int cpu; + u32 jiffies32; }; struct xt_connlimit_rb { @@ -126,6 +128,8 @@ static bool add_hlist(struct hlist_head *head, return false; conn->tuple = *tuple; conn->addr = *addr; + conn->cpu = raw_smp_processor_id(); + conn->jiffies32 = (u32)jiffies; hlist_add_head(&conn->node, head); return true; } @@ -148,8 +152,26 @@ static unsigned int check_hlist(struct net *net, hlist_for_each_entry_safe(conn, n, head, node) { found = nf_conntrack_find_get(net, zone, &conn->tuple); if (found == NULL) { - hlist_del(&conn->node); - kmem_cache_free(connlimit_conn_cachep, conn); + /* If connection is not found, it may be because + * it has not made into conntrack table yet. We + * check if it is a recently created connection + * on a different core and do not delete it in that + * case. + */ + + unsigned long a, b; + int cpu = raw_smp_processor_id(); + __u32 age; + + b = conn->jiffies; + a = (u32)jiffies; + age = a - b; + if (conn->cpu != cpu && age <= 2) { + length++; + } else { + hlist_del(&conn->node); + kmem_cache_free(connlimit_conn_cachep, conn); + } continue; } @@ -271,6 +293,8 @@ static void tree_nodes_free(struct rb_root *root, conn->tuple = *tuple; conn->addr = *addr; + conn->cpu = raw_smp_processor_id(); + conn->jiffies32 = (u32)jiffies; rbconn->addr = *addr; INIT_HLIST_HEAD(&rbconn->hhead);