On Tue, Jan 05, 2021 at 01:28:18AM -0800, Joe Perches wrote:

On Tue, 2021-01-05 at 14:29 +0530, Dwaipayan Ray wrote:

quoted

On Tue, Jan 5, 2021 at 2:14 PM Joe Perches [off-list ref] wrote:

quoted

On Tue, 2021-01-05 at 13:53 +0530, Dwaipayan Ray wrote:

quoted

strcpy() performs no bounds checking on the destination buffer.
This could result in linear overflows beyond the end of the buffer.

strlcpy() reads the entire source buffer first. This read
may exceed the destination size limit. This can be both inefficient
and lead to linear read overflows.

The safe replacement to both of these is to use strscpy() instead.
Add a new checkpatch warning which alerts the user on finding usage of
strcpy() or strlcpy().

I do not believe that strscpy is preferred over strcpy.

When the size of the output buffer is known to be larger
than the input, strcpy is faster.

There are about 2k uses of strcpy.
Is there a use where strcpy use actually matters?
I don't know offhand...

But I believe compilers do not optimize away the uses of strscpy
to a simple memcpy like they do for strcpy with a const from

        strcpy(foo, "bar");

Yes the optimization here definitely helps. So in case the programmer
knows that the destination buffer is always larger, then strcpy() should be
preferred? I think the documentation might have been too strict about
strcpy() uses here:

Documentation/process/deprecated.rst:
"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(),..."

Kees/Jonathan:

Perhaps this text is overly restrictive.

There are ~2k uses of strcpy in the kernel.

About half of these are where the buffer length of foo is known and the
use is 'strcpy(foo, "bar")' so the compiler converts/optimizes away the
strcpy to memcpy and may not even put "bar" into the string table.

I believe strscpy uses do not have this optimization.

Is there a case where the runtime costs actually matters?
I expect so.

The original goal was to use another helper that worked on static
strings like this. Linus rejected that idea, so we're in a weird place.
I think we could perhaps build a strcpy() replacement that requires
compile-time validated arguments, and to break the build if not.

i.e.

given:
	char array[8];
	char *ptr;

allow:


	strcpy(array, "1234567");

disallow:

	strcpy(array, "12345678");	/* too long */
	strcpy(array, src);		/* not optimized, so use strscpy? */
	strcpy(ptr, "1234567");		/* unknown destination size */
	strcpy(ptr, src);		/* unknown destination size */

What do you think?

-- 
Kees Cook
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees