event/opdl: fix sprintf with snprintf

Message ID 1549264682-2979-1-git-send-email-pallantlax.poornima@intel.com
State Accepted
Delegated to: Jerin Jacob
Headers show
Series
  • event/opdl: fix sprintf with snprintf
Related show

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/checkpatch success coding style OK

Commit Message

Pallantla Poornima Feb. 4, 2019, 7:18 a.m.
sprintf function is not secure as it doesn't check the length of string.
More secure function snprintf is used.

Fixes: 3c7f3dcfb0 ("event/opdl: add PMD main body and helper function")
Cc: stable@dpdk.org

Signed-off-by: Pallantla Poornima <pallantlax.poornima@intel.com>
---
 drivers/event/opdl/opdl_evdev.c        | 7 ++++---
 drivers/event/opdl/opdl_evdev_xstats.c | 7 +++----
 2 files changed, 7 insertions(+), 7 deletions(-)

Comments

Jerin Jacob Kollanukkaran March 11, 2019, 6:51 a.m. | #1
On Mon, 2019-02-04 at 07:18 +0000, Pallantla Poornima wrote:
> sprintf function is not secure as it doesn't check the length of
> string.
> More secure function snprintf is used.
> 
> Fixes: 3c7f3dcfb0 ("event/opdl: add PMD main body and helper
> function")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Pallantla Poornima <pallantlax.poornima@intel.com>
> ---
>  drivers/event/opdl/opdl_evdev.c        | 7 ++++---
>  drivers/event/opdl/opdl_evdev_xstats.c | 7 +++----
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/event/opdl/opdl_evdev.c
> b/drivers/event/opdl/opdl_evdev.c
> index a4f0bc8b6..d2d2be44b 100644
> --- a/drivers/event/opdl/opdl_evdev.c
> +++ b/drivers/event/opdl/opdl_evdev.c
> @@ -422,16 +422,17 @@ opdl_dump(struct rte_eventdev *dev, FILE *f)
>  			else
>  				p_type = "????";
>  
> -			sprintf(queue_id, "%02u", port->external_qid);
> +			snprintf(queue_id, sizeof(queue_id), "%02u",
> +					port->external_qid);

Use more safer rte_strlcpy() function. Please introduce the marco for
queue_id size(currently it set to 64) and use it for queue_id
declaration and here.
 


>  			if (port->p_type == OPDL_REGULAR_PORT ||
>  					port->p_type ==
> OPDL_ASYNC_PORT)
> -				sprintf(total_cyc,
> +				snprintf(total_cyc, sizeof(total_cyc),

Use more safer rte_strlcpy() function. Please introduce the marco for
total_cyc size(currently it set to 64) and use it for total_cyc
declaration and here.



>  					" %'16"PRIu64"",
>  					(cpg != 0 ?
>  					 port->port_stat[total_cycles]
> / cpg
>  					 : 0));
>  			else
> -				sprintf(total_cyc,
> +				snprintf(total_cyc, sizeof(total_cyc),
>  					"             ----");
>  			fprintf(f,
>  				"%4s %10u %8u %9s %'16"PRIu64"
> %'16"PRIu64" %s "
> diff --git a/drivers/event/opdl/opdl_evdev_xstats.c
> b/drivers/event/opdl/opdl_evdev_xstats.c
> index 0e6c6bd5e..27b3d8802 100644
> --- a/drivers/event/opdl/opdl_evdev_xstats.c
> +++ b/drivers/event/opdl/opdl_evdev_xstats.c
> @@ -32,10 +32,9 @@ opdl_xstats_init(struct rte_eventdev *dev)
>  			uint32_t index = (i * max_num_port_xstat) + j;
>  
>  			/* Name */
> -			sprintf(device->port_xstat[index].stat.name,
> -			       "port_%02u_%s",
> -			       i,
> -			       port_xstat_str[j]);
> +			snprintf(device->port_xstat[index].stat.name,
> +				sizeof(device-
> >port_xstat[index].stat.name),

Same as above. Use RTE_EVENT_DEV_XSTATS_NAME_SIZE for size.

> +				"port_%02u_%s", i, port_xstat_str[j]);
>  
>  			/* ID */
>  			device->port_xstat[index].id = index;
Jerin Jacob Kollanukkaran March 11, 2019, 1:52 p.m. | #2
On Mon, 2019-03-11 at 06:51 +0000, Jerin Jacob Kollanukkaran wrote:
> On Mon, 2019-02-04 at 07:18 +0000, Pallantla Poornima wrote:
> > 
> > %'16"PRIu64" %s "
> > diff --git a/drivers/event/opdl/opdl_evdev_xstats.c
> > b/drivers/event/opdl/opdl_evdev_xstats.c
> > index 0e6c6bd5e..27b3d8802 100644
> > --- a/drivers/event/opdl/opdl_evdev_xstats.c
> > +++ b/drivers/event/opdl/opdl_evdev_xstats.c
> > @@ -32,10 +32,9 @@ opdl_xstats_init(struct rte_eventdev *dev)
> >  			uint32_t index = (i * max_num_port_xstat) + j;
> >  
> >  			/* Name */
> > -			sprintf(device->port_xstat[index].stat.name,
> > -			       "port_%02u_%s",
> > -			       i,
> > -			       port_xstat_str[j]);
> > +			snprintf(device->port_xstat[index].stat.name,
> > +				sizeof(device-
> > > port_xstat[index].stat.name),
> 
> Same as above. Use RTE_EVENT_DEV_XSTATS_NAME_SIZE for size.

I overlooked the code. This code looks good to me.
If there is no comment for Liang Ma then i will merge this patch as is.
Jerin Jacob Kollanukkaran March 12, 2019, 8:39 a.m. | #3
On Mon, 2019-03-11 at 13:52 +0000, Jerin Jacob Kollanukkaran wrote:
> On Mon, 2019-03-11 at 06:51 +0000, Jerin Jacob Kollanukkaran wrote:
> > On Mon, 2019-02-04 at 07:18 +0000, Pallantla Poornima wrote:
> > > %'16"PRIu64" %s "
> > > diff --git a/drivers/event/opdl/opdl_evdev_xstats.c
> > > b/drivers/event/opdl/opdl_evdev_xstats.c
> > > index 0e6c6bd5e..27b3d8802 100644
> > > --- a/drivers/event/opdl/opdl_evdev_xstats.c
> > > +++ b/drivers/event/opdl/opdl_evdev_xstats.c
> > > @@ -32,10 +32,9 @@ opdl_xstats_init(struct rte_eventdev *dev)
> > >  			uint32_t index = (i * max_num_port_xstat) + j;
> > >  
> > >  			/* Name */
> > > -			sprintf(device->port_xstat[index].stat.name,
> > > -			       "port_%02u_%s",
> > > -			       i,
> > > -			       port_xstat_str[j]);
> > > +			snprintf(device->port_xstat[index].stat.name,
> > > +				sizeof(device-
> > > > port_xstat[index].stat.name),
> > 
> > Same as above. Use RTE_EVENT_DEV_XSTATS_NAME_SIZE for size.
> 
> I overlooked the code. This code looks good to me.
> If there is no comment from Liang Ma then i will merge this patch as
> is.


Applied to dpdk-next-eventdev/master. Thanks.


>

Patch

diff --git a/drivers/event/opdl/opdl_evdev.c b/drivers/event/opdl/opdl_evdev.c
index a4f0bc8b6..d2d2be44b 100644
--- a/drivers/event/opdl/opdl_evdev.c
+++ b/drivers/event/opdl/opdl_evdev.c
@@ -422,16 +422,17 @@  opdl_dump(struct rte_eventdev *dev, FILE *f)
 			else
 				p_type = "????";
 
-			sprintf(queue_id, "%02u", port->external_qid);
+			snprintf(queue_id, sizeof(queue_id), "%02u",
+					port->external_qid);
 			if (port->p_type == OPDL_REGULAR_PORT ||
 					port->p_type == OPDL_ASYNC_PORT)
-				sprintf(total_cyc,
+				snprintf(total_cyc, sizeof(total_cyc),
 					" %'16"PRIu64"",
 					(cpg != 0 ?
 					 port->port_stat[total_cycles] / cpg
 					 : 0));
 			else
-				sprintf(total_cyc,
+				snprintf(total_cyc, sizeof(total_cyc),
 					"             ----");
 			fprintf(f,
 				"%4s %10u %8u %9s %'16"PRIu64" %'16"PRIu64" %s "
diff --git a/drivers/event/opdl/opdl_evdev_xstats.c b/drivers/event/opdl/opdl_evdev_xstats.c
index 0e6c6bd5e..27b3d8802 100644
--- a/drivers/event/opdl/opdl_evdev_xstats.c
+++ b/drivers/event/opdl/opdl_evdev_xstats.c
@@ -32,10 +32,9 @@  opdl_xstats_init(struct rte_eventdev *dev)
 			uint32_t index = (i * max_num_port_xstat) + j;
 
 			/* Name */
-			sprintf(device->port_xstat[index].stat.name,
-			       "port_%02u_%s",
-			       i,
-			       port_xstat_str[j]);
+			snprintf(device->port_xstat[index].stat.name,
+				sizeof(device->port_xstat[index].stat.name),
+				"port_%02u_%s", i, port_xstat_str[j]);
 
 			/* ID */
 			device->port_xstat[index].id = index;