* [dpdk-dev] [PATCH] eal: fix core number validation @ 2018-12-20 10:00 Hari Kumar Vemula 2018-12-21 8:27 ` David Marchand 2019-01-02 14:23 ` David Marchand 0 siblings, 2 replies; 6+ messages in thread From: Hari Kumar Vemula @ 2018-12-20 10:00 UTC (permalink / raw) To: dev; +Cc: reshma.pattan, ferruh.yigit, Hari Kumar Vemula, stable 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) { + RTE_LOG(ERR, EAL, + "Invalid core number given core range should be(0-%u)\n", + cfg->lcore_count); + return -1; + } + for (idx = min; idx <= max; idx++) { if (cfg->lcore_role[idx] != ROLE_RTE) { cfg->lcore_role[idx] = ROLE_RTE; -- 2.17.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: fix core number validation 2018-12-20 10:00 [dpdk-dev] [PATCH] eal: fix core number validation Hari Kumar Vemula @ 2018-12-21 8:27 ` David Marchand 2018-12-21 9:19 ` Burakov, Anatoly 2019-01-02 14:23 ` David Marchand 1 sibling, 1 reply; 6+ messages in thread From: David Marchand @ 2018-12-21 8:27 UTC (permalink / raw) To: Hari Kumar Vemula; +Cc: dev, reshma.pattan, Yigit, Ferruh, stable 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> > Thanks for reporting. I agree that some validation steps are missing, but I tried a little bit and did not reproduce a crash. On my 8 cores system: [dmarchan@dmarchan dpdk]$ ./master/app/testpmd --no-huge -l 567890,567891,567892 -m 512 --log-level *:debug -- -i --total-num-mbufs 2048 [...] EAL: Support maximum 128 logical core(s) by configuration. EAL: Detected 8 lcore(s) [...] EAL: invalid core list Usage: ./master/app/testpmd [options] etc... [dmarchan@dmarchan dpdk]$ ./master/app/testpmd --no-huge -l 2,3,-1 -m 512 --log-level *:debug -- -i --total-num-mbufs 2048 [...] EAL: Support maximum 128 logical core(s) by configuration. EAL: Detected 8 lcore(s) EAL: Detected 1 NUMA nodes [...] Done testpmd> Bye... Idem with [dmarchan@dmarchan dpdk]$ ./master/app/testpmd --no-huge -l 2,3,567890 -m 512 --log-level *:debug -- -i --total-num-mbufs 2048 [...] EAL: Support maximum 128 logical core(s) by configuration. EAL: Detected 8 lcore(s) EAL: Detected 1 NUMA nodes [...] Done testpmd> Bye... Since you have identified a potential crash, can you give an example of such a crash ? Besides, we have tests that check arguments, so an update of the test would be nice. Thanks. -- David Marchand ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: fix core number validation 2018-12-21 8:27 ` David Marchand @ 2018-12-21 9:19 ` Burakov, Anatoly 2018-12-21 9:28 ` David Marchand 0 siblings, 1 reply; 6+ messages in thread From: Burakov, Anatoly @ 2018-12-21 9:19 UTC (permalink / raw) To: David Marchand, Hari Kumar Vemula Cc: dev, reshma.pattan, Yigit, Ferruh, stable On 21-Dec-18 8:27 AM, David Marchand wrote: > 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> >> > > Thanks for reporting. > I agree that some validation steps are missing, but I tried a little bit > and did not reproduce a crash. > > On my 8 cores system: > [dmarchan@dmarchan dpdk]$ ./master/app/testpmd --no-huge -l > 567890,567891,567892 -m 512 --log-level *:debug -- -i --total-num-mbufs 2048 > [...] > EAL: Support maximum 128 logical core(s) by configuration. > EAL: Detected 8 lcore(s) > [...] > EAL: invalid core list > > Usage: ./master/app/testpmd [options] > etc... > > > [dmarchan@dmarchan dpdk]$ ./master/app/testpmd --no-huge -l 2,3,-1 -m 512 > --log-level *:debug -- -i --total-num-mbufs 2048 > [...] > EAL: Support maximum 128 logical core(s) by configuration. > EAL: Detected 8 lcore(s) > EAL: Detected 1 NUMA nodes > [...] > Done > testpmd> > Bye... > > Idem with > > [dmarchan@dmarchan dpdk]$ ./master/app/testpmd --no-huge -l 2,3,567890 -m > 512 --log-level *:debug -- -i --total-num-mbufs 2048 > [...] > EAL: Support maximum 128 logical core(s) by configuration. > EAL: Detected 8 lcore(s) > EAL: Detected 1 NUMA nodes > [...] > Done > testpmd> > Bye... > > > Since you have identified a potential crash, can you give an example of > such a crash ? > Besides, we have tests that check arguments, so an update of the test would > be nice. > > Thanks. > I believe these lcore numbers are used to index the lcore list later, which would cause out-of-bounds access, which may or may not cause a crash, depending on how lucky you get. -- Thanks, Anatoly ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: fix core number validation 2018-12-21 9:19 ` Burakov, Anatoly @ 2018-12-21 9:28 ` David Marchand 2019-01-02 12:47 ` Vemula, Hari KumarX 0 siblings, 1 reply; 6+ messages in thread From: David Marchand @ 2018-12-21 9:28 UTC (permalink / raw) To: Burakov, Anatoly, Hari Kumar Vemula Cc: dev, reshma.pattan, Yigit, Ferruh, stable On Fri, Dec 21, 2018 at 10:19 AM Burakov, Anatoly <anatoly.burakov@intel.com> wrote: > On 21-Dec-18 8:27 AM, David Marchand wrote: > > Since you have identified a potential crash, can you give an example of > > such a crash ? > > Besides, we have tests that check arguments, so an update of the test > would > > be nice. > > I believe these lcore numbers are used to index the lcore list later, > which would cause out-of-bounds access, which may or may not cause a > crash, depending on how lucky you get. > Sure, that and the fact that my testpmd was doing nothing with them :-). Anyway, the unit tests missed this case, so we need an update. And the more I look at those string parsing, the more I think that the service cores parsing has the same issue (copy/paste yay ;-)). -- David Marchand ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: fix core number validation 2018-12-21 9:28 ` David Marchand @ 2019-01-02 12:47 ` Vemula, Hari KumarX 0 siblings, 0 replies; 6+ messages in thread From: Vemula, Hari KumarX @ 2019-01-02 12:47 UTC (permalink / raw) To: David Marchand, Burakov, Anatoly Cc: dev, Pattan, Reshma, Yigit, Ferruh, stable Hi, From: David Marchand [mailto:david.marchand@redhat.com] Sent: Friday, December 21, 2018 2:58 PM To: Burakov, Anatoly <anatoly.burakov@intel.com>; Vemula, Hari KumarX <hari.kumarx.vemula@intel.com> Cc: dev@dpdk.org; Pattan, Reshma <reshma.pattan@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org Subject: Re: [dpdk-dev] [PATCH] eal: fix core number validation On Fri, Dec 21, 2018 at 10:19 AM Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>> wrote: On 21-Dec-18 8:27 AM, David Marchand wrote: > Since you have identified a potential crash, can you give an example of > such a crash ? > Besides, we have tests that check arguments, so an update of the test would > be nice. I believe these lcore numbers are used to index the lcore list later, which would cause out-of-bounds access, which may or may not cause a crash, depending on how lucky you get. Sure, that and the fact that my testpmd was doing nothing with them :-). Anyway, the unit tests missed this case, so we need an update. And the more I look at those string parsing, the more I think that the service cores parsing has the same issue (copy/paste yay ;-)). DPDK EAL doesn’t have eal parameter valid/invalid core number check. For example, if we use following command to launch dpdk testpmd sample, it will return segmentation fault directly: ./x86_64-native-linuxapp-gcc/app/testpmd -l 9999999 -n 4 –-i EAL: Detected 48 lcore(s) EAL: Detected 2 NUMA nodes Segmentation fault (core dumped) UT is present for -l command line option to validate core number, but there is no check for higher core list number like above. -- Hari. -- David Marchand -------------------------------------------------------------- Intel Research and Development Ireland Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: fix core number validation 2018-12-20 10:00 [dpdk-dev] [PATCH] eal: fix core number validation Hari Kumar Vemula 2018-12-21 8:27 ` David Marchand @ 2019-01-02 14:23 ` David Marchand 1 sibling, 0 replies; 6+ messages in thread From: David Marchand @ 2019-01-02 14:23 UTC (permalink / raw) To: Hari Kumar Vemula; +Cc: dev, reshma.pattan, Yigit, Ferruh, stable 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-02 14:24 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-20 10:00 [dpdk-dev] [PATCH] eal: fix core number validation Hari Kumar Vemula 2018-12-21 8:27 ` David Marchand 2018-12-21 9:19 ` Burakov, Anatoly 2018-12-21 9:28 ` David Marchand 2019-01-02 12:47 ` Vemula, Hari KumarX 2019-01-02 14:23 ` David Marchand
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).