From patchwork Fri Dec 23 13:06:32 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 133046 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 299F3B71A2 for ; Sat, 24 Dec 2011 00:06:43 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756888Ab1LWNGh (ORCPT ); Fri, 23 Dec 2011 08:06:37 -0500 Received: from mail-ww0-f44.google.com ([74.125.82.44]:64106 "EHLO mail-ww0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753344Ab1LWNGf (ORCPT ); Fri, 23 Dec 2011 08:06:35 -0500 Received: by wgbdr13 with SMTP id dr13so17439360wgb.1 for ; Fri, 23 Dec 2011 05:06:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=message-id:subject:from:to:cc:date:in-reply-to:references :content-type:x-mailer:content-transfer-encoding:mime-version; bh=iKeff9XH+XjoErsno5fsvCM+EkzKj8nCYqwPdZZdq6g=; b=XP+bCJiO8OX4BZLXajfdUpBIfkCgOBIODBPeG+Adf4qTWp22nBCM6OCc6CJFBKMSMW 2F59A/weA7JBtyqMjQyOaWpkktfntMaTamAc/sXrTZrkVAsn5WWX8oH0mtNr4HQBpK6/ EpVso9AsKzn19prqHZ9uV1TKI02zBn5KsYbXw= Received: by 10.216.135.233 with SMTP id u83mr13986757wei.33.1324645594671; Fri, 23 Dec 2011 05:06:34 -0800 (PST) Received: from [10.150.51.215] (gw0.net.jmsp.net. [212.23.165.14]) by mx.google.com with ESMTPS id k5sm31492151wiz.9.2011.12.23.05.06.32 (version=SSLv3 cipher=OTHER); Fri, 23 Dec 2011 05:06:33 -0800 (PST) Message-ID: <1324645592.2223.9.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC> Subject: Re: [PATCH v2] rps: fix insufficient bounds checking in store_rps_dev_flow_table_cnt() From: Eric Dumazet To: Xi Wang Cc: Tom Herbert , "David S. Miller" , netdev@vger.kernel.org Date: Fri, 23 Dec 2011 14:06:32 +0100 In-Reply-To: <4DA4756B-0654-49F4-B135-9A9F89BC7D21@gmail.com> References: <1324493459-19764-1-git-send-email-xi.wang@gmail.com> <4EF3BEBA.4040402@gmail.com> <1324613414.2674.2.camel@edumazet-laptop> <1324616007.2674.8.camel@edumazet-laptop> <1324617390.2674.13.camel@edumazet-laptop> <1324618555.10854.4.camel@edumazet-laptop> <4DA4756B-0654-49F4-B135-9A9F89BC7D21@gmail.com> X-Mailer: Evolution 3.2.1- Mime-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Le vendredi 23 décembre 2011 à 00:56 -0500, Xi Wang a écrit : > On Dec 23, 2011, at 12:35 AM, Eric Dumazet wrote: > > By the way, the theorical limit on number of flows on 64bit platform is > > 2^32 (rxhash being an u32) > > > > Not sure spending 32GB per table would be wise for typical machines :) > > True, a large rps_flow_cnt is not that useful. ;-) > > Anyway, my point is that if the patch looks confusing to you, then > it is probably not good. I am happy to see a more elegant fix. > I'll submit following patch for net-next, once your patch is in this tree. - Full u32 range on 64bit arches (2^32 flows max) - Use of kstrtoul() instead of simple_strtoul() net/core/net-sysfs.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index 385aefe..63bd152 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -618,7 +618,7 @@ static ssize_t show_rps_dev_flow_table_cnt(struct netdev_rx_queue *queue, char *buf) { struct rps_dev_flow_table *flow_table; - unsigned int val = 0; + unsigned long val = 0; rcu_read_lock(); flow_table = rcu_dereference(queue->rps_flow_table); @@ -626,7 +626,7 @@ static ssize_t show_rps_dev_flow_table_cnt(struct netdev_rx_queue *queue, val = flow_table->mask + 1; rcu_read_unlock(); - return sprintf(buf, "%u\n", val); + return sprintf(buf, "%lu\n", val); } static void rps_dev_flow_table_release_work(struct work_struct *work) @@ -650,24 +650,23 @@ static ssize_t store_rps_dev_flow_table_cnt(struct netdev_rx_queue *queue, struct rx_queue_attribute *attr, const char *buf, size_t len) { - unsigned int count; - char *endp; + unsigned long i, count; struct rps_dev_flow_table *table, *old_table; static DEFINE_SPINLOCK(rps_dev_flow_lock); + int rc; if (!capable(CAP_NET_ADMIN)) return -EPERM; - count = simple_strtoul(buf, &endp, 0); - if (endp == buf) - return -EINVAL; + rc = kstrtoul(buf, 0, &count); + if (rc < 0) + return rc; if (count) { - int i; - - if (count > INT_MAX) - return -EINVAL; count = roundup_pow_of_two(count); + if (!count || + count != (unsigned long)(u32)count) + return -EINVAL; if (count > (ULONG_MAX - sizeof(struct rps_dev_flow_table)) / sizeof(struct rps_dev_flow)) { /* Enforce a limit to prevent overflow */