* [PATCH] app/pdump: check lcore is not the maximum core
@ 2022-02-18 15:18 Reshma Pattan
2022-02-18 16:27 ` Stephen Hemminger
2022-02-21 10:19 ` [PATCH v2] " Reshma Pattan
0 siblings, 2 replies; 11+ messages in thread
From: Reshma Pattan @ 2022-02-18 15:18 UTC (permalink / raw)
To: dev; +Cc: stephen, ferruh.yigit, Reshma Pattan, vipin.varghese, stable
Check lcore id value is not the maximum core supported.
Using lcore id without this check might cause
out of bound access inside the rte_eal_wait_lcore.
Coverity issue: 375841
Fixes: b2854d5317e8 ("app/pdump: support multi-core capture")
Cc: vipin.varghese@intel.com
Cc: stable@dpdk.org
Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
app/pdump/main.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/app/pdump/main.c b/app/pdump/main.c
index 04a38e8911..686c27d965 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -931,11 +931,19 @@ dump_packets(void)
}
lcore_id = rte_get_next_lcore(lcore_id, 1, 0);
+ if (lcore_id == RTE_MAX_LCORE) {
+ printf("Invalid core %u for the packet capture!\n", lcore_id);
+ return;
+ }
for (i = 0; i < num_tuples; i++) {
rte_eal_remote_launch(dump_packets_core,
&pdump_t[i], lcore_id);
lcore_id = rte_get_next_lcore(lcore_id, 1, 0);
+ if (lcore_id == RTE_MAX_LCORE) {
+ printf("Invalid core %u for the capture for the tuple=%d!\n", lcore_id, i);
+ return;
+ }
if (rte_eal_wait_lcore(lcore_id) < 0)
rte_exit(EXIT_FAILURE, "failed to wait\n");
--
2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] app/pdump: check lcore is not the maximum core 2022-02-18 15:18 [PATCH] app/pdump: check lcore is not the maximum core Reshma Pattan @ 2022-02-18 16:27 ` Stephen Hemminger 2022-02-21 10:19 ` [PATCH v2] " Reshma Pattan 1 sibling, 0 replies; 11+ messages in thread From: Stephen Hemminger @ 2022-02-18 16:27 UTC (permalink / raw) To: Reshma Pattan; +Cc: dev, ferruh.yigit, vipin.varghese, stable On Fri, 18 Feb 2022 15:18:41 +0000 Reshma Pattan <reshma.pattan@intel.com> wrote: > lcore_id = rte_get_next_lcore(lcore_id, 1, 0); > + if (lcore_id == RTE_MAX_LCORE) { > + printf("Invalid core %u for the packet capture!\n", lcore_id); > + return; > + } Since nothing useful can be done, maybe rte_exit()? Or at least print to stderr and return error status. Also, if you write same code in two places, it should be a function. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] app/pdump: check lcore is not the maximum core 2022-02-18 15:18 [PATCH] app/pdump: check lcore is not the maximum core Reshma Pattan 2022-02-18 16:27 ` Stephen Hemminger @ 2022-02-21 10:19 ` Reshma Pattan 2022-02-22 11:02 ` [PATCH v3] " Reshma Pattan 1 sibling, 1 reply; 11+ messages in thread From: Reshma Pattan @ 2022-02-21 10:19 UTC (permalink / raw) To: dev; +Cc: stephen, ferruh.yigit, Reshma Pattan, vipin.varghese, stable Check lcore id value is not the maximum core supported. Using lcore id without this check might cause out of bound access inside the rte_eal_wait_lcore. Coverity issue: 375841 Fixes: b2854d5317e8 ("app/pdump: support multi-core capture") Cc: vipin.varghese@intel.com Cc: stable@dpdk.org Signed-off-by: Reshma Pattan <reshma.pattan@intel.com> --- app/pdump/main.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/pdump/main.c b/app/pdump/main.c index 04a38e8911..7677a5f8f5 100644 --- a/app/pdump/main.c +++ b/app/pdump/main.c @@ -930,12 +930,15 @@ dump_packets(void) return; } - lcore_id = rte_get_next_lcore(lcore_id, 1, 0); for (i = 0; i < num_tuples; i++) { + lcore_id = rte_get_next_lcore(lcore_id, 1, 0); + if (lcore_id == RTE_MAX_LCORE) + rte_exit(EXIT_FAILURE, + "Max core limit %u reached for packet capture", lcore_id); + rte_eal_remote_launch(dump_packets_core, &pdump_t[i], lcore_id); - lcore_id = rte_get_next_lcore(lcore_id, 1, 0); if (rte_eal_wait_lcore(lcore_id) < 0) rte_exit(EXIT_FAILURE, "failed to wait\n"); -- 2.27.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3] app/pdump: check lcore is not the maximum core 2022-02-21 10:19 ` [PATCH v2] " Reshma Pattan @ 2022-02-22 11:02 ` Reshma Pattan 2022-02-25 9:59 ` Pattan, Reshma ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Reshma Pattan @ 2022-02-22 11:02 UTC (permalink / raw) To: dev; +Cc: stephen, ferruh.yigit, Reshma Pattan, vipin.varghese, stable Check lcore id value is not the maximum core supported. Using lcore id without this check might cause out of bound access inside the rte_eal_wait_lcore. Coverity issue: 375841 Fixes: b2854d5317e8 ("app/pdump: support multi-core capture") Cc: vipin.varghese@intel.com Cc: stable@dpdk.org Signed-off-by: Reshma Pattan <reshma.pattan@intel.com> --- v3: add new function to get next core id and validate it. --- app/pdump/main.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/app/pdump/main.c b/app/pdump/main.c index 04a38e8911..e4e62811c9 100644 --- a/app/pdump/main.c +++ b/app/pdump/main.c @@ -900,6 +900,15 @@ dump_packets_core(void *arg) return 0; } +static inline void +get_next_core(uint32_t *lcore_id) +{ + *lcore_id = rte_get_next_lcore(*lcore_id, 1, 0); + if (*lcore_id == RTE_MAX_LCORE) + rte_exit(EXIT_FAILURE, + "Max core limit %u reached for packet capture", *lcore_id); +} + static inline void dump_packets(void) { @@ -930,12 +939,12 @@ dump_packets(void) return; } - lcore_id = rte_get_next_lcore(lcore_id, 1, 0); + get_next_core(&lcore_id); for (i = 0; i < num_tuples; i++) { rte_eal_remote_launch(dump_packets_core, &pdump_t[i], lcore_id); - lcore_id = rte_get_next_lcore(lcore_id, 1, 0); + get_next_core(&lcore_id); if (rte_eal_wait_lcore(lcore_id) < 0) rte_exit(EXIT_FAILURE, "failed to wait\n"); -- 2.25.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v3] app/pdump: check lcore is not the maximum core 2022-02-22 11:02 ` [PATCH v3] " Reshma Pattan @ 2022-02-25 9:59 ` Pattan, Reshma 2022-02-25 17:34 ` Stephen Hemminger 2022-02-28 9:58 ` [PATCH v4] " Reshma Pattan 2 siblings, 0 replies; 11+ messages in thread From: Pattan, Reshma @ 2022-02-25 9:59 UTC (permalink / raw) To: stephen; +Cc: Yigit, Ferruh, vipin.varghese, stable, dev > -----Original Message----- > From: Pattan, Reshma <reshma.pattan@intel.com> <snip> Hi Stephen, Can you take a look and ack the patch. Thanks, Reshma ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] app/pdump: check lcore is not the maximum core 2022-02-22 11:02 ` [PATCH v3] " Reshma Pattan 2022-02-25 9:59 ` Pattan, Reshma @ 2022-02-25 17:34 ` Stephen Hemminger 2022-02-28 9:58 ` [PATCH v4] " Reshma Pattan 2 siblings, 0 replies; 11+ messages in thread From: Stephen Hemminger @ 2022-02-25 17:34 UTC (permalink / raw) To: Reshma Pattan; +Cc: dev, ferruh.yigit, vipin.varghese, stable On Tue, 22 Feb 2022 11:02:24 +0000 Reshma Pattan <reshma.pattan@intel.com> wrote: > Check lcore id value is not the maximum core supported. > Using lcore id without this check might cause > out of bound access inside the rte_eal_wait_lcore. > > Coverity issue: 375841 > Fixes: b2854d5317e8 ("app/pdump: support multi-core capture") > Cc: vipin.varghese@intel.com > Cc: stable@dpdk.org > > Signed-off-by: Reshma Pattan <reshma.pattan@intel.com> > --- > v3: add new function to get next core id and validate it. > --- > app/pdump/main.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/app/pdump/main.c b/app/pdump/main.c > index 04a38e8911..e4e62811c9 100644 > --- a/app/pdump/main.c > +++ b/app/pdump/main.c > @@ -900,6 +900,15 @@ dump_packets_core(void *arg) > return 0; > } > > +static inline void > +get_next_core(uint32_t *lcore_id) > +{ > + *lcore_id = rte_get_next_lcore(*lcore_id, 1, 0); > + if (*lcore_id == RTE_MAX_LCORE) > + rte_exit(EXIT_FAILURE, > + "Max core limit %u reached for packet capture", *lcore_id); > +} > + This looks good, can I make a some suggestions. Since function either exits or returns a good value, I would prefer that it returned the lcore id. Avoiding call by reference if possible. Also, the lcore is currently typed unsigned int in rte_lcore.h therefore use that type? Inline is certainly unnecessary here, not critical path and compiler is likely to do it anyway. Also, DPDK uses lcore for most places (rather than core) so use that name. Result is: static unsigned int get_next_lcore(unsigned int lcore) { lcore = rte_get_next_lcore(lcore, 1, 0); if (lcore >= RTE_MAX_LCORE) "Max core limit %u reached for packet capture", lcore); return lcore; } > static inline void > dump_packets(void) > { > @@ -930,12 +939,12 @@ dump_packets(void) > return; > } > > - lcore_id = rte_get_next_lcore(lcore_id, 1, 0); > + get_next_core(&lcore_id); > > for (i = 0; i < num_tuples; i++) { > rte_eal_remote_launch(dump_packets_core, > &pdump_t[i], lcore_id); > - lcore_id = rte_get_next_lcore(lcore_id, 1, 0); > + get_next_core(&lcore_id); > > if (rte_eal_wait_lcore(lcore_id) < 0) > rte_exit(EXIT_FAILURE, "failed to wait\n"); ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4] app/pdump: check lcore is not the maximum core 2022-02-22 11:02 ` [PATCH v3] " Reshma Pattan 2022-02-25 9:59 ` Pattan, Reshma 2022-02-25 17:34 ` Stephen Hemminger @ 2022-02-28 9:58 ` Reshma Pattan 2022-02-28 17:09 ` Stephen Hemminger 2022-03-08 13:47 ` Mcnamara, John 2 siblings, 2 replies; 11+ messages in thread From: Reshma Pattan @ 2022-02-28 9:58 UTC (permalink / raw) To: dev; +Cc: stephen, ferruh.yigit, Reshma Pattan, vipin.varghese, stable Check lcore id value is not the maximum core supported. Using lcore id without this check might cause out of bound access inside the rte_eal_wait_lcore. Coverity issue: 375841 Fixes: b2854d5317e8 ("app/pdump: support multi-core capture") Cc: vipin.varghese@intel.com Cc: stable@dpdk.org Signed-off-by: Reshma Pattan <reshma.pattan@intel.com> --- v4: Remove inline of new function Change lcore type as unsigned int return lcore from the function --- app/pdump/main.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/app/pdump/main.c b/app/pdump/main.c index 04a38e8911..96fa76f8da 100644 --- a/app/pdump/main.c +++ b/app/pdump/main.c @@ -900,11 +900,21 @@ dump_packets_core(void *arg) return 0; } +static unsigned int +get_next_core(unsigned int lcore) +{ + lcore = rte_get_next_lcore(lcore, 1, 0); + if (lcore == RTE_MAX_LCORE) + rte_exit(EXIT_FAILURE, + "Max core limit %u reached for packet capture", lcore); + return lcore; +} + static inline void dump_packets(void) { int i; - uint32_t lcore_id = 0; + unsigned int lcore_id = 0; if (!multiple_core_capture) { printf(" core (%u), capture for (%d) tuples\n", @@ -930,12 +940,12 @@ dump_packets(void) return; } - lcore_id = rte_get_next_lcore(lcore_id, 1, 0); + lcore_id = get_next_core(lcore_id); for (i = 0; i < num_tuples; i++) { rte_eal_remote_launch(dump_packets_core, &pdump_t[i], lcore_id); - lcore_id = rte_get_next_lcore(lcore_id, 1, 0); + lcore_id = get_next_core(lcore_id); if (rte_eal_wait_lcore(lcore_id) < 0) rte_exit(EXIT_FAILURE, "failed to wait\n"); -- 2.25.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4] app/pdump: check lcore is not the maximum core 2022-02-28 9:58 ` [PATCH v4] " Reshma Pattan @ 2022-02-28 17:09 ` Stephen Hemminger 2022-03-08 13:47 ` Mcnamara, John 1 sibling, 0 replies; 11+ messages in thread From: Stephen Hemminger @ 2022-02-28 17:09 UTC (permalink / raw) To: Reshma Pattan; +Cc: dev, ferruh.yigit, vipin.varghese, stable On Mon, 28 Feb 2022 09:58:56 +0000 Reshma Pattan <reshma.pattan@intel.com> wrote: > Check lcore id value is not the maximum core supported. > Using lcore id without this check might cause > out of bound access inside the rte_eal_wait_lcore. > > Coverity issue: 375841 > Fixes: b2854d5317e8 ("app/pdump: support multi-core capture") > Cc: vipin.varghese@intel.com > Cc: stable@dpdk.org > > Signed-off-by: Reshma Pattan <reshma.pattan@intel.com> Great thanks. Acked-by: Stephen Hemminger <stephen@networkplumber.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v4] app/pdump: check lcore is not the maximum core 2022-02-28 9:58 ` [PATCH v4] " Reshma Pattan 2022-02-28 17:09 ` Stephen Hemminger @ 2022-03-08 13:47 ` Mcnamara, John 2022-03-08 14:27 ` Thomas Monjalon 1 sibling, 1 reply; 11+ messages in thread From: Mcnamara, John @ 2022-03-08 13:47 UTC (permalink / raw) To: Pattan, Reshma, dev, thomas Cc: stephen, Yigit, Ferruh, Pattan, Reshma, vipin.varghese, stable Thomas, could you include this in RC3 if possible. > -----Original Message----- > From: Reshma Pattan <reshma.pattan@intel.com> > Sent: Monday, February 28, 2022 9:59 AM > To: dev@dpdk.org > Cc: stephen@networkplumber.org; Yigit, Ferruh <ferruh.yigit@intel.com>; > Pattan, Reshma <reshma.pattan@intel.com>; vipin.varghese@intel.com; > stable@dpdk.org > Subject: [PATCH v4] app/pdump: check lcore is not the maximum core > > Check lcore id value is not the maximum core supported. > Using lcore id without this check might cause out of bound access inside > the rte_eal_wait_lcore. > > Coverity issue: 375841 > Fixes: b2854d5317e8 ("app/pdump: support multi-core capture") > Cc: vipin.varghese@intel.com > Cc: stable@dpdk.org > > Signed-off-by: Reshma Pattan <reshma.pattan@intel.com> > --- > v4: Remove inline of new function > Change lcore type as unsigned int > return lcore from the function > --- > app/pdump/main.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/app/pdump/main.c b/app/pdump/main.c index > 04a38e8911..96fa76f8da 100644 > --- a/app/pdump/main.c > +++ b/app/pdump/main.c > @@ -900,11 +900,21 @@ dump_packets_core(void *arg) > return 0; > } > > +static unsigned int > +get_next_core(unsigned int lcore) > +{ > + lcore = rte_get_next_lcore(lcore, 1, 0); > + if (lcore == RTE_MAX_LCORE) > + rte_exit(EXIT_FAILURE, > + "Max core limit %u reached for packet capture", > lcore); > + return lcore; > +} > + > static inline void > dump_packets(void) > { > int i; > - uint32_t lcore_id = 0; > + unsigned int lcore_id = 0; > > if (!multiple_core_capture) { > printf(" core (%u), capture for (%d) tuples\n", @@ -930,12 > +940,12 @@ dump_packets(void) > return; > } > > - lcore_id = rte_get_next_lcore(lcore_id, 1, 0); > + lcore_id = get_next_core(lcore_id); > > for (i = 0; i < num_tuples; i++) { > rte_eal_remote_launch(dump_packets_core, > &pdump_t[i], lcore_id); > - lcore_id = rte_get_next_lcore(lcore_id, 1, 0); > + lcore_id = get_next_core(lcore_id); > > if (rte_eal_wait_lcore(lcore_id) < 0) > rte_exit(EXIT_FAILURE, "failed to wait\n"); > -- > 2.25.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4] app/pdump: check lcore is not the maximum core 2022-03-08 13:47 ` Mcnamara, John @ 2022-03-08 14:27 ` Thomas Monjalon 2022-03-08 15:03 ` Mcnamara, John 0 siblings, 1 reply; 11+ messages in thread From: Thomas Monjalon @ 2022-03-08 14:27 UTC (permalink / raw) To: Mcnamara, John Cc: Pattan, Reshma, dev, stephen, Yigit, Ferruh, Pattan, Reshma, vipin.varghese, stable 08/03/2022 14:47, Mcnamara, John: > Thomas, could you include this in RC3 if possible. I did already yesterday: https://git.dpdk.org/dpdk/commit/?id=3ee04ebc91 Looks like I forgot to send an email. Applied, thanks :) ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v4] app/pdump: check lcore is not the maximum core 2022-03-08 14:27 ` Thomas Monjalon @ 2022-03-08 15:03 ` Mcnamara, John 0 siblings, 0 replies; 11+ messages in thread From: Mcnamara, John @ 2022-03-08 15:03 UTC (permalink / raw) To: Thomas Monjalon Cc: Pattan, Reshma, dev, stephen, Yigit, Ferruh, Pattan, Reshma, vipin.varghese, stable > -----Original Message----- > From: Thomas Monjalon <thomas@monjalon.net> > Subject: Re: [PATCH v4] app/pdump: check lcore is not the maximum core > > 08/03/2022 14:47, Mcnamara, John: > > Thomas, could you include this in RC3 if possible. > > I did already yesterday: > https://git.dpdk.org/dpdk/commit/?id=3ee04ebc91 > > Looks like I forgot to send an email. > > Applied, thanks :) Thank you. :-) ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-03-08 15:03 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-18 15:18 [PATCH] app/pdump: check lcore is not the maximum core Reshma Pattan 2022-02-18 16:27 ` Stephen Hemminger 2022-02-21 10:19 ` [PATCH v2] " Reshma Pattan 2022-02-22 11:02 ` [PATCH v3] " Reshma Pattan 2022-02-25 9:59 ` Pattan, Reshma 2022-02-25 17:34 ` Stephen Hemminger 2022-02-28 9:58 ` [PATCH v4] " Reshma Pattan 2022-02-28 17:09 ` Stephen Hemminger 2022-03-08 13:47 ` Mcnamara, John 2022-03-08 14:27 ` Thomas Monjalon 2022-03-08 15:03 ` Mcnamara, John
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).