On Thu, Oct 15, 2020 at 04:17:31PM -0700, Kees Cook wrote: > The notes on replacing the deprecated str*cpy() functions didn't call > enough attention to the change in return type. Add these details and > clean up the language a bit more. > > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> Acked-by: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx> -- Gustavo > --- > Documentation/process/deprecated.rst | 44 ++++++++++++++++------------ > 1 file changed, 26 insertions(+), 18 deletions(-) > > diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst > index ff71d802b53d..9d83b8db8874 100644 > --- a/Documentation/process/deprecated.rst > +++ b/Documentation/process/deprecated.rst > @@ -106,23 +106,29 @@ NUL or newline terminated. > > strcpy() > -------- > -strcpy() performs no bounds checking on the destination > -buffer. This could result in linear overflows beyond the > -end of the buffer, leading to all kinds of misbehaviors. While > -`CONFIG_FORTIFY_SOURCE=y` and various compiler flags help reduce the > -risk of using this function, there is no good reason to add new uses of > -this function. The safe replacement is strscpy(). > +strcpy() performs no bounds checking on the destination buffer. This > +could result in linear overflows beyond the end of the buffer, leading to > +all kinds of misbehaviors. While `CONFIG_FORTIFY_SOURCE=y` and various > +compiler flags help reduce the risk of using this function, there is > +no good reason to add new uses of this function. The safe replacement > +is strscpy(), though care must be given to any cases where the return > +value of strcpy() was used, since strscpy() does not return a pointer to > +the destination, but rather a count of non-NUL bytes copied (or negative > +errno when it truncates). > > strncpy() on NUL-terminated strings > ----------------------------------- > -Use of strncpy() does not guarantee that the destination buffer > -will be NUL terminated. This can lead to various linear read overflows > -and other misbehavior due to the missing termination. It also NUL-pads the > -destination buffer if the source contents are shorter than the destination > -buffer size, which may be a needless performance penalty for callers using > -only NUL-terminated strings. The safe replacement is strscpy(). > -(Users of strscpy() still needing NUL-padding should instead > -use strscpy_pad().) > +Use of strncpy() does not guarantee that the destination buffer will > +be NUL terminated. This can lead to various linear read overflows and > +other misbehavior due to the missing termination. It also NUL-pads > +the destination buffer if the source contents are shorter than the > +destination buffer size, which may be a needless performance penalty > +for callers using only NUL-terminated strings. The safe replacement is > +strscpy(), though care must be given to any cases where the return value > +of strncpy() was used, since strscpy() does not return a pointer to the > +destination, but rather a count of non-NUL bytes copied (or negative > +errno when it truncates). Any cases still needing NUL-padding should > +instead use strscpy_pad(). > > If a caller is using non-NUL-terminated strings, strncpy() can > still be used, but destinations should be marked with the `__nonstring > @@ -131,10 +137,12 @@ attribute to avoid future compiler warnings. > > strlcpy() > --------- > -strlcpy() reads the entire source buffer first, possibly exceeding > -the given limit of bytes to copy. This is inefficient and can lead to > -linear read overflows if a source string is not NUL-terminated. The > -safe replacement is strscpy(). > +strlcpy() reads the entire source buffer first (since the return value > +is meant to match that of strlen()). This read may exceed the destination > +size limit. This is both inefficient and can lead to linear read overflows > +if a source string is not NUL-terminated. The safe replacement is strscpy(), > +though care must be given to any cases where the return value of strlcpy() > +is used, since strscpy() will return negative errno values when it truncates. > > %p format specifier > ------------------- > -- > 2.25.1 >
