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

Kumar Kartikeya Dwivedi memxor at gmail.com
Fri Jan 29 09:25:43 UTC 2021


On 0129, Greg Kroah-Hartman wrote:
> [SNIP] 
> > 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.
> 

It's marked as deprecated in Documentation/process/deprecated.rst, is
inefficient, so its usage should be discouraged (and eventually it can be
dropped from the tree). So yes, this change makes sense. When the return value
isn't being checked (the caller is ok with truncation), there is no functional
difference between the two, in addition strscpy avoids a useless strlen call
internally.  strncpy is not a good candidate (as it doesn't guarantee NUL
termination), and strcpy is broken anyway. 

strlcpy is a dangerous API (atleast in kernel context), and introduces cognitive
overhead on the developer where they need to remember whether the source will
be valid all the time. strscpy instead only reads up to count bytes.

When the return value is being checked, it needs to be adpated to the -E2BIG
negative return code instead. In this case though, that isn't required.

> > Should I send a v2 with the reason?
> 
> I've already rejected the first one as being incomplete :)
> 
> thanks,
> 
> greg k-h

-- 
Kartikeya


More information about the devel mailing list