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 BD1A0B4FD for ; Fri, 13 Feb 2015 21:21:30 +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 1YMMjo-0006Sc-WA; Fri, 13 Feb 2015 15:21:26 -0500 Date: Fri, 13 Feb 2015 15:21:15 -0500 From: Neil Horman To: "Ananyev, Konstantin" Message-ID: <20150213202114.GA19071@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> <20150213175523.GA17402@neilslaptop.think-freely.org> <2601191342CEEE43887BDE71AB977258213E64ED@irsmsx105.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <2601191342CEEE43887BDE71AB977258213E64ED@irsmsx105.ger.corp.intel.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 20:21:31 -0000 On Fri, Feb 13, 2015 at 06:11:02PM +0000, Ananyev, Konstantin wrote: > > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman > > Sent: Friday, February 13, 2015 5:55 PM > > To: Olivier MATZ > > Cc: dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v6 04/19] eal: fix wrong strnlen() return value in 32bit icc > > > > 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. > > Why do you consider is as public API? > It is declared in private librte_eal header file: > lib/librte_eal/common/eal_options.h > So yes, in theory it can be used by some other functions inside librte_eal. > But it shouldn't be visible outside librte_eal. > Konstantin > I apologize, I misread the Makefile. I thought I saw that it was symlinked as a publically accessible header file, but thats not the case. As such, it should be ok. Acked-by: Neil Horman > > 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 > > >