DPDK patches and discussions
 help / color / mirror / Atom feed
* [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).