[PATCH] drivers: fix to replace strcat with strncat

[PATCH] drivers: fix to replace strcat with strncat

[Chaitanya Babu Talluri]

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

Re: [PATCH] drivers: fix to replace strcat with strncat

[Ferruh Yigit]

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.

Re: [PATCH] drivers: fix to replace strcat with strncat

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

Re: [PATCH] drivers: fix to replace strcat with strncat

[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?

	

Re: [PATCH] drivers: fix to replace strcat with strncat

[Thomas Monjalon]

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.

Re: [PATCH] drivers: fix to replace strcat with strncat

[Thomas Monjalon]

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?

[PATCH v2] drivers: fix to replace strcat with strncat

[Chaitanya Babu Talluri]

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

[PATCH v2] drivers: fix to replace strcat with strncat

[Chaitanya Babu Talluri]

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

Re: [PATCH v2] drivers: fix to replace strcat with strncat

[Parthasarathy, JananeeX M]

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

Re: [PATCH v2] drivers: fix to replace strcat with strncat

[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?

Thanks,
ferruh

Re: [PATCH v2] drivers: fix to replace strcat with strncat

[Thomas Monjalon]

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

Re: [PATCH v2] drivers: fix to replace strcat with strncat

[Bruce Richardson]

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

Re: [PATCH v2] drivers: fix to replace strcat with strncat

[Ferruh Yigit]

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

[PATCH v3] drivers: fix to replace strcat with strlcat

[Chaitanya Babu Talluri]

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

Re: [dpdk-stable] [PATCH v3] drivers: fix to replace strcat with strlcat

[Ferruh Yigit]

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

Re: [PATCH v3] drivers: fix to replace strcat with strlcat

[Bruce Richardson]

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);"

Re: [PATCH v3] drivers: fix to replace strcat with strlcat

[Pattan, Reshma]

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

[PATCH v4] drivers: fix possible overflow with strcat

[Chaitanya Babu Talluri]

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

Re: [dpdk-stable] [PATCH v4] drivers: fix possible overflow with strcat

[Ferruh Yigit]

<...>

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

[PATCH v5] drivers: fix possible overflow with strcat

[Chaitanya Babu Talluri]

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

Re: [PATCH v5] drivers: fix possible overflow with strcat

[Ferruh Yigit]

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;
>  			}
>  		}
> 

[PATCH v6] drivers/net: fix possible overflow with strcat

[Chaitanya Babu Talluri]

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

Re: [PATCH v6] drivers/net: fix possible overflow with strcat

[Pattan, Reshma]

> -----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?

[PATCH v7] drivers/net: fix possible overflow using strlcat

[Chaitanya Babu Talluri]

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

Re: [PATCH v7] drivers/net: fix possible overflow using strlcat

[Ferruh Yigit]

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!

[PATCH v8] drivers/net: fix possible overflow using strlcat

[Chaitanya Babu Talluri]

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

Re: [EXT] [PATCH v8] drivers/net: fix possible overflow using strlcat

[Shahed Shaikh]

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

Re: [dpdk-stable] [EXT] [PATCH v8] drivers/net: fix possible overflow using strlcat

[Ferruh Yigit]

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.