> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Monday, February 16, 2015 10:52 PM
> To: Liang, Cunming; dev at dpdk.org
> Cc: Ananyev, Konstantin; nhorman at 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 at intel.com>
> > Signed-off-by: Cunming Liang <cunming.liang at 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
