app/testpmd: fix copying the name of the dynflag
diff mbox series

Message ID 1580390564-186435-1-git-send-email-orika@mellanox.com
State Superseded, archived
Delegated to: Ferruh Yigit
Headers show
Series
  • app/testpmd: fix copying the name of the dynflag
Related show

Checks

Context Check Description
ci/Intel-compilation fail apply issues
ci/iol-testing fail Testing issues
ci/travis-robot success Travis build: passed
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/checkpatch success coding style OK

Commit Message

Ori Kam Jan. 30, 2020, 1:22 p.m. UTC
When working with testpmd and setting the dynflag name, we copy the
name given by the cmd to the dynflag name.

The issue is that the size of the dynflag name is smaller then the
string used by testpmd.

This commit solves this issue by usign strncpy.

Coverity issue: 353610

Fixes: b57b66a97ebf ("app/testpmd: support mbuf dynamic flag")

Signed-off-by: Ori Kam <orika@mellanox.com>
---
 app/test-pmd/cmdline.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Iremonger, Bernard Jan. 30, 2020, 4:54 p.m. UTC | #1
Hi Ori,

> -----Original Message-----
> From: Ori Kam <orika@mellanox.com>
> Sent: Thursday, January 30, 2020 1:23 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; orika@mellanox.com; Yigit, Ferruh
> <ferruh.yigit@intel.com>; viacheslavo@mellanox.com
> Subject: [PATCH] app/testpmd: fix copying the name of the dynflag
> 
> When working with testpmd and setting the dynflag name, we copy the
> name given by the cmd to the dynflag name.
> 
> The issue is that the size of the dynflag name is smaller then the string used
> by testpmd.
> 
> This commit solves this issue by usign strncpy.
> 
> Coverity issue: 353610
> 
> Fixes: b57b66a97ebf ("app/testpmd: support mbuf dynamic flag")
> 
> Signed-off-by: Ori Kam <orika@mellanox.com>
> ---
>  app/test-pmd/cmdline.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> dab22bc..09429f9 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -18867,7 +18867,7 @@ struct cmd_config_tx_dynf_specific_result {
>  		return;
>  	flag = rte_mbuf_dynflag_lookup(res->name, NULL);
>  	if (flag <= 0) {
> -		strcpy(desc_flag.name, res->name);
> +		strncpy(desc_flag.name, res->name,
> sizeof(*desc_flag.name));

This does not look correct to me.
Should it be sizeof(desc_flag.name[RTE_MBUF_DYN_NAMESIZE]));

>  		desc_flag.flags = 0;
>  		flag = rte_mbuf_dynflag_register(&desc_flag);
>  		if (flag < 0) {
> --
> 1.8.3.1

Regards,

Bernard.
Ori Kam Jan. 30, 2020, 6:55 p.m. UTC | #2
Hi Bernard,

Thanks for review, PSB

Ori

> -----Original Message-----
> From: Iremonger, Bernard <bernard.iremonger@intel.com>
> Sent: Thursday, January 30, 2020 6:54 PM
> To: Ori Kam <orika@mellanox.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>
> Subject: RE: [PATCH] app/testpmd: fix copying the name of the dynflag
> 
> Hi Ori,
> 
> > -----Original Message-----
> > From: Ori Kam <orika@mellanox.com>
> > Sent: Thursday, January 30, 2020 1:23 PM
> > To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; Iremonger, Bernard
> > <bernard.iremonger@intel.com>
> > Cc: dev@dpdk.org; orika@mellanox.com; Yigit, Ferruh
> > <ferruh.yigit@intel.com>; viacheslavo@mellanox.com
> > Subject: [PATCH] app/testpmd: fix copying the name of the dynflag
> >
> > When working with testpmd and setting the dynflag name, we copy the
> > name given by the cmd to the dynflag name.
> >
> > The issue is that the size of the dynflag name is smaller then the string used
> > by testpmd.
> >
> > This commit solves this issue by usign strncpy.
> >
> > Coverity issue: 353610
> >
> > Fixes: b57b66a97ebf ("app/testpmd: support mbuf dynamic flag")
> >
> > Signed-off-by: Ori Kam <orika@mellanox.com>
> > ---
> >  app/test-pmd/cmdline.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > dab22bc..09429f9 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -18867,7 +18867,7 @@ struct cmd_config_tx_dynf_specific_result {
> >  		return;
> >  	flag = rte_mbuf_dynflag_lookup(res->name, NULL);
> >  	if (flag <= 0) {
> > -		strcpy(desc_flag.name, res->name);
> > +		strncpy(desc_flag.name, res->name,
> > sizeof(*desc_flag.name));
> 
> This does not look correct to me.
> Should it be sizeof(desc_flag.name[RTE_MBUF_DYN_NAMESIZE]));
>

You are correct, I will send a new version.
 
> >  		desc_flag.flags = 0;
> >  		flag = rte_mbuf_dynflag_register(&desc_flag);
> >  		if (flag < 0) {
> > --
> > 1.8.3.1
> 
> Regards,
> 
> Bernard.
Ori Kam Jan. 30, 2020, 7:09 p.m. UTC | #3
Hi Bernard,

> -----Original Message-----
> From: Ori Kam
> Sent: Thursday, January 30, 2020 8:56 PM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>
> Subject: RE: [PATCH] app/testpmd: fix copying the name of the dynflag
> 
> Hi Bernard,
> 
> Thanks for review, PSB
> 
> Ori
> 
> > -----Original Message-----
> > From: Iremonger, Bernard <bernard.iremonger@intel.com>
> > Sent: Thursday, January 30, 2020 6:54 PM
> > To: Ori Kam <orika@mellanox.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> > Wu, Jingjing <jingjing.wu@intel.com>
> > Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; Slava Ovsiienko
> > <viacheslavo@mellanox.com>
> > Subject: RE: [PATCH] app/testpmd: fix copying the name of the dynflag
> >
> > Hi Ori,
> >
> > > -----Original Message-----
> > > From: Ori Kam <orika@mellanox.com>
> > > Sent: Thursday, January 30, 2020 1:23 PM
> > > To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> > > <jingjing.wu@intel.com>; Iremonger, Bernard
> > > <bernard.iremonger@intel.com>
> > > Cc: dev@dpdk.org; orika@mellanox.com; Yigit, Ferruh
> > > <ferruh.yigit@intel.com>; viacheslavo@mellanox.com
> > > Subject: [PATCH] app/testpmd: fix copying the name of the dynflag
> > >
> > > When working with testpmd and setting the dynflag name, we copy the
> > > name given by the cmd to the dynflag name.
> > >
> > > The issue is that the size of the dynflag name is smaller then the string used
> > > by testpmd.
> > >
> > > This commit solves this issue by usign strncpy.
> > >
> > > Coverity issue: 353610
> > >
> > > Fixes: b57b66a97ebf ("app/testpmd: support mbuf dynamic flag")
> > >
> > > Signed-off-by: Ori Kam <orika@mellanox.com>
> > > ---
> > >  app/test-pmd/cmdline.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > > dab22bc..09429f9 100644
> > > --- a/app/test-pmd/cmdline.c
> > > +++ b/app/test-pmd/cmdline.c
> > > @@ -18867,7 +18867,7 @@ struct cmd_config_tx_dynf_specific_result {
> > >  		return;
> > >  	flag = rte_mbuf_dynflag_lookup(res->name, NULL);
> > >  	if (flag <= 0) {
> > > -		strcpy(desc_flag.name, res->name);
> > > +		strncpy(desc_flag.name, res->name,
> > > sizeof(*desc_flag.name));
> >
> > This does not look correct to me.
> > Should it be sizeof(desc_flag.name[RTE_MBUF_DYN_NAMESIZE]));
> >
> 
> You are correct, I will send a new version.
> 
I wasn't  clear I will fix the code but the fix is not like you suggested.
Your fix also doesn't work the return size will be 1.
The solution is sizeof(desc_flag.name)

> > >  		desc_flag.flags = 0;
> > >  		flag = rte_mbuf_dynflag_register(&desc_flag);
> > >  		if (flag < 0) {
> > > --
> > > 1.8.3.1
> >
> > Regards,
> >
> > Bernard.

Patch
diff mbox series

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index dab22bc..09429f9 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -18867,7 +18867,7 @@  struct cmd_config_tx_dynf_specific_result {
 		return;
 	flag = rte_mbuf_dynflag_lookup(res->name, NULL);
 	if (flag <= 0) {
-		strcpy(desc_flag.name, res->name);
+		strncpy(desc_flag.name, res->name, sizeof(*desc_flag.name));
 		desc_flag.flags = 0;
 		flag = rte_mbuf_dynflag_register(&desc_flag);
 		if (flag < 0) {