[dpdk-dev] [PATCH v6 04/19] eal: fix wrong strnlen() return value in 32bit icc

Ananyev, Konstantin konstantin.ananyev at intel.com
Fri Feb 13 19:11:02 CET 2015



> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Neil Horman
> Sent: Friday, February 13, 2015 5:55 PM
> To: Olivier MATZ
> Cc: dev at 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 <konstantin.ananyev at intel.com>
> > >> Signed-off-by: Cunming Liang <cunming.liang at intel.com>
> > >> ---
> > >>  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

> 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
> >


More information about the dev mailing list