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

  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@intel.com>
Signed-off-by: Cunming Liang <cunming.liang@intel.com>
---
 v7 changes:
   rollback to use strnlen

 v5 changes:
   using strlen instead of strnlen.

 lib/librte_eal/common/eal_common_options.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
  

Hi,

On 02/15/2015 04:15 AM, 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@intel.com>
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>

Sorry but I don't think using strnlen() is appropriate here. See
http://dpdk.org/ml/archives/dev/2015-February/013309.html

Regards,
Olivier
  

> -----Original Message-----

> From: Olivier MATZ [mailto:olivier.matz@6wind.com]

> Sent: Monday, February 16, 2015 10:52 PM

> To: Liang, Cunming; dev@dpdk.org

> Cc: Ananyev, Konstantin; nhorman@tuxdriver.com

> Subject: Re: [PATCH v7 04/19] eal: fix wrong strnlen() return value in 32bit icc

> 

> Hi,

> 

> On 02/15/2015 04:15 AM, 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@intel.com>

> > Signed-off-by: Cunming Liang <cunming.liang@intel.com>

> 

> Sorry but I don't think using strnlen() is appropriate here. See

> http://dpdk.org/ml/archives/dev/2015-February/013309.html

> 

> I still don't agree that we should use strnlen(coremask, ARG_MAX).

> 

> The API of eal_parse_coremask() requires that a valid string is passed

> as an argument, so strlen() is perfectly fine. It's up to the caller to

> ensure that the string is valid.

[LCM] To me, personally I think either strlen() or strnlen(str, EAL_ARG_MAX) is ok.
Neil's point is that 'eal_parse_common_option()' extern as a EAL global function, itself should avoid the dirty input. That's for security programming.
As strlen()  intended to be used only to calculate the size of incoming untrusted data in a buffer of known size.
Your point is strlen() is enough as it only be used in EAL, and so far all input comes from optarg which is trusted data from getopt_long().
Add Thomas in cc list, I'll submit a v8 to make sure in both case there's patch series ready.
> 

> Using strnlen(coremask, ARG_MAX) in eal_parse_coremask() with an

> arbitrary length does not protect from having a segfault in case the

> string is invalid and the caller's buffer length is < ARG_MAX.

> [LCM] I'm afraid not getting your point here. It causes segfault only if the input string is NULL, doesn't it ?

As it already check the case, so using strnlen do protect against the unterminated string.
>

> This would still be true even if eal_parse_coremask() is public.

> 

> Regards,

> Olivier