From patchwork Mon Feb 6 22:50:33 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alban Browaeys X-Patchwork-Id: 724800 X-Patchwork-Delegate: pablo@netfilter.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3vHN5q1J8Cz9s2P for ; Tue, 7 Feb 2017 09:52:31 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="M/CZQjpB"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752209AbdBFWvs (ORCPT ); Mon, 6 Feb 2017 17:51:48 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:35055 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752207AbdBFWvN (ORCPT ); Mon, 6 Feb 2017 17:51:13 -0500 Received: by mail-wm0-f67.google.com with SMTP id u63so24883826wmu.2; Mon, 06 Feb 2017 14:51:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=OPkxMWTT0qaAzE3MSIvsTUmQvpPNyGt6Ak3oxZsjH+A=; b=M/CZQjpBSLW8h2ov5XxNVpP8CI2RPcEjC4VehbRkUadyWSdhJgnGTPiM3sULthb/fC /Lnwh/EUV8hS0zeFwflFtsm2hSb1yA7Ug53nEbnTRxqd4NdiF5Fu0EMswgbLsGPrfqYg +431n5ik3FFLzQIjrBqK8vKA0DLmz9mn/4wxIbBT+wMS1/Q/jRNut4vKHIfCFBPoSuEL XjBm2DItIrYR/9JcDL73z1+ynnSIMVJAGGdDWw/1Zz6xvbqkE/cBKFLnLVeKPzot6CrU ttAArQtmmeGvabRXUcDxIE5ca3NJPBQeLF/0I64XyfTY0PNIMxESQ2X62iNDKlUODXUN 2Hig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=OPkxMWTT0qaAzE3MSIvsTUmQvpPNyGt6Ak3oxZsjH+A=; b=mnGrsXnFcNDXfHavKamo72kAidW+P48NoM5PNR7qt0IrNRIOvW3Skk1io85kD6yhtu cyDU+TV+MzTRhP4BJSDah5O4j+L2rYhSKu3XPGLnOZZ1TlB1pq8mQLRLP3+O7n4BeOg7 JpcCQBzOn6qcX4sjJgPsdoLDof44yMEMjtulSHwocY/ERP6SUE4x81QlREygD37+zsyk RK2B/PsMzRbAJ41nn9kF5YnPkp5UrrGIUmH50s7yQA2y0FFC/9R6emgAnvIt6h5OjOmh P6LF46GLGFGPTB23I3OaNQG+JZxVXeiNcqReneS9H1YtWTcy9AWBTIZ3lxfzgIulu2bK FGiw== X-Gm-Message-State: AMke39n+z82y0IlZX+ux+R7rHtRriR5ROIUQq4iD09ciMCyoROtRGq1bR98p4kyOopYX6A== X-Received: by 10.28.88.6 with SMTP id m6mr10110164wmb.4.1486421466455; Mon, 06 Feb 2017 14:51:06 -0800 (PST) Received: from cyclope.prahal.homelinux.net (ARouen-653-1-38-190.w90-22.abo.wanadoo.fr. [90.22.197.190]) by smtp.gmail.com with ESMTPSA id y97sm15446625wmh.24.2017.02.06.14.51.05 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 06 Feb 2017 14:51:05 -0800 (PST) From: Alban Browaeys To: Pablo Neira Ayuso Cc: Patrick McHardy , Jozsef Kadlecsik , "David S. Miller" , netfilter-devel@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Alban Browaeys Subject: [PATCH v2] netfilter: xt_hashlimit: Fix integer divide round to zero. Date: Mon, 6 Feb 2017 23:50:33 +0100 Message-Id: <20170206225033.26231-1-alban.browaeys@gmail.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <1486391287.2374.1.camel@gmail.com> References: <1486391287.2374.1.camel@gmail.com> Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Diving the divider by the multiplier before applying to the input. When this would "divide by zero", divide the multiplier by the divider first then multiply the input by this value. Currently user2creds outputs zero when input value is bigger than the number of slices and lower than scale. This as then user input is applied an integer divide operation to a number greater than itself (scale). That rounds up to zero, then we mulitply zero by the credits slice size. iptables -t filter -I INPUT --protocol tcp --match hashlimit --hashlimit 40/second --hashlimit-burst 20 --hashlimit-mode srcip --hashlimit-name syn-flood --jump RETURN thus trigger the overflow detection code: xt_hashlimit: overflow, try lower: 25000/20 (25000 as hashlimit avd and 20 the burst) Here: 134217 slices of (HZ * CREDITS_PER_JIFFY) size. 500000 is user input value 1000000 is XT_HASHLIMIT_SCALE_v2 gives: 0 as user2creds output Setting burst to "1" typically solve the issue ... but setting it to "40" does too ! This is on 32bit arch calling into revision 2 of hashlimit. Signed-off-by: Alban Browaeys --- v2: - fix missing conditional statement in revision 1 case . I removed the code duplication between revision 1 and 2. v1: https://lkml.org/lkml/2017/2/4/173 --- net/netfilter/xt_hashlimit.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c index 10063408141d..84ad5ab34558 100644 --- a/net/netfilter/xt_hashlimit.c +++ b/net/netfilter/xt_hashlimit.c @@ -463,23 +463,16 @@ static u32 xt_hashlimit_len_to_chunks(u32 len) /* Precision saver. */ static u64 user2credits(u64 user, int revision) { - if (revision == 1) { - /* If multiplying would overflow... */ - if (user > 0xFFFFFFFF / (HZ*CREDITS_PER_JIFFY_v1)) - /* Divide first. */ - return div64_u64(user, XT_HASHLIMIT_SCALE) - * HZ * CREDITS_PER_JIFFY_v1; - - return div64_u64(user * HZ * CREDITS_PER_JIFFY_v1, - XT_HASHLIMIT_SCALE); - } else { - if (user > 0xFFFFFFFFFFFFFFFFULL / (HZ*CREDITS_PER_JIFFY)) - return div64_u64(user, XT_HASHLIMIT_SCALE_v2) - * HZ * CREDITS_PER_JIFFY; + u64 scale = (revision == 1) ? + XT_HASHLIMIT_SCALE : XT_HASHLIMIT_SCALE_v2; + u64 cpj = (revision == 1) ? + CREDITS_PER_JIFFY_v1 : CREDITS_PER_JIFFY; - return div64_u64(user * HZ * CREDITS_PER_JIFFY, - XT_HASHLIMIT_SCALE_v2); - } + /* Avoid overflow: divide the constant operands first */ + if (scale >= HZ * cpj) + return div64_u64(user, div64_u64(scale, HZ * cpj)); + + return user * div64_u64(HZ * cpj, scale); } static u32 user2credits_byte(u32 user)