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

["Parthasarathy, JananeeX M" <jananeex.m.parthasarathy@intel.com>]

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