Re: [PATCH] drivers/input: Remove all strcpy() uses in favor of strscpy()

On Sun, Aug 01, 2021 at 09:55:28AM -0700, Joe Perches wrote:

On Sun, 2021-08-01 at 09:39 -0700, Joe Perches wrote:

quoted

On Sun, 2021-08-01 at 16:00 +0100, Russell King (Oracle) wrote:

quoted

On Sun, Aug 01, 2021 at 04:43:16PM +0200, Len Baker wrote:

quoted

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. The safe replacement is strscpy().

[]

quoted

quoted

So if the string doesn't fit, it's fine to silently truncate it?

Rather than converting every single strcpy() in the kernel to
strscpy(), maybe there should be some consideration given to how the
issue of a strcpy() that overflows the buffer should be handled.
E.g. in the case of a known string such as the above, if it's longer
than the destination, should we find a way to make the compiler issue
a warning at compile time?

(apologies for the earlier blank reply, sometimes I dislike my email client)

stracpy could do that with a trivial addition like below:

Old lkml references:

https://lore.kernel.org/lkml/cover.1563889130.git.joe@perches.com/ (local)
and
https://lore.kernel.org/lkml/56dc4de7e0db153cb10954ac251cb6c27c33da4a.camel@perches.com/ (local)

But Linus T wants a copy_string mechanism instead:
https://lore.kernel.org/lkml/CAHk-=wgqQKoAnhmhGE-2PBFt7oQs9LLAATKbYa573UO=DPBE0Q@mail.gmail.com/ (local)

/**
 * stracpy - Copy a C-string into an array of char/u8/s8 or equivalent
 * @dest: Where to copy the string, must be an array of char and not a pointer
 * @src: String to copy, may be a pointer or const char array
 *
 * Helper for strscpy().
 * Copies a maximum of sizeof(@dest) bytes of @src with %NUL termination.
 *
 * A BUILD_BUG_ON is used for cases where @dest is not a char array or
 * @src is a char array and is larger than @dest.
 *
 * Returns:
 * * The number of characters copied (not including the trailing %NUL)
 * * -E2BIG if @dest is a zero size array or @src was truncated.
 */
#define stracpy(dest, src)						\
({									\
	BUILD_BUG_ON(!(__same_type(dest, char[]) ||			\
		       __same_type(dest, unsigned char[]) ||		\
		       __same_type(dest, signed char[])));		\
	BUILD_BUG_ON((__same_type(src, char[]) ||			\
		      __same_type(src, unsigned char[]) ||		\
		      __same_type(src, signed char[])) &&		\
		     ARRAY_SIZE(src) > ARRAY_SIZE(dest));		\
									\
	strscpy(dest, src, ARRAY_SIZE(dest));				\
})

I'm wondering, instead, if we could convert strcpy() into this instead
of adding another API? I.e. convert all the places that warn (if this
were strcpy), and then land the conversion.

-- 
Kees Cook