app/testpmd: fix copying the name of the dynflag
Checks
Commit Message
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
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.
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.
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.
@@ -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) {