patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH] drivers: fix to replace strcat with strncat
@ 2019-01-14  6:04 Chaitanya Babu Talluri
  2019-01-14 13:29 ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
                   ` (3 more replies)
  0 siblings, 4 replies; 28+ messages in thread
From: Chaitanya Babu Talluri @ 2019-01-14  6:04 UTC (permalink / raw)
  To: dev
  Cc: reshma.pattan, rmody, shshaikh, beilei.xing, qi.z.zhang,
	alejandro.lucero, pablo.de.lara.guarch, declan.doherty,
	Chaitanya Babu Talluri, stable

Strcat does not check the destination length and there might be
chances of string overflow so insted of strcat, strncat is used.

Fixes: 540a211084 ("bnx2x: driver core")
Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
Cc: stable@dpdk.org

Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
---
 drivers/net/bnx2x/bnx2x.c                  | 6 ++++--
 drivers/net/i40e/i40e_ethdev.c             | 6 ++++--
 drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c | 8 +++++---
 test/test/test_cryptodev.c                 | 3 ++-
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index 4c775c163..0e1e6447a 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -11734,13 +11734,15 @@ static const char *get_bnx2x_flags(uint32_t flags)
 
 	for (i = 0; i < 5; i++)
 		if (flags & (1 << i)) {
-			strcat(flag_str, flag[i]);
+			strncat(flag_str, flag[i],
+				BNX2X_INFO_STR_MAX - strlen(flag_str) - 1);
 			flags ^= (1 << i);
 		}
 	if (flags) {
 		static char unknown[BNX2X_INFO_STR_MAX];
 		snprintf(unknown, 32, "Unknown flag mask %x", flags);
-		strcat(flag_str, unknown);
+		strncat(flag_str, unknown,
+				BNX2X_INFO_STR_MAX  - strlen(flag_str) - 1);
 	}
 	return flag_str;
 }
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 8dc1a4af8..56867ff84 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -12175,8 +12175,10 @@ i40e_update_customized_pctype(struct rte_eth_dev *dev, uint8_t *pkg,
 			for (n = 0; n < proto_num; n++) {
 				if (proto[n].proto_id != proto_id)
 					continue;
-				strcat(name, proto[n].name);
-				strcat(name, "_");
+				strncat(name, proto[n].name,
+					sizeof(name) - strlen(name) - 1);
+				strncat(name, "_",
+					sizeof(name) - strlen(name) - 1);
 				break;
 			}
 		}
diff --git a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
index 39bd48a83..2c72f773a 100644
--- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
+++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
@@ -73,6 +73,8 @@
 #define NFP_PCIE_CPP_BAR_PCIETOCPPEXPBAR(bar, slot) \
 	(((bar) * 8 + (slot)) * 4)
 
+#define LOCKFILE_HOME_PATH 256
+
 /*
  * Define to enable a bit more verbose debug output.
  * Set to 1 to enable a bit more verbose debug output.
@@ -685,11 +687,11 @@ nfp_acquire_secondary_process_lock(struct nfp_pcie_user *desc)
 	 * driver is used because that implies root user.
 	 */
 	home_path = getenv("HOME");
-	lockfile = calloc(strlen(home_path) + strlen(lockname) + 1,
+	lockfile = calloc(LOCKFILE_HOME_PATH + strlen(lockname) + 1,
 			  sizeof(char));
 
-	strcat(lockfile, home_path);
-	strcat(lockfile, "/.lock_nfp_secondary");
+	strncat(lockfile, home_path, LOCKFILE_HOME_PATH);
+	strncat(lockfile, lockname, strlen(lockfile));
 	desc->secondary_lock = open(lockfile, O_RDWR | O_CREAT | O_NONBLOCK,
 				    0666);
 	if (desc->secondary_lock < 0) {
diff --git a/test/test/test_cryptodev.c b/test/test/test_cryptodev.c
index 84065eb49..a979603b9 100644
--- a/test/test/test_cryptodev.c
+++ b/test/test/test_cryptodev.c
@@ -374,7 +374,8 @@ testsuite_setup(void)
 			snprintf(vdev_args, sizeof(vdev_args),
 					"%s%d", temp_str, i);
 			strcpy(temp_str, vdev_args);
-			strcat(temp_str, ";");
+			strncat(temp_str, ";",
+					VDEV_ARGS_SIZE - strlen(temp_str) - 1);
 			slave_core_count++;
 			socket_id = lcore_config[i].socket_id;
 		}
-- 
2.17.2

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] drivers: fix to replace strcat with strncat
  2019-01-14  6:04 [dpdk-stable] [PATCH] drivers: fix to replace strcat with strncat Chaitanya Babu Talluri
@ 2019-01-14 13:29 ` Ferruh Yigit
  2019-01-14 16:24   ` Stephen Hemminger
  2019-01-14 14:21 ` Bruce Richardson
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 28+ messages in thread
From: Ferruh Yigit @ 2019-01-14 13:29 UTC (permalink / raw)
  To: Chaitanya Babu Talluri, dev, alejandro.lucero
  Cc: reshma.pattan, rmody, shshaikh, beilei.xing, qi.z.zhang,
	pablo.de.lara.guarch, declan.doherty, stable

On 1/14/2019 6:04 AM, Chaitanya Babu Talluri wrote:
> Strcat does not check the destination length and there might be
> chances of string overflow so insted of strcat, strncat is used.
> 
> Fixes: 540a211084 ("bnx2x: driver core")
> Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
> Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
> Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>

<...>

> @@ -685,11 +687,11 @@ nfp_acquire_secondary_process_lock(struct nfp_pcie_user *desc)
>  	 * driver is used because that implies root user.
>  	 */
>  	home_path = getenv("HOME");
> -	lockfile = calloc(strlen(home_path) + strlen(lockname) + 1,
> +	lockfile = calloc(LOCKFILE_HOME_PATH + strlen(lockname) + 1,
>  			  sizeof(char));
>  
> -	strcat(lockfile, home_path);
> -	strcat(lockfile, "/.lock_nfp_secondary");
> +	strncat(lockfile, home_path, LOCKFILE_HOME_PATH);
> +	strncat(lockfile, lockname, strlen(lockfile));

I guess this need to be 'LOCKFILE_HOME_PATH - strlen(lockfile) - 1' instead.
But also this can be implemented as 'snprintf()'

Since 'lockfile' allocated dynamically based on sizes of existing strings, using
'lockname' instead of "/.lock_nfp_secondary" will show that there won't be any
overflow but tools still may be complaining about 'strcat' usage.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] drivers: fix to replace strcat with strncat
  2019-01-14  6:04 [dpdk-stable] [PATCH] drivers: fix to replace strcat with strncat Chaitanya Babu Talluri
  2019-01-14 13:29 ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
@ 2019-01-14 14:21 ` Bruce Richardson
  2019-01-15  1:53   ` Thomas Monjalon
  2019-01-18 15:11 ` [dpdk-stable] [PATCH v2] " Chaitanya Babu Talluri
  2019-01-18 15:23 ` Chaitanya Babu Talluri
  3 siblings, 1 reply; 28+ messages in thread
From: Bruce Richardson @ 2019-01-14 14:21 UTC (permalink / raw)
  To: Chaitanya Babu Talluri
  Cc: dev, reshma.pattan, rmody, shshaikh, beilei.xing, qi.z.zhang,
	alejandro.lucero, pablo.de.lara.guarch, declan.doherty, stable

On Mon, Jan 14, 2019 at 06:04:35AM +0000, Chaitanya Babu Talluri wrote:
> Strcat does not check the destination length and there might be
> chances of string overflow so insted of strcat, strncat is used.
> 
> Fixes: 540a211084 ("bnx2x: driver core")
> Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
> Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
> Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
> ---
>  drivers/net/bnx2x/bnx2x.c                  | 6 ++++--
>  drivers/net/i40e/i40e_ethdev.c             | 6 ++++--
>  drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c | 8 +++++---
>  test/test/test_cryptodev.c                 | 3 ++-
>  4 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
> index 4c775c163..0e1e6447a 100644
> --- a/drivers/net/bnx2x/bnx2x.c
> +++ b/drivers/net/bnx2x/bnx2x.c
> @@ -11734,13 +11734,15 @@ static const char *get_bnx2x_flags(uint32_t flags)
>  
>  	for (i = 0; i < 5; i++)
>  		if (flags & (1 << i)) {
> -			strcat(flag_str, flag[i]);
> +			strncat(flag_str, flag[i],
> +				BNX2X_INFO_STR_MAX - strlen(flag_str) - 1);
>  			flags ^= (1 << i);
>  		}
>  	if (flags) {
>  		static char unknown[BNX2X_INFO_STR_MAX];
>  		snprintf(unknown, 32, "Unknown flag mask %x", flags);
> -		strcat(flag_str, unknown);
> +		strncat(flag_str, unknown,
> +				BNX2X_INFO_STR_MAX  - strlen(flag_str) - 1);
>  	}
>  	return flag_str;
>  }

While I believe this is correct, this (and the changes below) would be a
lot neater using strlcat function. We should perhaps look to add strlcat
alongside strlcpy for DPDK use.

/Bruce

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] drivers: fix to replace strcat with strncat
  2019-01-14 13:29 ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
@ 2019-01-14 16:24   ` Stephen Hemminger
  2019-01-17 16:44     ` Thomas Monjalon
  0 siblings, 1 reply; 28+ messages in thread
From: Stephen Hemminger @ 2019-01-14 16:24 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Chaitanya Babu Talluri, dev, alejandro.lucero, reshma.pattan,
	rmody, shshaikh, beilei.xing, qi.z.zhang, pablo.de.lara.guarch,
	declan.doherty, stable

On Mon, 14 Jan 2019 13:29:38 +0000
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 1/14/2019 6:04 AM, Chaitanya Babu Talluri wrote:
> > Strcat does not check the destination length and there might be
> > chances of string overflow so insted of strcat, strncat is used.
> > 
> > Fixes: 540a211084 ("bnx2x: driver core")
> > Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
> > Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
> > Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>  
> 
> <...>
> 
> > @@ -685,11 +687,11 @@ nfp_acquire_secondary_process_lock(struct nfp_pcie_user *desc)
> >  	 * driver is used because that implies root user.
> >  	 */
> >  	home_path = getenv("HOME");
> > -	lockfile = calloc(strlen(home_path) + strlen(lockname) + 1,
> > +	lockfile = calloc(LOCKFILE_HOME_PATH + strlen(lockname) + 1,
> >  			  sizeof(char));
> >  
> > -	strcat(lockfile, home_path);
> > -	strcat(lockfile, "/.lock_nfp_secondary");
> > +	strncat(lockfile, home_path, LOCKFILE_HOME_PATH);
> > +	strncat(lockfile, lockname, strlen(lockfile));  
> 
> I guess this need to be 'LOCKFILE_HOME_PATH - strlen(lockfile) - 1' instead.
> But also this can be implemented as 'snprintf()'
> 
> Since 'lockfile' allocated dynamically based on sizes of existing strings, using
> 'lockname' instead of "/.lock_nfp_secondary" will show that there won't be any
> overflow but tools still may be complaining about 'strcat' usage.
> 
> 

Why not use vasprintf() instead of doing manual construction?

	

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] drivers: fix to replace strcat with strncat
  2019-01-14 14:21 ` Bruce Richardson
@ 2019-01-15  1:53   ` Thomas Monjalon
  0 siblings, 0 replies; 28+ messages in thread
From: Thomas Monjalon @ 2019-01-15  1:53 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: dev, Chaitanya Babu Talluri, reshma.pattan, rmody, shshaikh,
	beilei.xing, qi.z.zhang, alejandro.lucero, pablo.de.lara.guarch,
	declan.doherty, stable

14/01/2019 15:21, Bruce Richardson:
> On Mon, Jan 14, 2019 at 06:04:35AM +0000, Chaitanya Babu Talluri wrote:
> > Strcat does not check the destination length and there might be
> > chances of string overflow so insted of strcat, strncat is used.
[...]
> 
> While I believe this is correct, this (and the changes below) would be a
> lot neater using strlcat function. We should perhaps look to add strlcat
> alongside strlcpy for DPDK use.

Yes it looks reasonnable.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] drivers: fix to replace strcat with strncat
  2019-01-14 16:24   ` Stephen Hemminger
@ 2019-01-17 16:44     ` Thomas Monjalon
  0 siblings, 0 replies; 28+ messages in thread
From: Thomas Monjalon @ 2019-01-17 16:44 UTC (permalink / raw)
  To: Chaitanya Babu Talluri
  Cc: dev, Stephen Hemminger, Ferruh Yigit, alejandro.lucero,
	reshma.pattan, rmody, shshaikh, beilei.xing, qi.z.zhang,
	pablo.de.lara.guarch, declan.doherty, stable

14/01/2019 17:24, Stephen Hemminger:
> On Mon, 14 Jan 2019 13:29:38 +0000
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> > On 1/14/2019 6:04 AM, Chaitanya Babu Talluri wrote:
> > > Strcat does not check the destination length and there might be
> > > chances of string overflow so insted of strcat, strncat is used.
> > > 
> > > Fixes: 540a211084 ("bnx2x: driver core")
> > > Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
> > > Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
> > > Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
> > > Cc: stable@dpdk.org
> > > 
> > > Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>  
> > 
> > <...>
> > 
> > > @@ -685,11 +687,11 @@ nfp_acquire_secondary_process_lock(struct nfp_pcie_user *desc)
> > >  	 * driver is used because that implies root user.
> > >  	 */
> > >  	home_path = getenv("HOME");
> > > -	lockfile = calloc(strlen(home_path) + strlen(lockname) + 1,
> > > +	lockfile = calloc(LOCKFILE_HOME_PATH + strlen(lockname) + 1,
> > >  			  sizeof(char));
> > >  
> > > -	strcat(lockfile, home_path);
> > > -	strcat(lockfile, "/.lock_nfp_secondary");
> > > +	strncat(lockfile, home_path, LOCKFILE_HOME_PATH);
> > > +	strncat(lockfile, lockname, strlen(lockfile));  
> > 
> > I guess this need to be 'LOCKFILE_HOME_PATH - strlen(lockfile) - 1' instead.
> > But also this can be implemented as 'snprintf()'
> > 
> > Since 'lockfile' allocated dynamically based on sizes of existing strings, using
> > 'lockname' instead of "/.lock_nfp_secondary" will show that there won't be any
> > overflow but tools still may be complaining about 'strcat' usage.
> > 
> > 
> 
> Why not use vasprintf() instead of doing manual construction?

Any update please?

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [dpdk-stable] [PATCH v2] drivers: fix to replace strcat with strncat
  2019-01-14  6:04 [dpdk-stable] [PATCH] drivers: fix to replace strcat with strncat Chaitanya Babu Talluri
  2019-01-14 13:29 ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
  2019-01-14 14:21 ` Bruce Richardson
@ 2019-01-18 15:11 ` Chaitanya Babu Talluri
  2019-01-18 15:23 ` Chaitanya Babu Talluri
  3 siblings, 0 replies; 28+ messages in thread
From: Chaitanya Babu Talluri @ 2019-01-18 15:11 UTC (permalink / raw)
  To: dev; +Cc: reshma.pattan

Strcat does not check the destination length and there might be
chances of string overflow so insted of strcat, strncat is used.

Fixes: 540a211084 ("bnx2x: driver core")
Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
Cc: stable@dpdk.org

Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
---
v2: Instead of strncat, used snprintf.
---
 drivers/net/bnx2x/bnx2x.c                  | 6 ++++--
 drivers/net/i40e/i40e_ethdev.c             | 6 ++++--
 drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c | 8 +++++---
 test/test/test_cryptodev.c                 | 3 ++-
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index 4c775c163..0e1e6447a 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -11734,13 +11734,15 @@ static const char *get_bnx2x_flags(uint32_t flags)
 
 	for (i = 0; i < 5; i++)
 		if (flags & (1 << i)) {
-			strcat(flag_str, flag[i]);
+			strncat(flag_str, flag[i],
+				BNX2X_INFO_STR_MAX - strlen(flag_str) - 1);
 			flags ^= (1 << i);
 		}
 	if (flags) {
 		static char unknown[BNX2X_INFO_STR_MAX];
 		snprintf(unknown, 32, "Unknown flag mask %x", flags);
-		strcat(flag_str, unknown);
+		strncat(flag_str, unknown,
+				BNX2X_INFO_STR_MAX  - strlen(flag_str) - 1);
 	}
 	return flag_str;
 }
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 8dc1a4af8..56867ff84 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -12175,8 +12175,10 @@ i40e_update_customized_pctype(struct rte_eth_dev *dev, uint8_t *pkg,
 			for (n = 0; n < proto_num; n++) {
 				if (proto[n].proto_id != proto_id)
 					continue;
-				strcat(name, proto[n].name);
-				strcat(name, "_");
+				strncat(name, proto[n].name,
+					sizeof(name) - strlen(name) - 1);
+				strncat(name, "_",
+					sizeof(name) - strlen(name) - 1);
 				break;
 			}
 		}
diff --git a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
index 39bd48a83..a9c727185 100644
--- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
+++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
@@ -73,6 +73,8 @@
 #define NFP_PCIE_CPP_BAR_PCIETOCPPEXPBAR(bar, slot) \
 	(((bar) * 8 + (slot)) * 4)
 
+#define LOCKFILE_HOME_PATH 256
+
 /*
  * Define to enable a bit more verbose debug output.
  * Set to 1 to enable a bit more verbose debug output.
@@ -685,11 +687,11 @@ nfp_acquire_secondary_process_lock(struct nfp_pcie_user *desc)
 	 * driver is used because that implies root user.
 	 */
 	home_path = getenv("HOME");
-	lockfile = calloc(strlen(home_path) + strlen(lockname) + 1,
+	lockfile = calloc(LOCKFILE_HOME_PATH + strlen(lockname) + 1,
 			  sizeof(char));
 
-	strcat(lockfile, home_path);
-	strcat(lockfile, "/.lock_nfp_secondary");
+	snprintf(lockfile, LOCKFILE_HOME_PATH + strlen(lockname),
+			"%s%s", home_path, lockname);
 	desc->secondary_lock = open(lockfile, O_RDWR | O_CREAT | O_NONBLOCK,
 				    0666);
 	if (desc->secondary_lock < 0) {
diff --git a/test/test/test_cryptodev.c b/test/test/test_cryptodev.c
index 84065eb49..a979603b9 100644
--- a/test/test/test_cryptodev.c
+++ b/test/test/test_cryptodev.c
@@ -374,7 +374,8 @@ testsuite_setup(void)
 			snprintf(vdev_args, sizeof(vdev_args),
 					"%s%d", temp_str, i);
 			strcpy(temp_str, vdev_args);
-			strcat(temp_str, ";");
+			strncat(temp_str, ";",
+					VDEV_ARGS_SIZE - strlen(temp_str) - 1);
 			slave_core_count++;
 			socket_id = lcore_config[i].socket_id;
 		}
-- 
2.17.2

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [dpdk-stable] [PATCH v2] drivers: fix to replace strcat with strncat
  2019-01-14  6:04 [dpdk-stable] [PATCH] drivers: fix to replace strcat with strncat Chaitanya Babu Talluri
                   ` (2 preceding siblings ...)
  2019-01-18 15:11 ` [dpdk-stable] [PATCH v2] " Chaitanya Babu Talluri
@ 2019-01-18 15:23 ` Chaitanya Babu Talluri
  2019-01-21 10:43   ` [dpdk-stable] [dpdk-dev] " Parthasarathy, JananeeX M
  2019-02-27  6:02   ` [dpdk-stable] [PATCH v3] drivers: fix to replace strcat with strlcat Chaitanya Babu Talluri
  3 siblings, 2 replies; 28+ messages in thread
From: Chaitanya Babu Talluri @ 2019-01-18 15:23 UTC (permalink / raw)
  To: dev
  Cc: rmody, reshma.pattan, shshaikh, beilei.xing, qi.z.zhang,
	alejandro.lucero, pablo.de.lara.guarch, declan.doherty,
	Chaitanya Babu Talluri, stable

Strcat does not check the destination length and there might be
chances of string overflow so insted of strcat, strncat is used.

Fixes: 540a211084 ("bnx2x: driver core")
Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
Cc: stable@dpdk.org

Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
---
v2: Instead of strncat, used snprintf.
---
 drivers/net/bnx2x/bnx2x.c                  | 6 ++++--
 drivers/net/i40e/i40e_ethdev.c             | 6 ++++--
 drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c | 8 +++++---
 test/test/test_cryptodev.c                 | 3 ++-
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index 4c775c163..0e1e6447a 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -11734,13 +11734,15 @@ static const char *get_bnx2x_flags(uint32_t flags)
 
 	for (i = 0; i < 5; i++)
 		if (flags & (1 << i)) {
-			strcat(flag_str, flag[i]);
+			strncat(flag_str, flag[i],
+				BNX2X_INFO_STR_MAX - strlen(flag_str) - 1);
 			flags ^= (1 << i);
 		}
 	if (flags) {
 		static char unknown[BNX2X_INFO_STR_MAX];
 		snprintf(unknown, 32, "Unknown flag mask %x", flags);
-		strcat(flag_str, unknown);
+		strncat(flag_str, unknown,
+				BNX2X_INFO_STR_MAX  - strlen(flag_str) - 1);
 	}
 	return flag_str;
 }
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 8dc1a4af8..56867ff84 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -12175,8 +12175,10 @@ i40e_update_customized_pctype(struct rte_eth_dev *dev, uint8_t *pkg,
 			for (n = 0; n < proto_num; n++) {
 				if (proto[n].proto_id != proto_id)
 					continue;
-				strcat(name, proto[n].name);
-				strcat(name, "_");
+				strncat(name, proto[n].name,
+					sizeof(name) - strlen(name) - 1);
+				strncat(name, "_",
+					sizeof(name) - strlen(name) - 1);
 				break;
 			}
 		}
diff --git a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
index 39bd48a83..a9c727185 100644
--- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
+++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
@@ -73,6 +73,8 @@
 #define NFP_PCIE_CPP_BAR_PCIETOCPPEXPBAR(bar, slot) \
 	(((bar) * 8 + (slot)) * 4)
 
+#define LOCKFILE_HOME_PATH 256
+
 /*
  * Define to enable a bit more verbose debug output.
  * Set to 1 to enable a bit more verbose debug output.
@@ -685,11 +687,11 @@ nfp_acquire_secondary_process_lock(struct nfp_pcie_user *desc)
 	 * driver is used because that implies root user.
 	 */
 	home_path = getenv("HOME");
-	lockfile = calloc(strlen(home_path) + strlen(lockname) + 1,
+	lockfile = calloc(LOCKFILE_HOME_PATH + strlen(lockname) + 1,
 			  sizeof(char));
 
-	strcat(lockfile, home_path);
-	strcat(lockfile, "/.lock_nfp_secondary");
+	snprintf(lockfile, LOCKFILE_HOME_PATH + strlen(lockname),
+			"%s%s", home_path, lockname);
 	desc->secondary_lock = open(lockfile, O_RDWR | O_CREAT | O_NONBLOCK,
 				    0666);
 	if (desc->secondary_lock < 0) {
diff --git a/test/test/test_cryptodev.c b/test/test/test_cryptodev.c
index 84065eb49..a979603b9 100644
--- a/test/test/test_cryptodev.c
+++ b/test/test/test_cryptodev.c
@@ -374,7 +374,8 @@ testsuite_setup(void)
 			snprintf(vdev_args, sizeof(vdev_args),
 					"%s%d", temp_str, i);
 			strcpy(temp_str, vdev_args);
-			strcat(temp_str, ";");
+			strncat(temp_str, ";",
+					VDEV_ARGS_SIZE - strlen(temp_str) - 1);
 			slave_core_count++;
 			socket_id = lcore_config[i].socket_id;
 		}
-- 
2.17.2

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2] drivers: fix to replace strcat with strncat
  2019-01-18 15:23 ` Chaitanya Babu Talluri
@ 2019-01-21 10:43   ` Parthasarathy, JananeeX M
  2019-02-07 11:56     ` Ferruh Yigit
  2019-02-27  6:02   ` [dpdk-stable] [PATCH v3] drivers: fix to replace strcat with strlcat Chaitanya Babu Talluri
  1 sibling, 1 reply; 28+ messages in thread
From: Parthasarathy, JananeeX M @ 2019-01-21 10:43 UTC (permalink / raw)
  To: dev
  Cc: rmody, Pattan, Reshma, shshaikh, Xing, Beilei, Zhang, Qi Z,
	alejandro.lucero, De Lara Guarch, Pablo, Doherty, Declan,
	Chaitanya Babu, TalluriX, stable, Yigit, Ferruh



>-----Original Message-----
>From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Chaitanya Babu Talluri
>Sent: Friday, January 18, 2019 8:54 PM
>To: dev@dpdk.org
>Cc: rmody@marvell.com; Pattan, Reshma <reshma.pattan@intel.com>;
>shshaikh@marvell.com; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
><qi.z.zhang@intel.com>; alejandro.lucero@netronome.com; De Lara Guarch,
>Pablo <pablo.de.lara.guarch@intel.com>; Doherty, Declan
><declan.doherty@intel.com>; Chaitanya Babu, TalluriX
><tallurix.chaitanya.babu@intel.com>; stable@dpdk.org
>Subject: [dpdk-dev] [PATCH v2] drivers: fix to replace strcat with strncat
>
>Strcat does not check the destination length and there might be chances of
>string overflow so insted of strcat, strncat is used.
>
>Fixes: 540a211084 ("bnx2x: driver core")
>Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
>Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
>Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
>Cc: stable@dpdk.org
>
>Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
>---
>v2: Instead of strncat, used snprintf.
>---
> drivers/net/bnx2x/bnx2x.c                  | 6 ++++--
> drivers/net/i40e/i40e_ethdev.c             | 6 ++++--
> drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c | 8 +++++---
> test/test/test_cryptodev.c                 | 3 ++-
> 4 files changed, 15 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c index
>4c775c163..0e1e6447a 100644
>--- a/drivers/net/bnx2x/bnx2x.c
>+++ b/drivers/net/bnx2x/bnx2x.c
>@@ -11734,13 +11734,15 @@ static const char *get_bnx2x_flags(uint32_t
>flags)
>
> 	for (i = 0; i < 5; i++)
> 		if (flags & (1 << i)) {
>-			strcat(flag_str, flag[i]);
>+			strncat(flag_str, flag[i],
>+				BNX2X_INFO_STR_MAX - strlen(flag_str) - 1);
> 			flags ^= (1 << i);
> 		}
> 	if (flags) {
> 		static char unknown[BNX2X_INFO_STR_MAX];
> 		snprintf(unknown, 32, "Unknown flag mask %x", flags);
>-		strcat(flag_str, unknown);
>+		strncat(flag_str, unknown,
>+				BNX2X_INFO_STR_MAX  - strlen(flag_str) - 1);
> 	}
> 	return flag_str;
> }
>diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
>index 8dc1a4af8..56867ff84 100644
>--- a/drivers/net/i40e/i40e_ethdev.c
>+++ b/drivers/net/i40e/i40e_ethdev.c
>@@ -12175,8 +12175,10 @@ i40e_update_customized_pctype(struct
>rte_eth_dev *dev, uint8_t *pkg,
> 			for (n = 0; n < proto_num; n++) {
> 				if (proto[n].proto_id != proto_id)
> 					continue;
>-				strcat(name, proto[n].name);
>-				strcat(name, "_");
>+				strncat(name, proto[n].name,
>+					sizeof(name) - strlen(name) - 1);
>+				strncat(name, "_",
>+					sizeof(name) - strlen(name) - 1);
> 				break;
> 			}
> 		}
>diff --git a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
>b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
>index 39bd48a83..a9c727185 100644
>--- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
>+++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
>@@ -73,6 +73,8 @@
> #define NFP_PCIE_CPP_BAR_PCIETOCPPEXPBAR(bar, slot) \
> 	(((bar) * 8 + (slot)) * 4)
>
>+#define LOCKFILE_HOME_PATH 256
>+
> /*
>  * Define to enable a bit more verbose debug output.
>  * Set to 1 to enable a bit more verbose debug output.
>@@ -685,11 +687,11 @@ nfp_acquire_secondary_process_lock(struct
>nfp_pcie_user *desc)
> 	 * driver is used because that implies root user.
> 	 */
> 	home_path = getenv("HOME");
>-	lockfile = calloc(strlen(home_path) + strlen(lockname) + 1,
>+	lockfile = calloc(LOCKFILE_HOME_PATH + strlen(lockname) + 1,
> 			  sizeof(char));
>
>-	strcat(lockfile, home_path);
>-	strcat(lockfile, "/.lock_nfp_secondary");
>+	snprintf(lockfile, LOCKFILE_HOME_PATH + strlen(lockname),
>+			"%s%s", home_path, lockname);
> 	desc->secondary_lock = open(lockfile, O_RDWR | O_CREAT |
>O_NONBLOCK,
> 				    0666);
> 	if (desc->secondary_lock < 0) {
>diff --git a/test/test/test_cryptodev.c b/test/test/test_cryptodev.c index
>84065eb49..a979603b9 100644
>--- a/test/test/test_cryptodev.c
>+++ b/test/test/test_cryptodev.c
>@@ -374,7 +374,8 @@ testsuite_setup(void)
> 			snprintf(vdev_args, sizeof(vdev_args),
> 					"%s%d", temp_str, i);
> 			strcpy(temp_str, vdev_args);
>-			strcat(temp_str, ";");
>+			strncat(temp_str, ";",
>+					VDEV_ARGS_SIZE - strlen(temp_str) -
>1);
> 			slave_core_count++;
> 			socket_id = lcore_config[i].socket_id;
> 		}
>--
>2.17.2

Any Review Please!!
--------------------------------------------------------------
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] 28+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2] drivers: fix to replace strcat with strncat
  2019-01-21 10:43   ` [dpdk-stable] [dpdk-dev] " Parthasarathy, JananeeX M
@ 2019-02-07 11:56     ` Ferruh Yigit
  2019-02-07 12:08       ` Thomas Monjalon
  2019-02-07 13:27       ` Bruce Richardson
  0 siblings, 2 replies; 28+ messages in thread
From: Ferruh Yigit @ 2019-02-07 11:56 UTC (permalink / raw)
  To: Parthasarathy, JananeeX M, dev
  Cc: rmody, Pattan, Reshma, shshaikh, Xing, Beilei, Zhang, Qi Z,
	alejandro.lucero, De Lara Guarch, Pablo, Doherty, Declan,
	Chaitanya Babu, TalluriX, stable, Bruce Richardson,
	Thomas Monjalon

On 1/21/2019 10:43 AM, Parthasarathy, JananeeX M wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Chaitanya Babu Talluri
>> Sent: Friday, January 18, 2019 8:54 PM
>> To: dev@dpdk.org
>> Cc: rmody@marvell.com; Pattan, Reshma <reshma.pattan@intel.com>;
>> shshaikh@marvell.com; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
>> <qi.z.zhang@intel.com>; alejandro.lucero@netronome.com; De Lara Guarch,
>> Pablo <pablo.de.lara.guarch@intel.com>; Doherty, Declan
>> <declan.doherty@intel.com>; Chaitanya Babu, TalluriX
>> <tallurix.chaitanya.babu@intel.com>; stable@dpdk.org
>> Subject: [dpdk-dev] [PATCH v2] drivers: fix to replace strcat with strncat
>>
>> Strcat does not check the destination length and there might be chances of
>> string overflow so insted of strcat, strncat is used.
>>
>> Fixes: 540a211084 ("bnx2x: driver core")
>> Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
>> Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
>> Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>

<...>

> 
> Any Review Please!!
> 

cc'ed Bruce & Thomas.

What do you think getting strlcat() patch first and updating this patch to use
strlcat()?

Are we OK to get strlcat as the default API?

Thanks,
ferruh

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2] drivers: fix to replace strcat with strncat
  2019-02-07 11:56     ` Ferruh Yigit
@ 2019-02-07 12:08       ` Thomas Monjalon
  2019-02-07 13:27       ` Bruce Richardson
  1 sibling, 0 replies; 28+ messages in thread
From: Thomas Monjalon @ 2019-02-07 12:08 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Parthasarathy, JananeeX M, dev, rmody, Pattan, Reshma, shshaikh,
	Xing, Beilei, Zhang, Qi Z, alejandro.lucero, De Lara Guarch,
	Pablo, Doherty, Declan, Chaitanya Babu, TalluriX, stable,
	Bruce Richardson

07/02/2019 12:56, Ferruh Yigit:
> On 1/21/2019 10:43 AM, Parthasarathy, JananeeX M wrote:
> > 
> > 
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Chaitanya Babu Talluri
> >> Sent: Friday, January 18, 2019 8:54 PM
> >> To: dev@dpdk.org
> >> Cc: rmody@marvell.com; Pattan, Reshma <reshma.pattan@intel.com>;
> >> shshaikh@marvell.com; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> >> <qi.z.zhang@intel.com>; alejandro.lucero@netronome.com; De Lara Guarch,
> >> Pablo <pablo.de.lara.guarch@intel.com>; Doherty, Declan
> >> <declan.doherty@intel.com>; Chaitanya Babu, TalluriX
> >> <tallurix.chaitanya.babu@intel.com>; stable@dpdk.org
> >> Subject: [dpdk-dev] [PATCH v2] drivers: fix to replace strcat with strncat
> >>
> >> Strcat does not check the destination length and there might be chances of
> >> string overflow so insted of strcat, strncat is used.
> >>
> >> Fixes: 540a211084 ("bnx2x: driver core")
> >> Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
> >> Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
> >> Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
> 
> <...>
> 
> > 
> > Any Review Please!!
> > 
> 
> cc'ed Bruce & Thomas.
> 
> What do you think getting strlcat() patch first and updating this patch to use
> strlcat()?
> 
> Are we OK to get strlcat as the default API?

No problem

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2] drivers: fix to replace strcat with strncat
  2019-02-07 11:56     ` Ferruh Yigit
  2019-02-07 12:08       ` Thomas Monjalon
@ 2019-02-07 13:27       ` Bruce Richardson
  2019-02-13 11:54         ` Ferruh Yigit
  1 sibling, 1 reply; 28+ messages in thread
From: Bruce Richardson @ 2019-02-07 13:27 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Parthasarathy, JananeeX M, dev, rmody, Pattan, Reshma, shshaikh,
	Xing, Beilei, Zhang, Qi Z, alejandro.lucero, De Lara Guarch,
	Pablo, Doherty, Declan, Chaitanya Babu, TalluriX, stable,
	Thomas Monjalon

On Thu, Feb 07, 2019 at 11:56:30AM +0000, Ferruh Yigit wrote:
> On 1/21/2019 10:43 AM, Parthasarathy, JananeeX M wrote:
> > 
> > 
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Chaitanya Babu Talluri
> >> Sent: Friday, January 18, 2019 8:54 PM
> >> To: dev@dpdk.org
> >> Cc: rmody@marvell.com; Pattan, Reshma <reshma.pattan@intel.com>;
> >> shshaikh@marvell.com; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> >> <qi.z.zhang@intel.com>; alejandro.lucero@netronome.com; De Lara Guarch,
> >> Pablo <pablo.de.lara.guarch@intel.com>; Doherty, Declan
> >> <declan.doherty@intel.com>; Chaitanya Babu, TalluriX
> >> <tallurix.chaitanya.babu@intel.com>; stable@dpdk.org
> >> Subject: [dpdk-dev] [PATCH v2] drivers: fix to replace strcat with strncat
> >>
> >> Strcat does not check the destination length and there might be chances of
> >> string overflow so insted of strcat, strncat is used.
> >>
> >> Fixes: 540a211084 ("bnx2x: driver core")
> >> Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
> >> Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
> >> Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
> 
> <...>
> 
> > 
> > Any Review Please!!
> > 
> 
> cc'ed Bruce & Thomas.
> 
> What do you think getting strlcat() patch first and updating this patch to use
> strlcat()?
> 
> Are we OK to get strlcat as the default API?
> 
"strlcat" is just saner to use, so +1 for this approach.

/Bruce

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2] drivers: fix to replace strcat with strncat
  2019-02-07 13:27       ` Bruce Richardson
@ 2019-02-13 11:54         ` Ferruh Yigit
  0 siblings, 0 replies; 28+ messages in thread
From: Ferruh Yigit @ 2019-02-13 11:54 UTC (permalink / raw)
  To: Parthasarathy, JananeeX M
  Cc: Bruce Richardson, dev, rmody, Pattan, Reshma, shshaikh, Xing,
	Beilei, Zhang, Qi Z, alejandro.lucero, De Lara Guarch, Pablo,
	Doherty, Declan, Chaitanya Babu, TalluriX, stable,
	Thomas Monjalon

On 2/7/2019 1:27 PM, Bruce Richardson wrote:
> On Thu, Feb 07, 2019 at 11:56:30AM +0000, Ferruh Yigit wrote:
>> On 1/21/2019 10:43 AM, Parthasarathy, JananeeX M wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Chaitanya Babu Talluri
>>>> Sent: Friday, January 18, 2019 8:54 PM
>>>> To: dev@dpdk.org
>>>> Cc: rmody@marvell.com; Pattan, Reshma <reshma.pattan@intel.com>;
>>>> shshaikh@marvell.com; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
>>>> <qi.z.zhang@intel.com>; alejandro.lucero@netronome.com; De Lara Guarch,
>>>> Pablo <pablo.de.lara.guarch@intel.com>; Doherty, Declan
>>>> <declan.doherty@intel.com>; Chaitanya Babu, TalluriX
>>>> <tallurix.chaitanya.babu@intel.com>; stable@dpdk.org
>>>> Subject: [dpdk-dev] [PATCH v2] drivers: fix to replace strcat with strncat
>>>>
>>>> Strcat does not check the destination length and there might be chances of
>>>> string overflow so insted of strcat, strncat is used.
>>>>
>>>> Fixes: 540a211084 ("bnx2x: driver core")
>>>> Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
>>>> Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
>>>> Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
>>
>> <...>
>>
>>>
>>> Any Review Please!!
>>>
>>
>> cc'ed Bruce & Thomas.
>>
>> What do you think getting strlcat() patch first and updating this patch to use
>> strlcat()?
>>
>> Are we OK to get strlcat as the default API?
>>
> "strlcat" is just saner to use, so +1 for this approach.

Hi Jananee,

'strlcat' support is merged into main repo now. Can you please send a new
version of this patch to use 'strlcat'?

Thanks,
ferruh

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [dpdk-stable] [PATCH v3] drivers: fix to replace strcat with strlcat
  2019-01-18 15:23 ` Chaitanya Babu Talluri
  2019-01-21 10:43   ` [dpdk-stable] [dpdk-dev] " Parthasarathy, JananeeX M
@ 2019-02-27  6:02   ` Chaitanya Babu Talluri
  2019-02-27  9:43     ` Ferruh Yigit
                       ` (3 more replies)
  1 sibling, 4 replies; 28+ messages in thread
From: Chaitanya Babu Talluri @ 2019-02-27  6:02 UTC (permalink / raw)
  To: dev
  Cc: reshma.pattan, jananeex.m.parthasarathy, rmody, shshaikh,
	beilei.xing, qi.z.zhang, alejandro.lucero, pablo.de.lara.guarch,
	declan.doherty, Chaitanya Babu Talluri, stable

Strcat does not check the destination length and there might be
chances of string overflow so instead of strcat, strlcat is used.

Fixes: 540a211084 ("bnx2x: driver core")
Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
Cc: stable@dpdk.org

Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
---
v3: Instead of strncat, used strlcat.
v2: Instead of strncat, used snprintf.
---
 drivers/net/bnx2x/bnx2x.c                  |  6 ++++--
 drivers/net/i40e/i40e_ethdev.c             |  6 ++++--
 drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c | 10 ++++++----
 test/test/test_cryptodev.c                 |  5 ++++-
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index 4c775c163..e418fd7d1 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -11734,13 +11734,15 @@ static const char *get_bnx2x_flags(uint32_t flags)
 
 	for (i = 0; i < 5; i++)
 		if (flags & (1 << i)) {
-			strcat(flag_str, flag[i]);
+			strlcat(flag_str, flag[i],
+				BNX2X_INFO_STR_MAX - strlen(flag_str) - 1);
 			flags ^= (1 << i);
 		}
 	if (flags) {
 		static char unknown[BNX2X_INFO_STR_MAX];
 		snprintf(unknown, 32, "Unknown flag mask %x", flags);
-		strcat(flag_str, unknown);
+		strlcat(flag_str, unknown,
+			BNX2X_INFO_STR_MAX  - strlen(flag_str) - 1);
 	}
 	return flag_str;
 }
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index dca61f03a..fac4e943f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -12201,8 +12201,10 @@ i40e_update_customized_pctype(struct rte_eth_dev *dev, uint8_t *pkg,
 			for (n = 0; n < proto_num; n++) {
 				if (proto[n].proto_id != proto_id)
 					continue;
-				strcat(name, proto[n].name);
-				strcat(name, "_");
+				strlcat(name, proto[n].name,
+					sizeof(name) - strlen(name) - 1);
+				strlcat(name, "_",
+					sizeof(name) - strlen(name) - 1);
 				break;
 			}
 		}
diff --git a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
index 39bd48a83..1c54cc600 100644
--- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
+++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
@@ -73,6 +73,8 @@
 #define NFP_PCIE_CPP_BAR_PCIETOCPPEXPBAR(bar, slot) \
 	(((bar) * 8 + (slot)) * 4)
 
+#define LOCKFILE_HOME_PATH 256
+
 /*
  * Define to enable a bit more verbose debug output.
  * Set to 1 to enable a bit more verbose debug output.
@@ -685,11 +687,11 @@ nfp_acquire_secondary_process_lock(struct nfp_pcie_user *desc)
 	 * driver is used because that implies root user.
 	 */
 	home_path = getenv("HOME");
-	lockfile = calloc(strlen(home_path) + strlen(lockname) + 1,
-			  sizeof(char));
+	lockfile = calloc(LOCKFILE_HOME_PATH + strlen(lockname) + 1,
+			sizeof(char));
 
-	strcat(lockfile, home_path);
-	strcat(lockfile, "/.lock_nfp_secondary");
+	snprintf(lockfile, LOCKFILE_HOME_PATH + strlen(lockname),
+			"%s%s", home_path, lockname);
 	desc->secondary_lock = open(lockfile, O_RDWR | O_CREAT | O_NONBLOCK,
 				    0666);
 	if (desc->secondary_lock < 0) {
diff --git a/test/test/test_cryptodev.c b/test/test/test_cryptodev.c
index 32f1893bc..e99756f60 100644
--- a/test/test/test_cryptodev.c
+++ b/test/test/test_cryptodev.c
@@ -11,6 +11,8 @@
 #include <rte_memcpy.h>
 #include <rte_pause.h>
 #include <rte_bus_vdev.h>
+#include <string.h>
+#include <rte_string_fns.h>
 
 #include <rte_crypto.h>
 #include <rte_cryptodev.h>
@@ -375,7 +377,8 @@ testsuite_setup(void)
 			snprintf(vdev_args, sizeof(vdev_args),
 					"%s%d", temp_str, i);
 			strcpy(temp_str, vdev_args);
-			strcat(temp_str, ";");
+			strlcat(temp_str, ";",
+				VDEV_ARGS_SIZE - strlen(temp_str) - 1);
 			slave_core_count++;
 			socket_id = lcore_config[i].socket_id;
 		}
-- 
2.17.2

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [PATCH v3] drivers: fix to replace strcat with strlcat
  2019-02-27  6:02   ` [dpdk-stable] [PATCH v3] drivers: fix to replace strcat with strlcat Chaitanya Babu Talluri
@ 2019-02-27  9:43     ` Ferruh Yigit
  2019-02-27  9:49     ` [dpdk-stable] [dpdk-dev] " Bruce Richardson
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 28+ messages in thread
From: Ferruh Yigit @ 2019-02-27  9:43 UTC (permalink / raw)
  To: Chaitanya Babu Talluri, dev
  Cc: reshma.pattan, jananeex.m.parthasarathy, rmody, shshaikh,
	beilei.xing, qi.z.zhang, alejandro.lucero, pablo.de.lara.guarch,
	declan.doherty, stable

On 2/27/2019 6:02 AM, Chaitanya Babu Talluri wrote:
> Strcat does not check the destination length and there might be
> chances of string overflow so instead of strcat, strlcat is used.
> 
> Fixes: 540a211084 ("bnx2x: driver core")
> Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
> Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
> Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
> ---
> v3: Instead of strncat, used strlcat.
> v2: Instead of strncat, used snprintf.
> ---
>  drivers/net/bnx2x/bnx2x.c                  |  6 ++++--
>  drivers/net/i40e/i40e_ethdev.c             |  6 ++++--
>  drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c | 10 ++++++----
>  test/test/test_cryptodev.c                 |  5 ++++-
>  4 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
> index 4c775c163..e418fd7d1 100644
> --- a/drivers/net/bnx2x/bnx2x.c
> +++ b/drivers/net/bnx2x/bnx2x.c
> @@ -11734,13 +11734,15 @@ static const char *get_bnx2x_flags(uint32_t flags)
>  
>  	for (i = 0; i < 5; i++)
>  		if (flags & (1 << i)) {
> -			strcat(flag_str, flag[i]);
> +			strlcat(flag_str, flag[i],
> +				BNX2X_INFO_STR_MAX - strlen(flag_str) - 1);


Hi Chaitanya,

I am not sure if this is correct usage of `strlcat`, can you please check its
man page [1], my concern is specially following part:

"... Unlike those functions, strlcpy() and strlcat() take the full size of the
buffer (not just the length) and ... "

[1]
https://linux.die.net/man/3/strlcat

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v3] drivers: fix to replace strcat with strlcat
  2019-02-27  6:02   ` [dpdk-stable] [PATCH v3] drivers: fix to replace strcat with strlcat Chaitanya Babu Talluri
  2019-02-27  9:43     ` Ferruh Yigit
@ 2019-02-27  9:49     ` Bruce Richardson
  2019-02-27 10:26     ` [dpdk-stable] " Pattan, Reshma
  2019-03-05 13:14     ` [dpdk-stable] [PATCH v4] drivers: fix possible overflow with strcat Chaitanya Babu Talluri
  3 siblings, 0 replies; 28+ messages in thread
From: Bruce Richardson @ 2019-02-27  9:49 UTC (permalink / raw)
  To: Chaitanya Babu Talluri
  Cc: dev, reshma.pattan, jananeex.m.parthasarathy, rmody, shshaikh,
	beilei.xing, qi.z.zhang, alejandro.lucero, pablo.de.lara.guarch,
	declan.doherty, stable

On Wed, Feb 27, 2019 at 06:02:51AM +0000, Chaitanya Babu Talluri wrote:
> Strcat does not check the destination length and there might be
> chances of string overflow so instead of strcat, strlcat is used.
> 
> Fixes: 540a211084 ("bnx2x: driver core")
> Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
> Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
> Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
> ---
> v3: Instead of strncat, used strlcat.
> v2: Instead of strncat, used snprintf.
> ---
>  drivers/net/bnx2x/bnx2x.c                  |  6 ++++--
>  drivers/net/i40e/i40e_ethdev.c             |  6 ++++--
>  drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c | 10 ++++++----
>  test/test/test_cryptodev.c                 |  5 ++++-
>  4 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
> index 4c775c163..e418fd7d1 100644
> --- a/drivers/net/bnx2x/bnx2x.c
> +++ b/drivers/net/bnx2x/bnx2x.c
> @@ -11734,13 +11734,15 @@ static const char *get_bnx2x_flags(uint32_t flags)
>  
>  	for (i = 0; i < 5; i++)
>  		if (flags & (1 << i)) {
> -			strcat(flag_str, flag[i]);
> +			strlcat(flag_str, flag[i],
> +				BNX2X_INFO_STR_MAX - strlen(flag_str) - 1);
>  			flags ^= (1 << i);
>  		}
>  	if (flags) {
>  		static char unknown[BNX2X_INFO_STR_MAX];
>  		snprintf(unknown, 32, "Unknown flag mask %x", flags);
> -		strcat(flag_str, unknown);
> +		strlcat(flag_str, unknown,
> +			BNX2X_INFO_STR_MAX  - strlen(flag_str) - 1);
>  	}
This doesn't look right to me. "Strlcat" takes the saner approach of having
the length parameter being total length so subtraction etc. should not be
necessary. I think this should just be
"strlcat(flag_str, unknown, BNX2X_INFO_STR_MAX);"

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [PATCH v3] drivers: fix to replace strcat with strlcat
  2019-02-27  6:02   ` [dpdk-stable] [PATCH v3] drivers: fix to replace strcat with strlcat Chaitanya Babu Talluri
  2019-02-27  9:43     ` Ferruh Yigit
  2019-02-27  9:49     ` [dpdk-stable] [dpdk-dev] " Bruce Richardson
@ 2019-02-27 10:26     ` Pattan, Reshma
  2019-03-05 13:14     ` [dpdk-stable] [PATCH v4] drivers: fix possible overflow with strcat Chaitanya Babu Talluri
  3 siblings, 0 replies; 28+ messages in thread
From: Pattan, Reshma @ 2019-02-27 10:26 UTC (permalink / raw)
  To: Chaitanya Babu, TalluriX, dev
  Cc: Parthasarathy, JananeeX M, rmody, shshaikh, Xing, Beilei, Zhang,
	Qi Z, alejandro.lucero, De Lara Guarch, Pablo, Doherty, Declan,
	stable



> -----Original Message-----
> From: Chaitanya Babu, TalluriX
> Sent: Wednesday, February 27, 2019 6:03 AM
> To: dev@dpdk.org
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index dca61f03a..fac4e943f 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -12201,8 +12201,10 @@ i40e_update_customized_pctype(struct
> rte_eth_dev *dev, uint8_t *pkg,
>  			for (n = 0; n < proto_num; n++) {
>  				if (proto[n].proto_id != proto_id)
>  					continue;
> -				strcat(name, proto[n].name);
> -				strcat(name, "_");
> +				strlcat(name, proto[n].name,
> +					sizeof(name) - strlen(name) - 1);
> +				strlcat(name, "_",
> +					sizeof(name) - strlen(name) - 1);
>  				break;
>  			}
>  		}
You need to include rte_string_fns.h here , check the build failure at below link. 
http://patches.dpdk.org/patch/50535/

In other files the header is included indirectly, but I suggest include this explicitly to avoid any header dependencies .

Also, commit message heading should be "fix possible overflow with strlcat"? 

Thanks,
Reshma

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [dpdk-stable] [PATCH v4] drivers: fix possible overflow with strcat
  2019-02-27  6:02   ` [dpdk-stable] [PATCH v3] drivers: fix to replace strcat with strlcat Chaitanya Babu Talluri
                       ` (2 preceding siblings ...)
  2019-02-27 10:26     ` [dpdk-stable] " Pattan, Reshma
@ 2019-03-05 13:14     ` Chaitanya Babu Talluri
  2019-03-06 18:14       ` Ferruh Yigit
  2019-03-07 12:56       ` [dpdk-stable] [PATCH v5] " Chaitanya Babu Talluri
  3 siblings, 2 replies; 28+ messages in thread
From: Chaitanya Babu Talluri @ 2019-03-05 13:14 UTC (permalink / raw)
  To: dev
  Cc: reshma.pattan, jananeex.m.parthasarathy, rmody, shshaikh,
	beilei.xing, qi.z.zhang, alejandro.lucero, pablo.de.lara.guarch,
	declan.doherty, Chaitanya Babu Talluri, stable

strcat does not check the destination length and there might be
chances of string overflow so instead of strcat, strlcat is used.

Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
Fixes: 540a211084 ("bnx2x: driver core")
Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
Fixes: ef28aa96e5 ("net/nfp: support multiprocess")
Cc: stable@dpdk.org

Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
---
v4: Corrected usage of strlcat.
v3: Instead of strncat, used strlcat.
v2: Instead of strncat, used snprintf.
---
 app/test/test_cryptodev.c                  |  3 ++-
 drivers/net/bnx2x/bnx2x.c                  |  4 +++-
 drivers/net/i40e/i40e_ethdev.c             |  4 ++--
 drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c | 10 ++++++----
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 32f1893bc..2ff204137 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -15,6 +15,7 @@
 #include <rte_crypto.h>
 #include <rte_cryptodev.h>
 #include <rte_cryptodev_pmd.h>
+#include <rte_string_fns.h>
 
 #ifdef RTE_LIBRTE_PMD_CRYPTO_SCHEDULER
 #include <rte_cryptodev_scheduler.h>
@@ -375,7 +376,7 @@ testsuite_setup(void)
 			snprintf(vdev_args, sizeof(vdev_args),
 					"%s%d", temp_str, i);
 			strcpy(temp_str, vdev_args);
-			strcat(temp_str, ";");
+			strlcat(temp_str, ";", sizeof(temp_str));
 			slave_core_count++;
 			socket_id = lcore_config[i].socket_id;
 		}
diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index 4c775c163..73987518d 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -25,6 +25,7 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <zlib.h>
+#include <rte_string_fns.h>
 
 #define BNX2X_PMD_VER_PREFIX "BNX2X PMD"
 #define BNX2X_PMD_VERSION_MAJOR 1
@@ -11734,13 +11735,14 @@ static const char *get_bnx2x_flags(uint32_t flags)
 
 	for (i = 0; i < 5; i++)
 		if (flags & (1 << i)) {
-			strcat(flag_str, flag[i]);
+			strlcat(flag_str, flag[i], sizeof(flag_str));
 			flags ^= (1 << i);
 		}
 	if (flags) {
 		static char unknown[BNX2X_INFO_STR_MAX];
 		snprintf(unknown, 32, "Unknown flag mask %x", flags);
 		strcat(flag_str, unknown);
+		strlcat(flag_str, unknown, sizeof(flag_str));
 	}
 	return flag_str;
 }
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index dca61f03a..9bc9a4390 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -12201,8 +12201,8 @@ i40e_update_customized_pctype(struct rte_eth_dev *dev, uint8_t *pkg,
 			for (n = 0; n < proto_num; n++) {
 				if (proto[n].proto_id != proto_id)
 					continue;
-				strcat(name, proto[n].name);
-				strcat(name, "_");
+				strlcat(name, proto[n].name, sizeof(name));
+				strlcat(name, "_", sizeof(name));
 				break;
 			}
 		}
diff --git a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
index 39bd48a83..1c54cc600 100644
--- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
+++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
@@ -73,6 +73,8 @@
 #define NFP_PCIE_CPP_BAR_PCIETOCPPEXPBAR(bar, slot) \
 	(((bar) * 8 + (slot)) * 4)
 
+#define LOCKFILE_HOME_PATH 256
+
 /*
  * Define to enable a bit more verbose debug output.
  * Set to 1 to enable a bit more verbose debug output.
@@ -685,11 +687,11 @@ nfp_acquire_secondary_process_lock(struct nfp_pcie_user *desc)
 	 * driver is used because that implies root user.
 	 */
 	home_path = getenv("HOME");
-	lockfile = calloc(strlen(home_path) + strlen(lockname) + 1,
-			  sizeof(char));
+	lockfile = calloc(LOCKFILE_HOME_PATH + strlen(lockname) + 1,
+			sizeof(char));
 
-	strcat(lockfile, home_path);
-	strcat(lockfile, "/.lock_nfp_secondary");
+	snprintf(lockfile, LOCKFILE_HOME_PATH + strlen(lockname),
+			"%s%s", home_path, lockname);
 	desc->secondary_lock = open(lockfile, O_RDWR | O_CREAT | O_NONBLOCK,
 				    0666);
 	if (desc->secondary_lock < 0) {
-- 
2.17.2

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [PATCH v4] drivers: fix possible overflow with strcat
  2019-03-05 13:14     ` [dpdk-stable] [PATCH v4] drivers: fix possible overflow with strcat Chaitanya Babu Talluri
@ 2019-03-06 18:14       ` Ferruh Yigit
  2019-03-07 12:56       ` [dpdk-stable] [PATCH v5] " Chaitanya Babu Talluri
  1 sibling, 0 replies; 28+ messages in thread
From: Ferruh Yigit @ 2019-03-06 18:14 UTC (permalink / raw)
  To: Chaitanya Babu Talluri, dev
  Cc: reshma.pattan, jananeex.m.parthasarathy, rmody, shshaikh,
	beilei.xing, qi.z.zhang, alejandro.lucero, pablo.de.lara.guarch,
	declan.doherty, stable

<...>

> @@ -11734,13 +11735,14 @@ static const char *get_bnx2x_flags(uint32_t flags)
>  
>  	for (i = 0; i < 5; i++)
>  		if (flags & (1 << i)) {
> -			strcat(flag_str, flag[i]);
> +			strlcat(flag_str, flag[i], sizeof(flag_str));
>  			flags ^= (1 << i);
>  		}
>  	if (flags) {
>  		static char unknown[BNX2X_INFO_STR_MAX];
>  		snprintf(unknown, 32, "Unknown flag mask %x", flags);
>  		strcat(flag_str, unknown);
> +		strlcat(flag_str, unknown, sizeof(flag_str));

Intention is to replace the 'strcat' right, seems missed to remove old code.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [dpdk-stable] [PATCH v5] drivers: fix possible overflow with strcat
  2019-03-05 13:14     ` [dpdk-stable] [PATCH v4] drivers: fix possible overflow with strcat Chaitanya Babu Talluri
  2019-03-06 18:14       ` Ferruh Yigit
@ 2019-03-07 12:56       ` Chaitanya Babu Talluri
  2019-03-13 18:39         ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
  2019-03-14 13:34         ` [dpdk-stable] [PATCH v6] drivers/net: " Chaitanya Babu Talluri
  1 sibling, 2 replies; 28+ messages in thread
From: Chaitanya Babu Talluri @ 2019-03-07 12:56 UTC (permalink / raw)
  To: dev
  Cc: reshma.pattan, jananeex.m.parthasarathy, rmody, shshaikh,
	beilei.xing, qi.z.zhang, alejandro.lucero, pablo.de.lara.guarch,
	declan.doherty, Chaitanya Babu Talluri, stable

strcat does not check the destination length and there might be
chances of string overflow so instead of strcat, strlcat is used.

Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
Fixes: 540a211084 ("bnx2x: driver core")
Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
Cc: stable@dpdk.org

Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
---
v5: Removed strcat.
v4: Corrected usage of strlcat.
v3: Instead of strncat, used strlcat.
v2: Instead of strncat, used snprintf.
---
 app/test/test_cryptodev.c      | 3 ++-
 drivers/net/bnx2x/bnx2x.c      | 5 +++--
 drivers/net/i40e/i40e_ethdev.c | 4 ++--
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 32f1893bc..8d5c138a5 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -11,6 +11,7 @@
 #include <rte_memcpy.h>
 #include <rte_pause.h>
 #include <rte_bus_vdev.h>
+#include <rte_string_fns.h>
 
 #include <rte_crypto.h>
 #include <rte_cryptodev.h>
@@ -375,7 +376,7 @@ testsuite_setup(void)
 			snprintf(vdev_args, sizeof(vdev_args),
 					"%s%d", temp_str, i);
 			strcpy(temp_str, vdev_args);
-			strcat(temp_str, ";");
+			strlcat(temp_str, ";", sizeof(temp_str));
 			slave_core_count++;
 			socket_id = lcore_config[i].socket_id;
 		}
diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index 26b3828e8..ab092e23f 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -25,6 +25,7 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <zlib.h>
+#include <rte_string_fns.h>
 
 #define BNX2X_PMD_VER_PREFIX "BNX2X PMD"
 #define BNX2X_PMD_VERSION_MAJOR 1
@@ -11741,13 +11742,13 @@ static const char *get_bnx2x_flags(uint32_t flags)
 
 	for (i = 0; i < 5; i++)
 		if (flags & (1 << i)) {
-			strcat(flag_str, flag[i]);
+			strlcat(flag_str, flag[i], sizeof(flag_str));
 			flags ^= (1 << i);
 		}
 	if (flags) {
 		static char unknown[BNX2X_INFO_STR_MAX];
 		snprintf(unknown, 32, "Unknown flag mask %x", flags);
-		strcat(flag_str, unknown);
+		strlcat(flag_str, unknown, sizeof(flag_str));
 	}
 	return flag_str;
 }
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index dca61f03a..9bc9a4390 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -12201,8 +12201,8 @@ i40e_update_customized_pctype(struct rte_eth_dev *dev, uint8_t *pkg,
 			for (n = 0; n < proto_num; n++) {
 				if (proto[n].proto_id != proto_id)
 					continue;
-				strcat(name, proto[n].name);
-				strcat(name, "_");
+				strlcat(name, proto[n].name, sizeof(name));
+				strlcat(name, "_", sizeof(name));
 				break;
 			}
 		}
-- 
2.17.2

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v5] drivers: fix possible overflow with strcat
  2019-03-07 12:56       ` [dpdk-stable] [PATCH v5] " Chaitanya Babu Talluri
@ 2019-03-13 18:39         ` Ferruh Yigit
  2019-03-14 13:34         ` [dpdk-stable] [PATCH v6] drivers/net: " Chaitanya Babu Talluri
  1 sibling, 0 replies; 28+ messages in thread
From: Ferruh Yigit @ 2019-03-13 18:39 UTC (permalink / raw)
  To: Chaitanya Babu Talluri, dev
  Cc: reshma.pattan, jananeex.m.parthasarathy, rmody, shshaikh,
	beilei.xing, qi.z.zhang, alejandro.lucero, pablo.de.lara.guarch,
	declan.doherty, stable

On 3/7/2019 12:56 PM, Chaitanya Babu Talluri wrote:
> strcat does not check the destination length and there might be
> chances of string overflow so instead of strcat, strlcat is used.
> 
> Fixes: 6f4eec2565 ("test/crypto: enhance scheduler unit tests")
> Fixes: 540a211084 ("bnx2x: driver core")
> Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
> ---
> v5: Removed strcat.

You also dropped "drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c" change which was
in v4, intentional?

> v4: Corrected usage of strlcat.
> v3: Instead of strncat, used strlcat.
> v2: Instead of strncat, used snprintf.
> ---
>  app/test/test_cryptodev.c      | 3 ++-

test_cryptodev.c is not driver, also for organizational issues, can you send it
as separate patch?

When it is removed, you can use "drivers/net: ..." in patch title.

>  drivers/net/bnx2x/bnx2x.c      | 5 +++--
>  drivers/net/i40e/i40e_ethdev.c | 4 ++--
>  3 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> index 32f1893bc..8d5c138a5 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -11,6 +11,7 @@
>  #include <rte_memcpy.h>
>  #include <rte_pause.h>
>  #include <rte_bus_vdev.h>
> +#include <rte_string_fns.h>
>  
>  #include <rte_crypto.h>
>  #include <rte_cryptodev.h>
> @@ -375,7 +376,7 @@ testsuite_setup(void)
>  			snprintf(vdev_args, sizeof(vdev_args),
>  					"%s%d", temp_str, i);
>  			strcpy(temp_str, vdev_args);
> -			strcat(temp_str, ";");
> +			strlcat(temp_str, ";", sizeof(temp_str));
>  			slave_core_count++;
>  			socket_id = lcore_config[i].socket_id;
>  		}
> diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
> index 26b3828e8..ab092e23f 100644
> --- a/drivers/net/bnx2x/bnx2x.c
> +++ b/drivers/net/bnx2x/bnx2x.c
> @@ -25,6 +25,7 @@
>  #include <sys/stat.h>
>  #include <fcntl.h>
>  #include <zlib.h>
> +#include <rte_string_fns.h>
>  
>  #define BNX2X_PMD_VER_PREFIX "BNX2X PMD"
>  #define BNX2X_PMD_VERSION_MAJOR 1
> @@ -11741,13 +11742,13 @@ static const char *get_bnx2x_flags(uint32_t flags)
>  
>  	for (i = 0; i < 5; i++)
>  		if (flags & (1 << i)) {
> -			strcat(flag_str, flag[i]);
> +			strlcat(flag_str, flag[i], sizeof(flag_str));
>  			flags ^= (1 << i);
>  		}
>  	if (flags) {
>  		static char unknown[BNX2X_INFO_STR_MAX];
>  		snprintf(unknown, 32, "Unknown flag mask %x", flags);
> -		strcat(flag_str, unknown);
> +		strlcat(flag_str, unknown, sizeof(flag_str));
>  	}
>  	return flag_str;
>  }
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index dca61f03a..9bc9a4390 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -12201,8 +12201,8 @@ i40e_update_customized_pctype(struct rte_eth_dev *dev, uint8_t *pkg,
>  			for (n = 0; n < proto_num; n++) {
>  				if (proto[n].proto_id != proto_id)
>  					continue;
> -				strcat(name, proto[n].name);
> -				strcat(name, "_");
> +				strlcat(name, proto[n].name, sizeof(name));
> +				strlcat(name, "_", sizeof(name));
>  				break;
>  			}
>  		}
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [dpdk-stable] [PATCH v6] drivers/net: fix possible overflow with strcat
  2019-03-07 12:56       ` [dpdk-stable] [PATCH v5] " Chaitanya Babu Talluri
  2019-03-13 18:39         ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
@ 2019-03-14 13:34         ` Chaitanya Babu Talluri
  2019-03-14 14:09           ` Pattan, Reshma
  2019-03-18 12:41           ` [dpdk-stable] [PATCH v7] drivers/net: fix possible overflow using strlcat Chaitanya Babu Talluri
  1 sibling, 2 replies; 28+ messages in thread
From: Chaitanya Babu Talluri @ 2019-03-14 13:34 UTC (permalink / raw)
  To: dev
  Cc: reshma.pattan, jananeex.m.parthasarathy, rmody, shshaikh,
	beilei.xing, qi.z.zhang, Chaitanya Babu Talluri, stable

strcat does not check the destination length and there might be
chances of string overflow so instead of strcat, strlcat is used.

Fixes: 540a211084 ("bnx2x: driver core")
Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
Cc: stable@dpdk.org

Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
---
v6: Updated title.
v5: Removed strcat.
v4: Corrected usage of strlcat.
v3: Instead of strncat, used strlcat.
v2: Instead of strncat, used snprintf.
---
 drivers/net/bnx2x/bnx2x.c      | 5 +++--
 drivers/net/i40e/i40e_ethdev.c | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index 26b3828e8..ab092e23f 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -25,6 +25,7 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <zlib.h>
+#include <rte_string_fns.h>
 
 #define BNX2X_PMD_VER_PREFIX "BNX2X PMD"
 #define BNX2X_PMD_VERSION_MAJOR 1
@@ -11741,13 +11742,13 @@ static const char *get_bnx2x_flags(uint32_t flags)
 
 	for (i = 0; i < 5; i++)
 		if (flags & (1 << i)) {
-			strcat(flag_str, flag[i]);
+			strlcat(flag_str, flag[i], sizeof(flag_str));
 			flags ^= (1 << i);
 		}
 	if (flags) {
 		static char unknown[BNX2X_INFO_STR_MAX];
 		snprintf(unknown, 32, "Unknown flag mask %x", flags);
-		strcat(flag_str, unknown);
+		strlcat(flag_str, unknown, sizeof(flag_str));
 	}
 	return flag_str;
 }
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index dca61f03a..9bc9a4390 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -12201,8 +12201,8 @@ i40e_update_customized_pctype(struct rte_eth_dev *dev, uint8_t *pkg,
 			for (n = 0; n < proto_num; n++) {
 				if (proto[n].proto_id != proto_id)
 					continue;
-				strcat(name, proto[n].name);
-				strcat(name, "_");
+				strlcat(name, proto[n].name, sizeof(name));
+				strlcat(name, "_", sizeof(name));
 				break;
 			}
 		}
-- 
2.17.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [PATCH v6] drivers/net: fix possible overflow with strcat
  2019-03-14 13:34         ` [dpdk-stable] [PATCH v6] drivers/net: " Chaitanya Babu Talluri
@ 2019-03-14 14:09           ` Pattan, Reshma
  2019-03-18 12:41           ` [dpdk-stable] [PATCH v7] drivers/net: fix possible overflow using strlcat Chaitanya Babu Talluri
  1 sibling, 0 replies; 28+ messages in thread
From: Pattan, Reshma @ 2019-03-14 14:09 UTC (permalink / raw)
  To: Chaitanya Babu, TalluriX, dev
  Cc: Parthasarathy, JananeeX M, rmody, shshaikh, Xing, Beilei, Zhang,
	Qi Z, stable



> -----Original Message-----
> From: Chaitanya Babu, TalluriX
> Sent: Thursday, March 14, 2019 1:35 PM
> To: dev@dpdk.org
> Cc: Pattan, Reshma <reshma.pattan@intel.com>; Parthasarathy, JananeeX M
> <jananeex.m.parthasarathy@intel.com>; rmody@marvell.com;
> shshaikh@marvell.com; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Chaitanya Babu, TalluriX
> <tallurix.chaitanya.babu@intel.com>; stable@dpdk.org
> Subject: [PATCH v6] drivers/net: fix possible overflow with strcat
> 

%s/strcat/strlcat?

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [dpdk-stable] [PATCH v7] drivers/net: fix possible overflow using strlcat
  2019-03-14 13:34         ` [dpdk-stable] [PATCH v6] drivers/net: " Chaitanya Babu Talluri
  2019-03-14 14:09           ` Pattan, Reshma
@ 2019-03-18 12:41           ` Chaitanya Babu Talluri
  2019-03-20 20:18             ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
  2019-03-22  7:51             ` [dpdk-stable] [PATCH v8] " Chaitanya Babu Talluri
  1 sibling, 2 replies; 28+ messages in thread
From: Chaitanya Babu Talluri @ 2019-03-18 12:41 UTC (permalink / raw)
  To: dev
  Cc: reshma.pattan, jananeex.m.parthasarathy, rmody, shshaikh,
	beilei.xing, qi.z.zhang, Chaitanya Babu Talluri, stable

strcat does not check the destination length and there might be
chances of string overflow so instead of strcat, strlcat is used.

Fixes: 540a211084 ("bnx2x: driver core")
Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
Cc: stable@dpdk.org

Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
---
v7: Corrected title.
v6: Updated title.
v5: Removed strcat.
v4: Corrected usage of strlcat.
v3: Instead of strncat, used strlcat.
v2: Instead of strncat, used snprintf.
---
 drivers/net/bnx2x/bnx2x.c      | 5 +++--
 drivers/net/i40e/i40e_ethdev.c | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index 26b3828e8..fe0db021c 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -25,6 +25,7 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <zlib.h>
+#include <rte_string_fns.h>
 
 #define BNX2X_PMD_VER_PREFIX "BNX2X PMD"
 #define BNX2X_PMD_VERSION_MAJOR 1
@@ -11741,13 +11742,13 @@ static const char *get_bnx2x_flags(uint32_t flags)
 
 	for (i = 0; i < 5; i++)
 		if (flags & (1 << i)) {
-			strcat(flag_str, flag[i]);
+			strlcat(flag_str, flag[i], sizeof(flag_str))
 			flags ^= (1 << i);
 		}
 	if (flags) {
 		static char unknown[BNX2X_INFO_STR_MAX];
 		snprintf(unknown, 32, "Unknown flag mask %x", flags);
-		strcat(flag_str, unknown);
+		strlcat(flag_str, unknown, sizeof(flag_str));
 	}
 	return flag_str;
 }
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index dca61f03a..9bc9a4390 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -12201,8 +12201,8 @@ i40e_update_customized_pctype(struct rte_eth_dev *dev, uint8_t *pkg,
 			for (n = 0; n < proto_num; n++) {
 				if (proto[n].proto_id != proto_id)
 					continue;
-				strcat(name, proto[n].name);
-				strcat(name, "_");
+				strlcat(name, proto[n].name, sizeof(name));
+				strlcat(name, "_", sizeof(name));
 				break;
 			}
 		}
-- 
2.17.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v7] drivers/net: fix possible overflow using strlcat
  2019-03-18 12:41           ` [dpdk-stable] [PATCH v7] drivers/net: fix possible overflow using strlcat Chaitanya Babu Talluri
@ 2019-03-20 20:18             ` Ferruh Yigit
  2019-03-22  7:51             ` [dpdk-stable] [PATCH v8] " Chaitanya Babu Talluri
  1 sibling, 0 replies; 28+ messages in thread
From: Ferruh Yigit @ 2019-03-20 20:18 UTC (permalink / raw)
  To: Chaitanya Babu Talluri, dev
  Cc: reshma.pattan, jananeex.m.parthasarathy, rmody, shshaikh,
	beilei.xing, qi.z.zhang, stable

On 3/18/2019 12:41 PM, Chaitanya Babu Talluri wrote:
> strcat does not check the destination length and there might be
> chances of string overflow so instead of strcat, strlcat is used.
> 
> Fixes: 540a211084 ("bnx2x: driver core")
> Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
> ---
> v7: Corrected title.
> v6: Updated title.
> v5: Removed strcat.
> v4: Corrected usage of strlcat.
> v3: Instead of strncat, used strlcat.
> v2: Instead of strncat, used snprintf.
> ---
>  drivers/net/bnx2x/bnx2x.c      | 5 +++--
>  drivers/net/i40e/i40e_ethdev.c | 4 ++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
> index 26b3828e8..fe0db021c 100644
> --- a/drivers/net/bnx2x/bnx2x.c
> +++ b/drivers/net/bnx2x/bnx2x.c
> @@ -25,6 +25,7 @@
>  #include <sys/stat.h>
>  #include <fcntl.h>
>  #include <zlib.h>
> +#include <rte_string_fns.h>
>  
>  #define BNX2X_PMD_VER_PREFIX "BNX2X PMD"
>  #define BNX2X_PMD_VERSION_MAJOR 1
> @@ -11741,13 +11742,13 @@ static const char *get_bnx2x_flags(uint32_t flags)
>  
>  	for (i = 0; i < 5; i++)
>  		if (flags & (1 << i)) {
> -			strcat(flag_str, flag[i]);
> +			strlcat(flag_str, flag[i], sizeof(flag_str))

The patch doesn't compile because of the missing ';'
Can you please at least try to compile before sending the patch!

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [dpdk-stable] [PATCH v8] drivers/net: fix possible overflow using strlcat
  2019-03-18 12:41           ` [dpdk-stable] [PATCH v7] drivers/net: fix possible overflow using strlcat Chaitanya Babu Talluri
  2019-03-20 20:18             ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
@ 2019-03-22  7:51             ` Chaitanya Babu Talluri
  2019-03-22  8:02               ` [dpdk-stable] [EXT] " Shahed Shaikh
  1 sibling, 1 reply; 28+ messages in thread
From: Chaitanya Babu Talluri @ 2019-03-22  7:51 UTC (permalink / raw)
  To: dev
  Cc: reshma.pattan, jananeex.m.parthasarathy, rmody, shshaikh,
	beilei.xing, qi.z.zhang, Chaitanya Babu Talluri, stable

strcat does not check the destination length and there might be
chances of string overflow so instead of strcat, strlcat is used.

Fixes: 540a211084 ("bnx2x: driver core")
Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
Cc: stable@dpdk.org

Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
---
v8: Added missing semi-colon.
v7: Corrected title.
v6: Updated title.
v5: Removed strcat.
v4: Corrected usage of strlcat.
v3: Instead of strncat, used strlcat.
v2: Instead of strncat, used snprintf.
---
 drivers/net/bnx2x/bnx2x.c      | 5 +++--
 drivers/net/i40e/i40e_ethdev.c | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index 26b3828e8..ab092e23f 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -25,6 +25,7 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <zlib.h>
+#include <rte_string_fns.h>
 
 #define BNX2X_PMD_VER_PREFIX "BNX2X PMD"
 #define BNX2X_PMD_VERSION_MAJOR 1
@@ -11741,13 +11742,13 @@ static const char *get_bnx2x_flags(uint32_t flags)
 
 	for (i = 0; i < 5; i++)
 		if (flags & (1 << i)) {
-			strcat(flag_str, flag[i]);
+			strlcat(flag_str, flag[i], sizeof(flag_str));
 			flags ^= (1 << i);
 		}
 	if (flags) {
 		static char unknown[BNX2X_INFO_STR_MAX];
 		snprintf(unknown, 32, "Unknown flag mask %x", flags);
-		strcat(flag_str, unknown);
+		strlcat(flag_str, unknown, sizeof(flag_str));
 	}
 	return flag_str;
 }
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index dca61f03a..9bc9a4390 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -12201,8 +12201,8 @@ i40e_update_customized_pctype(struct rte_eth_dev *dev, uint8_t *pkg,
 			for (n = 0; n < proto_num; n++) {
 				if (proto[n].proto_id != proto_id)
 					continue;
-				strcat(name, proto[n].name);
-				strcat(name, "_");
+				strlcat(name, proto[n].name, sizeof(name));
+				strlcat(name, "_", sizeof(name));
 				break;
 			}
 		}
-- 
2.17.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [EXT] [PATCH v8] drivers/net: fix possible overflow using strlcat
  2019-03-22  7:51             ` [dpdk-stable] [PATCH v8] " Chaitanya Babu Talluri
@ 2019-03-22  8:02               ` Shahed Shaikh
  2019-03-22 10:35                 ` Ferruh Yigit
  0 siblings, 1 reply; 28+ messages in thread
From: Shahed Shaikh @ 2019-03-22  8:02 UTC (permalink / raw)
  To: Chaitanya Babu Talluri, dev
  Cc: reshma.pattan, jananeex.m.parthasarathy, Rasesh Mody,
	beilei.xing, qi.z.zhang, stable

> -----Original Message-----
> From: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
> Sent: Friday, March 22, 2019 1:22 PM
> To: dev@dpdk.org
> Cc: reshma.pattan@intel.com; jananeex.m.parthasarathy@intel.com; Rasesh
> Mody <rmody@marvell.com>; Shahed Shaikh <shshaikh@marvell.com>;
> beilei.xing@intel.com; qi.z.zhang@intel.com; Chaitanya Babu Talluri
> <tallurix.chaitanya.babu@intel.com>; stable@dpdk.org
> Subject: [EXT] [PATCH v8] drivers/net: fix possible overflow using strlcat
> 
> strcat does not check the destination length and there might be chances of
> string overflow so instead of strcat, strlcat is used.
> 
> Fixes: 540a211084 ("bnx2x: driver core")
> Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
> ---
> v8: Added missing semi-colon.
> v7: Corrected title.
> v6: Updated title.
> v5: Removed strcat.
> v4: Corrected usage of strlcat.
> v3: Instead of strncat, used strlcat.
> v2: Instead of strncat, used snprintf.
> ---
>  drivers/net/bnx2x/bnx2x.c      | 5 +++--
>  drivers/net/i40e/i40e_ethdev.c | 4 ++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c index
> 26b3828e8..ab092e23f 100644
> --- a/drivers/net/bnx2x/bnx2x.c
> +++ b/drivers/net/bnx2x/bnx2x.c
> @@ -25,6 +25,7 @@
>  #include <sys/stat.h>
>  #include <fcntl.h>
>  #include <zlib.h>
> +#include <rte_string_fns.h>
> 
>  #define BNX2X_PMD_VER_PREFIX "BNX2X PMD"
>  #define BNX2X_PMD_VERSION_MAJOR 1
> @@ -11741,13 +11742,13 @@ static const char *get_bnx2x_flags(uint32_t
> flags)
> 
>  	for (i = 0; i < 5; i++)
>  		if (flags & (1 << i)) {
> -			strcat(flag_str, flag[i]);
> +			strlcat(flag_str, flag[i], sizeof(flag_str));
>  			flags ^= (1 << i);
>  		}
>  	if (flags) {
>  		static char unknown[BNX2X_INFO_STR_MAX];
>  		snprintf(unknown, 32, "Unknown flag mask %x", flags);
> -		strcat(flag_str, unknown);
> +		strlcat(flag_str, unknown, sizeof(flag_str));
>  	}
>  	return flag_str;
>  }

For bnx2x PMD changes -
Acked-by: Shahed Shaikh <shshaikh@marvell.com>

Thanks,
Shahed 



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [dpdk-stable] [EXT] [PATCH v8] drivers/net: fix possible overflow using strlcat
  2019-03-22  8:02               ` [dpdk-stable] [EXT] " Shahed Shaikh
@ 2019-03-22 10:35                 ` Ferruh Yigit
  0 siblings, 0 replies; 28+ messages in thread
From: Ferruh Yigit @ 2019-03-22 10:35 UTC (permalink / raw)
  To: Shahed Shaikh, Chaitanya Babu Talluri, dev
  Cc: reshma.pattan, jananeex.m.parthasarathy, Rasesh Mody,
	beilei.xing, qi.z.zhang, stable

On 3/22/2019 8:02 AM, Shahed Shaikh wrote:
>> -----Original Message-----
>> From: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
>> Sent: Friday, March 22, 2019 1:22 PM
>> To: dev@dpdk.org
>> Cc: reshma.pattan@intel.com; jananeex.m.parthasarathy@intel.com; Rasesh
>> Mody <rmody@marvell.com>; Shahed Shaikh <shshaikh@marvell.com>;
>> beilei.xing@intel.com; qi.z.zhang@intel.com; Chaitanya Babu Talluri
>> <tallurix.chaitanya.babu@intel.com>; stable@dpdk.org
>> Subject: [EXT] [PATCH v8] drivers/net: fix possible overflow using strlcat
>>
>> strcat does not check the destination length and there might be chances of
>> string overflow so instead of strcat, strlcat is used.
>>
>> Fixes: 540a211084 ("bnx2x: driver core")
>> Fixes: e163c18a15 ("net/i40e: update ptype and pctype info")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
> 
> For bnx2x PMD changes -
> Acked-by: Shahed Shaikh <shshaikh@marvell.com>
> 

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2019-03-22 10:35 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-14  6:04 [dpdk-stable] [PATCH] drivers: fix to replace strcat with strncat Chaitanya Babu Talluri
2019-01-14 13:29 ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
2019-01-14 16:24   ` Stephen Hemminger
2019-01-17 16:44     ` Thomas Monjalon
2019-01-14 14:21 ` Bruce Richardson
2019-01-15  1:53   ` Thomas Monjalon
2019-01-18 15:11 ` [dpdk-stable] [PATCH v2] " Chaitanya Babu Talluri
2019-01-18 15:23 ` Chaitanya Babu Talluri
2019-01-21 10:43   ` [dpdk-stable] [dpdk-dev] " Parthasarathy, JananeeX M
2019-02-07 11:56     ` Ferruh Yigit
2019-02-07 12:08       ` Thomas Monjalon
2019-02-07 13:27       ` Bruce Richardson
2019-02-13 11:54         ` Ferruh Yigit
2019-02-27  6:02   ` [dpdk-stable] [PATCH v3] drivers: fix to replace strcat with strlcat Chaitanya Babu Talluri
2019-02-27  9:43     ` Ferruh Yigit
2019-02-27  9:49     ` [dpdk-stable] [dpdk-dev] " Bruce Richardson
2019-02-27 10:26     ` [dpdk-stable] " Pattan, Reshma
2019-03-05 13:14     ` [dpdk-stable] [PATCH v4] drivers: fix possible overflow with strcat Chaitanya Babu Talluri
2019-03-06 18:14       ` Ferruh Yigit
2019-03-07 12:56       ` [dpdk-stable] [PATCH v5] " Chaitanya Babu Talluri
2019-03-13 18:39         ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
2019-03-14 13:34         ` [dpdk-stable] [PATCH v6] drivers/net: " Chaitanya Babu Talluri
2019-03-14 14:09           ` Pattan, Reshma
2019-03-18 12:41           ` [dpdk-stable] [PATCH v7] drivers/net: fix possible overflow using strlcat Chaitanya Babu Talluri
2019-03-20 20:18             ` [dpdk-stable] [dpdk-dev] " Ferruh Yigit
2019-03-22  7:51             ` [dpdk-stable] [PATCH v8] " Chaitanya Babu Talluri
2019-03-22  8:02               ` [dpdk-stable] [EXT] " Shahed Shaikh
2019-03-22 10:35                 ` Ferruh Yigit

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).