[v2] drivers: fix to replace strcat with strncat
Checks
Commit Message
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(-)
Comments
>-----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.
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
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
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
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
@@ -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;
}
@@ -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;
}
}
@@ -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) {
@@ -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;
}