diff mbox series

[28/95] string.h: add FORTIFY coverage for strscpy()

Message ID 20201216044350.TKAL6BULW%akpm@linux-foundation.org (mailing list archive)
State New, archived
Headers show
Series [01/95] mm: fix a race on nr_swap_pages | expand

Commit Message

Andrew Morton Dec. 16, 2020, 4:43 a.m. UTC
From: Francis Laniel <laniel_francis@privacyrequired.com>
Subject: string.h: add FORTIFY coverage for strscpy()

The fortified version of strscpy ensures the following before vanilla strscpy
is called:

1. There is no read overflow because we either size is smaller than
   src length or we shrink size to src length by calling fortified
   strnlen.

2. There is no write overflow because we either failed during
   compilation or at runtime by checking that size is smaller than dest
   size.

Link: https://lkml.kernel.org/r/20201122162451.27551-4-laniel_francis@privacyrequired.com
Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
Acked-by: Kees Cook <keescook@chromium.org>
Cc: Daniel Axtens <dja@axtens.net>
Cc: Daniel Micay <danielmicay@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/linux/string.h |   48 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

Comments

Linus Torvalds Dec. 16, 2020, 7:26 a.m. UTC | #1
On Tue, Dec 15, 2020 at 8:43 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> From: Francis Laniel <laniel_francis@privacyrequired.com>
> Subject: string.h: add FORTIFY coverage for strscpy()
>
> The fortified version of strscpy ensures the following [..]

So I've taken this, because it follows the pattern of what we already
have, but I have to say that the "fortify" stuff is now about half of
string.h.

I think it should probably be split out into its own header file, and
then string.h could just do

  #if !defined(__NO_FORTIFY) && defined(__OPTIMIZE__) &&
defined(CONFIG_FORTIFY_SOURCE)
     #include <linux/fortify-string.h>
  #endif

or similar.

That won't help my merge window build times (I build with
allmodconfig, so I'll have CONFIG_FORTIFY_SOURCE=y and so my poor
compiler will end up always going off and seeing all that every time
somebody glances at string.h), but I think it would be a cleanup
regardless.

And people who _don't_ build with FORTIFY_SOURCE wouldn't have to read
and parse that big chunk every time <linux/string.h> gets included.

(And yes, just reading the source file and tokenizing it - even if
it's all inside an #ifdef that turns off all the other phases - is
actually quite noticeable overhead in a C compiler. Surprisingly so)

           Linus
diff mbox series

Patch

--- a/include/linux/string.h~stringh-add-fortify-coverage-for-strscpy
+++ a/include/linux/string.h
@@ -6,6 +6,7 @@ 
 #include <linux/compiler.h>	/* for inline */
 #include <linux/types.h>	/* for size_t */
 #include <linux/stddef.h>	/* for NULL */
+#include <linux/errno.h>	/* for E2BIG */
 #include <stdarg.h>
 #include <uapi/linux/string.h>
 
@@ -357,6 +358,53 @@  __FORTIFY_INLINE size_t strlcpy(char *p,
 	return ret;
 }
 
+/* defined after fortified strnlen to reuse it */
+extern ssize_t __real_strscpy(char *, const char *, size_t) __RENAME(strscpy);
+__FORTIFY_INLINE ssize_t strscpy(char *p, const char *q, size_t size)
+{
+	size_t len;
+	/* Use string size rather than possible enclosing struct size. */
+	size_t p_size = __builtin_object_size(p, 1);
+	size_t q_size = __builtin_object_size(q, 1);
+
+	/* If we cannot get size of p and q default to call strscpy. */
+	if (p_size == (size_t) -1 && q_size == (size_t) -1)
+		return __real_strscpy(p, q, size);
+
+	/*
+	 * If size can be known at compile time and is greater than
+	 * p_size, generate a compile time write overflow error.
+	 */
+	if (__builtin_constant_p(size) && size > p_size)
+		__write_overflow();
+
+	/*
+	 * This call protects from read overflow, because len will default to q
+	 * length if it smaller than size.
+	 */
+	len = strnlen(q, size);
+	/*
+	 * If len equals size, we will copy only size bytes which leads to
+	 * -E2BIG being returned.
+	 * Otherwise we will copy len + 1 because of the final '\O'.
+	 */
+	len = len == size ? size : len + 1;
+
+	/*
+	 * Generate a runtime write overflow error if len is greater than
+	 * p_size.
+	 */
+	if (len > p_size)
+		fortify_panic(__func__);
+
+	/*
+	 * We can now safely call vanilla strscpy because we are protected from:
+	 * 1. Read overflow thanks to call to strnlen().
+	 * 2. Write overflow thanks to above ifs.
+	 */
+	return __real_strscpy(p, q, len);
+}
+
 /* defined after fortified strlen and strnlen to reuse them */
 __FORTIFY_INLINE char *strncat(char *p, const char *q, __kernel_size_t count)
 {