From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 06968B4DA for ; Fri, 13 Feb 2015 18:55:38 +0100 (CET) Received: from [67.210.173.2] (helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1YMKSi-0004Pg-Il; Fri, 13 Feb 2015 12:55:35 -0500 Date: Fri, 13 Feb 2015 12:55:24 -0500 From: Neil Horman To: Olivier MATZ Message-ID: <20150213175523.GA17402@neilslaptop.think-freely.org> References: <1423728996-3004-1-git-send-email-cunming.liang@intel.com> <1423791501-1555-1-git-send-email-cunming.liang@intel.com> <1423791501-1555-5-git-send-email-cunming.liang@intel.com> <20150213134933.GA13495@neilslaptop.think-freely.org> <54DE04B8.6080708@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <54DE04B8.6080708@6wind.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v6 04/19] eal: fix wrong strnlen() return value in 32bit icc X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Feb 2015 17:55:38 -0000 On Fri, Feb 13, 2015 at 03:05:44PM +0100, Olivier MATZ wrote: > Hi Neil, > > On 02/13/2015 02:49 PM, Neil Horman wrote: > > On Fri, Feb 13, 2015 at 09:38:06AM +0800, Cunming Liang wrote: > >> The problem is that strnlen() here may return invalid value with 32bit icc. > >> (actually it returns it’s second parameter,e.g: sysconf(_SC_ARG_MAX)). > >> It starts to manifest hwen max_len parameter is > 2M and using icc –m32 –O2 (or above). > >> > >> Suggested-by: Konstantin Ananyev > >> Signed-off-by: Cunming Liang > >> --- > >> v5 changes: > >> using strlen instead of strnlen. > >> > >> lib/librte_eal/common/eal_common_options.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c > >> index 178e303..9cf2faa 100644 > >> --- a/lib/librte_eal/common/eal_common_options.c > >> +++ b/lib/librte_eal/common/eal_common_options.c > >> @@ -167,7 +167,7 @@ eal_parse_coremask(const char *coremask) > >> if (coremask[0] == '0' && ((coremask[1] == 'x') > >> || (coremask[1] == 'X'))) > >> coremask += 2; > >> - i = strnlen(coremask, PATH_MAX); > >> + i = strlen(coremask); > > This is crash prone. If coremask is passed in without a trailing null pointer, > > strlen will return a huge value that can overrun the array. > > We discussed that in a previous thread: > http://dpdk.org/ml/archives/dev/2015-February/012552.html > > coremask is always a valid nul-terminated string as it comes from > argv[] table. > It is not a memory fragment that is controlled by a user, so I don't > think using strnlen() instead of strlen() would solve any issue. > Thats absolutely false, you can't in any way make that assertion. eal_parse_common_option is a public API call. An application can construct its own string to pass into the parser. The test applications all use the command line functions so its not a visible issue from the test apps, but you can't assume what the test apps do is what everyone will do. It would be one thing if you could make the parse_common_option function private, but with the current layout you can't so you have to be ready for garbage input. Neil > Regards, > Olivier >