diff mbox series

app/testpmd: fix copying the name of the dynflag

Message ID 1580390564-186435-1-git-send-email-orika@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series app/testpmd: fix copying the name of the dynflag |

Checks

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

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.
diff mbox series

Patch

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) {