Re: [PATCH for-next] RDMA/efa: Use strscpy instead of strlcpy

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Mar 22, 2021 at 07:14:35PM +0200, Gal Pressman wrote:
> On 22/03/2021 18:55, Jason Gunthorpe wrote:
> > On Mon, Mar 22, 2021 at 03:11:33PM +0200, Gal Pressman wrote:
> >>
> >> On 22/03/2021 15:01, Jason Gunthorpe wrote:
> >>> On Tue, Mar 16, 2021 at 03:24:16PM +0200, Gal Pressman wrote:
> >>>> The strlcpy function doesn't limit the source length, use the preferred
> >>>> strscpy function instead.
> >>>
> >>> Why do we need to limit the source length here? Either this is a bug
> >>> because the source string is no NULL terminated or it is OK as is?
> >>
> >> It's not a bug as is, but it addresses checkpatch's warning:
> >> WARNING: Prefer strscpy over strlcpy - see: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
> > 
> > Okay.. but why is it so weird:
> > 
> >         strscpy(hinf->kernel_ver_str, utsname()->version,
> >                 min(sizeof(hinf->kernel_ver_str), sizeof(utsname()->version)));
> > 
> > ?
> > 
> > utsname()->version is null terminated, yes? Why does it need to be
> > min'd?
> 
> The size of the kernel buffer is different than the device buffer (65B vs 32B),
> the min() is there to prevent overflow regardless of the NULL termination.
> A NULL terminated 60 bytes utsname would be truncated to 32 bytes.

I don't understand.

If version is NULL terminated than this:

   strscpy(hinf->kernel_ver_str, utsname()->version,
           sizeof(hinf->kernel_ver_str))

Is the only thing needed? The whole point of strscpy is that it
truncates the string to fit the output.

Jason