On 23/03/2021 2:21, Jason Gunthorpe wrote: > 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@xxxxxxxxxxxxxx/ >>> >>> 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. You're right, will update the patch. Thanks Jason
