From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH v5] drivers: fix possible overflow with strcat Date: Wed, 13 Mar 2019 18:39:59 +0000 Message-ID: <75584dc7-b06f-67eb-903d-1e7b15544ff7@intel.com> References: <1551791666-26746-1-git-send-email-tallurix.chaitanya.babu@intel.com> <1551963385-1234-1-git-send-email-tallurix.chaitanya.babu@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: reshma.pattan@intel.com, jananeex.m.parthasarathy@intel.com, rmody@marvell.com, shshaikh@marvell.com, beilei.xing@intel.com, qi.z.zhang@intel.com, alejandro.lucero@netronome.com, pablo.de.lara.guarch@intel.com, declan.doherty@intel.com, stable@dpdk.org To: Chaitanya Babu Talluri , dev@dpdk.org Return-path: In-Reply-To: <1551963385-1234-1-git-send-email-tallurix.chaitanya.babu@intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > --- > 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 > #include > #include > +#include > > #include > #include > @@ -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 > #include > #include > +#include > > #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; > } > } >