[PATCH] staging: qlge/qlge_ethtool.c: strlcpy -> strscpy

Greg Kroah-Hartman gregkh at linuxfoundation.org
Fri Jan 29 09:12:11 UTC 2021


On Fri, Jan 29, 2021 at 02:38:33PM +0530, Kumar Kartikeya Dwivedi wrote:
> On 0129, Greg Kroah-Hartman wrote:
> > On Fri, Jan 29, 2021 at 01:51:55PM +0530, Kumar Kartikeya Dwivedi wrote:
> > > [Forgot to reply-all]
> > > 
> > > On 0129, Greg Kroah-Hartman wrote:
> > > > On Fri, Jan 29, 2021 at 12:15:23PM +0530, Kumar Kartikeya Dwivedi wrote:
> > > > > Fixes checkpatch warnings for usage of strlcpy.
> > > > 
> > > > What warning would that be?
> > > > 
> > > 
> > > 5dbdb2d87c294401a22e6a6002f08ebc9fbea38b
> > > 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5dbdb2d87c294401a22e6a6002f08ebc9fbea38b
> > 
> > Hint, in the future, write the above as: 5dbdb2d87c29 ("checkpatch:
> > prefer strscpy to strlcpy").  The documentation has examples of how to
> > do this easily.
> > 
> > And yes, I know that checkpatch says that, but I need to know how you
> > know this is the correct change.
> > 
> > > > And if we could just search/replace for this, why hasn't that already
> > > > happened for the whole tree?
> > > >
> > > 
> > > I think that's because it is hard to tell whether truncation is expected at the
> > > call site or not, so each change needs to be audited manually (to check the
> > > return value or not). In cases where it's just a safe strcpy, strscpy is a
> > > relatively better choice (due to not reading the entire source string).
> > 
> > Did you do that auditing?  I need to know that you did and that this is
> > fine, or that maybe, this isn't needed at all?  All of that information
> > needs to go in the changelog.
> 
> Yes, because it's copying the source strings to fixed size buffers in
> ethtool_drvinfo, so truncation would be fine here (as it's the driver name and
> other identity related stuff).

So there is no need to make this change, or it is required to make this
change?  I can't tell from your response here.

> Should I send a v2 with the reason?

I've already rejected the first one as being incomplete :)

thanks,

greg k-h


More information about the devel mailing list