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); 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; -- 2.17.2
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;
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.
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. >