RE: [PATCH v5 2/4] lib: fix comparison between devices

[Shani Peretz <shperetz@nvidia.com>]

Hey,
Sorry for my late response, I sent a fix (v7):
https://patches.dpdk.org/project/dpdk/patch/20250212163836.178976-2-shperetz@nvidia.com/

I added another parameter to the parse function - the size of the memory pointed by addr. 
so the function signature now is:
int XXX_parse(const char *name, void *addr, int addr_size, int *out_size)
So I now use it in rte_strscpy.

In addition, should I replace the call to rte_strscpy with strlcpy?


> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, 11 February 2025 20:05
> To: Bruce Richardson <bruce.richardson@intel.com>
> Cc: Shani Peretz <shperetz@nvidia.com>; dev@dpdk.org; Parav Pandit
> <parav@nvidia.com>; Xueming Li <xuemingl@nvidia.com>; Nipun Gupta
> <nipun.gupta@amd.com>; Nikhil Agarwal <nikhil.agarwal@amd.com>; Hemant
> Agrawal <hemant.agrawal@nxp.com>; Sachin Saxena
> <sachin.saxena@nxp.com>; Rosen Xu <rosen.xu@intel.com>; Chenbo Xia
> <chenbox@nvidia.com>; Tomasz Duszynski <tduszynski@marvell.com>;
> Chengwen Feng <fengchengwen@huawei.com>; NBU-Contact-longli
> (EXTERNAL) <longli@microsoft.com>; Wei Hu <weh@microsoft.com>; Kevin
> Laatz <kevin.laatz@intel.com>; Tyler Retzlaff <roretzla@linux.microsoft.com>;
> Jan Blunck <jblunck@infradead.org>
> Subject: Re: [PATCH v5 2/4] lib: fix comparison between devices
> 
> External email: Use caution opening links or attachments
> 
> 
> On Tue, 11 Feb 2025 17:54:26 +0000
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > On Tue, Feb 11, 2025 at 09:48:32AM -0800, Stephen Hemminger wrote:
> > > On Thu, 6 Feb 2025 02:08:36 +0200
> > > Shani Peretz <shperetz@nvidia.com> wrote:
> > >
> > > >  static int
> > > > -cdx_parse(const char *name, void *addr)
> > > > +cdx_parse(const char *name, void *addr, int *size)
> > > >  {
> > > > - const char **out = addr;
> > > >   int ret;
> > > >
> > > >   ret = strncmp(name, CDX_DEV_PREFIX, strlen(CDX_DEV_PREFIX));
> > > >
> > > > - if (ret == 0 && addr)
> > > > -         *out = name;
> > > > + if (ret != 0)
> > > > +         return ret;
> > > > +
> > > > + if (size != NULL)
> > > > +         *size = strlen(name) + 1;
> > > > +
> > > > + if (addr != NULL)
> > > > +         rte_strscpy(addr, name, strlen(name) + 1);
> > >
> > > Why use rte_strscpy() here?
> > >
> > > The intention of strscpy() is to handle case where the resulting
> > > buffer is limited in size. By using the input string length you
> > > aren't really doing anything different than strcpy(). Still unsafe if output
> (addr) is not big enough.
> >
> > And using strlcpy is probably fine too, without having to use
> > dpdk-specific string functions.
> >
> > /Bruce
> 
> The issue is that any length argument needs to come from caller based on the
> size of the target buffer. Not from length of source string.
> 
> If you want to make parse code string safe, then either size needs to be always
> present and in/out parameter or need to have a src_size and resulting size as
> separate params.