RE: [EXTERNAL] [PATCH v2] app/graph: fix destination buffer too small

[Kiran Kumar Kokkilagadda <kirankumark@marvell.com>]

> -----Original Message-----
> From: Mahmoud Matook <mahmoudmatook.mm@gmail.com>
> Sent: Thursday, June 27, 2024 1:49 AM
> To: Kiran Kumar Kokkilagadda <kirankumark@marvell.com>
> Cc: Sunil Kumar Kori <skori@marvell.com>; Rakesh Kudurumalla
> <rkudurumalla@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>; Jerin Jacob <jerinj@marvell.com>;
> dev@dpdk.org
> Subject: Re: [EXTERNAL] [PATCH v2] app/graph: fix destination buffer too
> small
> 
> On 06/25, Kiran Kumar Kokkilagadda wrote: > > > From: Mahmoud Maatuq
> <mahmoudmatook. mm@ gmail. com> > Sent: Tuesday, June 25, 2024 1: 31
> AM > To: Sunil Kumar Kori <skori@ marvell. com>; Rakesh Kudurumalla
> <rkudurumalla@ marvell. com>; 
> On 06/25, Kiran Kumar Kokkilagadda wrote:
> 
> >
> >
> > From: Mahmoud Maatuq <mahmoudmatook.mm@gmail.com>
> > Sent: Tuesday, June 25, 2024 1:31 AM
> > To: Sunil Kumar Kori <skori@marvell.com>; Rakesh Kudurumalla
> > <rkudurumalla@marvell.com>; Nithin Kumar Dabilpuram
> > <ndabilpuram@marvell.com>; Jerin Jacob <jerinj@marvell.com>
> > Cc: dev@dpdk.org; Mahmoud Maatuq
> <mahmoudmatook.mm@gmail.com>
> > Subject: [EXTERNAL] [PATCH v2] app/graph: fix destination buffer too
> > small
> >
> > as sizeof(config. rx. mempool_name) is < sizeof(res->mempool) we
> > should copy at most sizeof(config. rx. mempool_name) and replace memcpy
> with strlcpy as mempool name is a null terminated string Coverity issue:
> 415430 Fixes: 3850cb06ab9c ("app/graph:
> >
> >
> > as sizeof(config.rx.mempool_name) is < sizeof(res->mempool) we should
> >
> > copy at most sizeof(config.rx.mempool_name) and replace memcpy with
> >
> > strlcpy as mempool name is a null terminated string
> >
> >
> >
> > Coverity issue: 415430
> >
> > Fixes: 3850cb06ab9c ("app/graph: add ethdev commands")
> >
> > Cc: skori@marvell.com<mailto:skori@marvell.com>
> >
> >
> >
> > Signed-off-by: Mahmoud Maatuq
> >
> <mahmoudmatook.mm@gmail.com<mailto:mahmoudmatook.mm@gmail.co
> m>>
> >
> > ---

Acked-by: Kiran Kumar Kokkilagadda <kirankumark@marvell.com>


> >
> > v2:
> >
> > * replaced memcpy with strlcpy
> >
> > ---
> >
> >  app/graph/ethdev.c | 3 ++-
> >
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >
> >
> > diff --git a/app/graph/ethdev.c b/app/graph/ethdev.c
> >
> > index cfc1b18569..e7a02b40a9 100644
> >
> > --- a/app/graph/ethdev.c
> >
> > +++ b/app/graph/ethdev.c
> >
> > @@ -16,6 +16,7 @@
> >
> >
> >
> >  #include "ethdev_priv.h"
> >
> >  #include "module_api.h"
> >
> > +#include "rte_string_fns.h"
> >
> >
> >
> >  static const char
> >
> >  cmd_ethdev_mtu_help[] = "ethdev <ethdev_name> mtu <mtu_sz>";
> >
> > @@ -671,7 +672,7 @@ cmd_ethdev_parsed(void *parsed_result,
> > __rte_unused struct cmdline *cl, void *da
> >
> >               memset(&config, 0, sizeof(struct ethdev_config));
> >
> >               config.rx.n_queues = res->nb_rxq;
> >
> >               config.rx.queue_size = ETHDEV_RX_DESC_DEFAULT;
> >
> > -            memcpy(config.rx.mempool_name, res->mempool, strlen(res-
> >mempool));
> >
> > +           strlcpy(config.rx.mempool_name, res->mempool,
> > + sizeof(config.rx.mempool_name));
> >
> >
> >
> > Can be changed to strlcpy(config.rx.mempool_name, res->mempool-
> >name, sizeof(config.rx.mempool_name)); ?
> 
> mempool field is of type cmdline_fixed_string_t (array of char)
> 

Ok, I over looked the "res" type. 

> >
> >
> >
> >               config.tx.n_queues = res->nb_txq;
> >
> >               config.tx.queue_size = ETHDEV_TX_DESC_DEFAULT;
> >
> > --
> >
> > 2.43.0
> >
> >