[dpdk-dev] [PATCH] app/testbbdev: fix sprintf with snprintf

[dpdk-dev] [PATCH] app/testbbdev: fix sprintf with snprintf

[Pallantla Poornima]

sprintf function is not secure as it doesn't check the length of string.
More secure function snprintf is used.

Fixes: f714a18885 ("app/testbbdev: add test application for bbdev")
Cc: stable@dpdk.org

Signed-off-by: Pallantla Poornima <pallantlax.poornima@intel.com>
---
 app/test-bbdev/test_bbdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/test-bbdev/test_bbdev.c b/app/test-bbdev/test_bbdev.c
index a914817bc..b9fc750d3 100644
--- a/app/test-bbdev/test_bbdev.c
+++ b/app/test-bbdev/test_bbdev.c
@@ -788,14 +788,14 @@ test_bbdev_driver_init(void)
 
 	/* Initialize the maximum amount of devices */
 	do {
-		sprintf(name_tmp, "%s%i", "name_", num_devs);
+		snprintf(name_tmp, sizeof(name_tmp), "%s%i", "name_", num_devs);
 		dev2 = rte_bbdev_allocate(name_tmp);
 		TEST_ASSERT(dev2 != NULL,
 				"Failed to initialize bbdev driver");
 		++num_devs;
 	} while (num_devs < (RTE_BBDEV_MAX_DEVS - 1));
 
-	sprintf(name_tmp, "%s%i", "name_", num_devs);
+	snprintf(name_tmp, sizeof(name_tmp), "%s%i", "name_", num_devs);
 	dev2 = rte_bbdev_allocate(name_tmp);
 	TEST_ASSERT(dev2 == NULL, "Failed to initialize bbdev driver number %d "
 			"more drivers than RTE_BBDEV_MAX_DEVS: %d ", num_devs,
@@ -804,7 +804,7 @@ test_bbdev_driver_init(void)
 	num_devs--;
 
 	while (num_devs >= num_devs_tmp) {
-		sprintf(name_tmp, "%s%i", "name_", num_devs);
+		snprintf(name_tmp, sizeof(name_tmp), "%s%i", "name_", num_devs);
 		dev2 = rte_bbdev_get_named_dev(name_tmp);
 		TEST_ASSERT_SUCCESS(rte_bbdev_release(dev2),
 				"Failed to uninitialize bbdev driver %s ",
@@ -825,7 +825,7 @@ test_bbdev_driver_init(void)
 	TEST_ASSERT_FAIL(rte_bbdev_release(NULL),
 			"Failed to uninitialize bbdev driver with NULL bbdev");
 
-	sprintf(name_tmp, "%s", "invalid_name");
+	snprintf(name_tmp, sizeof(name_tmp), "%s", "invalid_name");
 	dev2 = rte_bbdev_get_named_dev(name_tmp);
 	TEST_ASSERT_FAIL(rte_bbdev_release(dev2),
 			"Failed to uninitialize bbdev driver with invalid name");
-- 
2.17.2

Re: [dpdk-dev] [PATCH] app/testbbdev: fix sprintf with snprintf

[Ferruh Yigit]

On 2/4/2019 7:15 AM, Pallantla Poornima wrote:
> @@ -825,7 +825,7 @@
> 
>  test_bbdev_driver_init(void)
> 
> TEST_ASSERT_FAIL(rte_bbdev_release(NULL),
> "Failed to uninitialize bbdev driver with NULL bbdev");
> - sprintf(name_tmp, "%s", "invalid_name");
> + snprintf(name_tmp, sizeof(name_tmp), "%s", "invalid_name");
> dev2 = rte_bbdev_get_named_dev(name_tmp);
> TEST_ASSERT_FAIL(rte_bbdev_release(dev2),
> "Failed to uninitialize bbdev driver with invalid name");

Can use strlcpy() instead of snprintf() here.

Re: [dpdk-dev] [PATCH] app/testbbdev: fix sprintf with snprintf

[Mokhtar, Amr]

> -----Original Message-----
> From: Poornima, PallantlaX
> Sent: Monday 4 February 2019 07:16
> To: dev@dpdk.org
> Cc: Pattan, Reshma <reshma.pattan@intel.com>; Mokhtar, Amr
> <amr.mokhtar@intel.com>; Poornima, PallantlaX
> <pallantlax.poornima@intel.com>; stable@dpdk.org
> Subject: [PATCH] app/testbbdev: fix sprintf with snprintf
> 
> sprintf function is not secure as it doesn't check the length of string.
> More secure function snprintf is used.
> 
> Fixes: f714a18885 ("app/testbbdev: add test application for bbdev")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Pallantla Poornima <pallantlax.poornima@intel.com>
> ---

Acked-by: Amr Mokhtar <amr.mokhtar@intel.com>

Re: [dpdk-dev] [PATCH] app/testbbdev: fix sprintf with snprintf

[Mokhtar, Amr]

> -----Original Message-----
> From: Parthasarathy, JananeeX M
> Sent: Friday 15 February 2019 11:08
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Poornima, PallantlaX
> <pallantlax.poornima@intel.com>; dev@dpdk.org
> Cc: Pattan, Reshma <reshma.pattan@intel.com>; Mokhtar, Amr
> <amr.mokhtar@intel.com>; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] app/testbbdev: fix sprintf with snprintf
> 
> Hi
> 
> >-----Original Message-----
> >From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> >Sent: Friday, February 08, 2019 7:44 PM
> >To: Poornima, PallantlaX <pallantlax.poornima@intel.com>;
> dev@dpdk.org
> >Cc: Pattan, Reshma <reshma.pattan@intel.com>; Mokhtar, Amr
> ><amr.mokhtar@intel.com>; stable@dpdk.org
> >Subject: Re: [dpdk-dev] [PATCH] app/testbbdev: fix sprintf with snprintf
> >
> >On 2/4/2019 7:15 AM, Pallantla Poornima wrote:
> >> @@ -825,7 +825,7 @@
> >>
> >>  test_bbdev_driver_init(void)
> >>
> >> TEST_ASSERT_FAIL(rte_bbdev_release(NULL),
> >> "Failed to uninitialize bbdev driver with NULL bbdev");
> >> - sprintf(name_tmp, "%s", "invalid_name");
> >> + snprintf(name_tmp, sizeof(name_tmp), "%s", "invalid_name");
> >> dev2 = rte_bbdev_get_named_dev(name_tmp);
> >> TEST_ASSERT_FAIL(rte_bbdev_release(dev2),
> >> "Failed to uninitialize bbdev driver with invalid name");
> >
> >Can use strlcpy() instead of snprintf() here.
> 
> Please confirm whether we can do this change (snprintf to strlcpy) as there
> was Ack from other reviewer.

I agree with Ferruh suggestion, as this is a simple string copying.
Feel free to use strlcpy instead and retain my Ack.
Thanks.

> Thanks
> M.P.Jananee