Re: [dpdk-dev] [PATCH] eal: fix core number validation

[David Marchand <david.marchand@redhat.com>]

On Thu, Dec 20, 2018 at 11:01 AM Hari Kumar Vemula <
hari.kumarx.vemula@intel.com> wrote:

> When incorrect core value or range provided,
> as part of -l command line option, a crash occurs.
>
> Added valid range checks to fix the crash.
>
> Fixes: d888cb8b9613 ("eal: add core list input format")
> Cc: stable@dpdk.org
>
> Signed-off-by: Hari Kumar Vemula <hari.kumarx.vemula@intel.com>
> ---
>  lib/librte_eal/common/eal_common_options.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/lib/librte_eal/common/eal_common_options.c
> b/lib/librte_eal/common/eal_common_options.c
> index e31eca5c0..ea6bb508b 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -602,6 +602,14 @@ eal_parse_corelist(const char *corelist)
>                         max = idx;
>                         if (min == RTE_MAX_LCORE)
>                                 min = idx;
> +                       if ((unsigned int)idx >= cfg->lcore_count ||
> +                                       (unsigned int)min >=
> cfg->lcore_count) {
>

Rather than check those conditions here, check directly after parsing the
input string.

Those casts will have side effects with negative values and the use of
current strtoul is suspicious to me.
You should switch to strtol, check for negative values and for the max
value according to cfg->lcore_count.



> +                               RTE_LOG(ERR, EAL,
> +                                       "Invalid core number given core
> range should be(0-%u)\n",
> +                                       cfg->lcore_count);
>

This log message will be a duplicate with the message in
eal_parse_common_option() so please drop this.

We could add the range in the current log message in
eal_parse_common_option(), but please, the range is [0, cfg->lcore_count -
1].


+                               return -1;
> +                       }
> +
>                         for (idx = min; idx <= max; idx++) {
>                                 if (cfg->lcore_role[idx] != ROLE_RTE) {
>                                         cfg->lcore_role[idx] = ROLE_RTE;
>


-- 
David Marchand