[RFC][PATCH v3 0/5] Fortify string function strscpy

[RFC][PATCH v3 0/5] Fortify string function strscpy

[laniel_francis]

From: Francis Laniel <laniel_francis@privacyrequired.com>

Hi.


I hope your families, friends and yourselves are fine.

This patch set answers to this issue:
https://github.com/KSPP/linux/issues/46

I based my modifications on top of two patches from Daniel Axtens which modify
calls to __builtin_object_size to ensure the true size of char * are returned
and not the surrounding structure size.

To sum up, in my first patch I implemented a fortified version of strscpy.
The second patch brings a new file in LKDTM driver to test this new version.
The test ensures the fortified version still returns the same value as the
vanilla one while panic'ing when there is a write overflow.
The third just corrects some typos in LKDTM related file.

If you see any problem or way to improve the code, feel free to share it.


Best regards.

Daniel Axtens (2):
  string.h: detect intra-object overflow in fortified string functions
  lkdtm: tests for FORTIFY_SOURCE

Francis Laniel (3):
  Fortify string function strscpy.
  Add new file in LKDTM to test fortified strscpy.
  Correct wrong filenames in comment.

 drivers/misc/lkdtm/Makefile             |  1 +
 drivers/misc/lkdtm/bugs.c               | 50 +++++++++++++++++++
 drivers/misc/lkdtm/core.c               |  3 ++
 drivers/misc/lkdtm/fortify.c            | 47 ++++++++++++++++++
 drivers/misc/lkdtm/lkdtm.h              | 19 +++++---
 include/linux/string.h                  | 65 ++++++++++++++++++++-----
 tools/testing/selftests/lkdtm/tests.txt |  1 +
 7 files changed, 168 insertions(+), 18 deletions(-)
 create mode 100644 drivers/misc/lkdtm/fortify.c

-- 
2.20.1

[PATCH v3 1/5] string.h: detect intra-object overflow in fortified string functions

[laniel_francis]

From: Daniel Axtens <dja@axtens.net>

When the fortify feature was first introduced in commit 6974f0c4555e
("include/linux/string.h: add the option of fortified string.h functions"),
Daniel Micay observed:

  * It should be possible to optionally use __builtin_object_size(x, 1) for
    some functions (C strings) to detect intra-object overflows (like
    glibc's _FORTIFY_SOURCE=2), but for now this takes the conservative
    approach to avoid likely compatibility issues.

This is a case that often cannot be caught by KASAN. Consider:

struct foo {
    char a[10];
    char b[10];
}

void test() {
    char *msg;
    struct foo foo;

    msg = kmalloc(16, GFP_KERNEL);
    strcpy(msg, "Hello world!!");
    // this copy overwrites foo.b
    strcpy(foo.a, msg);
}

The questionable copy overflows foo.a and writes to foo.b as well. It
cannot be detected by KASAN. Currently it is also not detected by fortify,
because strcpy considers __builtin_object_size(x, 0), which considers the
size of the surrounding object (here, struct foo). However, if we switch
the string functions over to use __builtin_object_size(x, 1), the compiler
will measure the size of the closest surrounding subobject (here, foo.a),
rather than the size of the surrounding object as a whole. See
https://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html for more info.

Only do this for string functions: we cannot use it on things like
memcpy, memmove, memcmp and memchr_inv due to code like this which
purposefully operates on multiple structure members:
(arch/x86/kernel/traps.c)

	/*
	 * regs->sp points to the failing IRET frame on the
	 * ESPFIX64 stack.  Copy it to the entry stack.  This fills
	 * in gpregs->ss through gpregs->ip.
	 *
	 */
	memmove(&gpregs->ip, (void *)regs->sp, 5*8);

This change passes an allyesconfig on powerpc and x86, and an x86 kernel
built with it survives running with syz-stress from syzkaller, so it seems
safe so far.

Cc: Daniel Micay <danielmicay@gmail.com>
Cc: Kees Cook <keescook@chromium.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 include/linux/string.h | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index b1f3894a0a3e..46e91d684c47 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -292,7 +292,7 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size)
 
 __FORTIFY_INLINE char *strncpy(char *p, const char *q, __kernel_size_t size)
 {
-	size_t p_size = __builtin_object_size(p, 0);
+	size_t p_size = __builtin_object_size(p, 1);
 	if (__builtin_constant_p(size) && p_size < size)
 		__write_overflow();
 	if (p_size < size)
@@ -302,7 +302,7 @@ __FORTIFY_INLINE char *strncpy(char *p, const char *q, __kernel_size_t size)
 
 __FORTIFY_INLINE char *strcat(char *p, const char *q)
 {
-	size_t p_size = __builtin_object_size(p, 0);
+	size_t p_size = __builtin_object_size(p, 1);
 	if (p_size == (size_t)-1)
 		return __underlying_strcat(p, q);
 	if (strlcat(p, q, p_size) >= p_size)
@@ -313,7 +313,7 @@ __FORTIFY_INLINE char *strcat(char *p, const char *q)
 __FORTIFY_INLINE __kernel_size_t strlen(const char *p)
 {
 	__kernel_size_t ret;
-	size_t p_size = __builtin_object_size(p, 0);
+	size_t p_size = __builtin_object_size(p, 1);
 
 	/* Work around gcc excess stack consumption issue */
 	if (p_size == (size_t)-1 ||
@@ -328,7 +328,7 @@ __FORTIFY_INLINE __kernel_size_t strlen(const char *p)
 extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(strnlen);
 __FORTIFY_INLINE __kernel_size_t strnlen(const char *p, __kernel_size_t maxlen)
 {
-	size_t p_size = __builtin_object_size(p, 0);
+	size_t p_size = __builtin_object_size(p, 1);
 	__kernel_size_t ret = __real_strnlen(p, maxlen < p_size ? maxlen : p_size);
 	if (p_size <= ret && maxlen != ret)
 		fortify_panic(__func__);
@@ -340,8 +340,8 @@ extern size_t __real_strlcpy(char *, const char *, size_t) __RENAME(strlcpy);
 __FORTIFY_INLINE size_t strlcpy(char *p, const char *q, size_t size)
 {
 	size_t ret;
-	size_t p_size = __builtin_object_size(p, 0);
-	size_t q_size = __builtin_object_size(q, 0);
+	size_t p_size = __builtin_object_size(p, 1);
+	size_t q_size = __builtin_object_size(q, 1);
 	if (p_size == (size_t)-1 && q_size == (size_t)-1)
 		return __real_strlcpy(p, q, size);
 	ret = strlen(q);
@@ -361,8 +361,8 @@ __FORTIFY_INLINE size_t strlcpy(char *p, const char *q, size_t size)
 __FORTIFY_INLINE char *strncat(char *p, const char *q, __kernel_size_t count)
 {
 	size_t p_len, copy_len;
-	size_t p_size = __builtin_object_size(p, 0);
-	size_t q_size = __builtin_object_size(q, 0);
+	size_t p_size = __builtin_object_size(p, 1);
+	size_t q_size = __builtin_object_size(q, 1);
 	if (p_size == (size_t)-1 && q_size == (size_t)-1)
 		return __underlying_strncat(p, q, count);
 	p_len = strlen(p);
@@ -475,11 +475,16 @@ __FORTIFY_INLINE void *kmemdup(const void *p, size_t size, gfp_t gfp)
 /* defined after fortified strlen and memcpy to reuse them */
 __FORTIFY_INLINE char *strcpy(char *p, const char *q)
 {
-	size_t p_size = __builtin_object_size(p, 0);
-	size_t q_size = __builtin_object_size(q, 0);
+	size_t p_size = __builtin_object_size(p, 1);
+	size_t q_size = __builtin_object_size(q, 1);
+	size_t size;
 	if (p_size == (size_t)-1 && q_size == (size_t)-1)
 		return __underlying_strcpy(p, q);
-	memcpy(p, q, strlen(q) + 1);
+	size = strlen(q) + 1;
+	/* test here to use the more stringent object size */
+	if (p_size < size)
+		fortify_panic(__func__);
+	memcpy(p, q, size);
 	return p;
 }
 
-- 
2.20.1

[PATCH v3 2/5] lkdtm: tests for FORTIFY_SOURCE

[laniel_francis]

From: Daniel Axtens <dja@axtens.net>

Add code to test both:

 - runtime detection of the overrun of a structure. This covers the
   __builtin_object_size(x, 0) case. This test is called FORTIFY_OBJECT.

 - runtime detection of the overrun of a char array within a structure.
   This covers the __builtin_object_size(x, 1) case which can be used
   for some string functions. This test is called FORTIFY_SUBOBJECT.

Suggested-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 drivers/misc/lkdtm/bugs.c  | 50 ++++++++++++++++++++++++++++++++++++++
 drivers/misc/lkdtm/core.c  |  2 ++
 drivers/misc/lkdtm/lkdtm.h |  2 ++
 3 files changed, 54 insertions(+)

diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
index 4dfbfd51bdf7..51cf65b28f36 100644
--- a/drivers/misc/lkdtm/bugs.c
+++ b/drivers/misc/lkdtm/bugs.c
@@ -492,3 +492,53 @@ noinline void lkdtm_CORRUPT_PAC(void)
 	pr_err("XFAIL: this test is arm64-only\n");
 #endif
 }
+
+void lkdtm_FORTIFY_OBJECT(void)
+{
+	struct target {
+		char a[10];
+	} target[2] = {};
+	int result;
+
+	/*
+	 * Using volatile prevents the compiler from determining the value of
+	 * 'size' at compile time. Without that, we would get a compile error
+	 * rather than a runtime error.
+	 */
+	volatile int size = 11;
+
+	pr_info("trying to read past the end of a struct\n");
+
+	result = memcmp(&target[0], &target[1], size);
+
+	/* Print result to prevent the code from being eliminated */
+	pr_err("FAIL: fortify did not catch an object overread!\n"
+	       "\"%d\" was the memcmp result.\n", result);
+}
+
+void lkdtm_FORTIFY_SUBOBJECT(void)
+{
+	struct target {
+		char a[10];
+		char b[10];
+	} target;
+	char *src;
+
+	src = kmalloc(20, GFP_KERNEL);
+	strscpy(src, "over ten bytes", 20);
+
+	pr_info("trying to strcpy past the end of a member of a struct\n");
+
+	/*
+	 * strncpy(target.a, src, 20); will hit a compile error because the
+	 * compiler knows at build time that target.a < 20 bytes. Use strcpy()
+	 * to force a runtime error.
+	 */
+	strcpy(target.a, src);
+
+	/* Use target.a to prevent the code from being eliminated */
+	pr_err("FAIL: fortify did not catch an sub-object overrun!\n"
+	       "\"%s\" was copied.\n", target.a);
+
+	kfree(src);
+}
diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index a5e344df9166..a002f39a5964 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -118,6 +118,8 @@ static const struct crashtype crashtypes[] = {
 	CRASHTYPE(UNSET_SMEP),
 	CRASHTYPE(CORRUPT_PAC),
 	CRASHTYPE(UNALIGNED_LOAD_STORE_WRITE),
+	CRASHTYPE(FORTIFY_OBJECT),
+	CRASHTYPE(FORTIFY_SUBOBJECT),
 	CRASHTYPE(OVERWRITE_ALLOCATION),
 	CRASHTYPE(WRITE_AFTER_FREE),
 	CRASHTYPE(READ_AFTER_FREE),
diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index 8878538b2c13..70c8b7c9460f 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -33,6 +33,8 @@ void lkdtm_STACK_GUARD_PAGE_TRAILING(void);
 void lkdtm_UNSET_SMEP(void);
 void lkdtm_DOUBLE_FAULT(void);
 void lkdtm_CORRUPT_PAC(void);
+void lkdtm_FORTIFY_OBJECT(void);
+void lkdtm_FORTIFY_SUBOBJECT(void);
 
 /* lkdtm_heap.c */
 void __init lkdtm_heap_init(void);
-- 
2.20.1

[RFC][PATCH v3 3/5] Fortify string function strscpy.

[laniel_francis]

From: Francis Laniel <laniel_francis@privacyrequired.com>

Fortified strscpy detects write overflows to dest.

Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
---
 include/linux/string.h | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index 46e91d684c47..add7426ff718 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -6,6 +6,8 @@
 #include <linux/compiler.h>	/* for inline */
 #include <linux/types.h>	/* for size_t */
 #include <linux/stddef.h>	/* for NULL */
+#include <linux/bug.h>		/* for WARN_ON_ONCE */
+#include <linux/errno.h>	/* for E2BIG */
 #include <stdarg.h>
 #include <uapi/linux/string.h>
 
@@ -357,6 +359,42 @@ __FORTIFY_INLINE size_t strlcpy(char *p, const char *q, size_t size)
 	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 1 as second argument to guess only p size even and not the
+	 * surrounding struct size (in case it is embedded inside a struct).
+	 */
+	size_t p_size = __builtin_object_size(p, 1);
+
+	/*
+	 * 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();
+
+	len = strnlen(q, size);
+	/*
+	 * strscpy handles read overflows by stop reading q when '\0' is
+	 * met.
+	 * We stick to this behavior here.
+	 */
+	len = (len >= size) ? size : len;
+
+	/* Otherwise generate a runtime write overflow error. */
+	if (len > p_size)
+		fortify_panic(__func__);
+	/*
+	 * Still use size as third argument to correctly compute max
+	 * inside strscpy.
+	 */
+	return __real_strscpy(p, q, size);
+}
+
 /* defined after fortified strlen and strnlen to reuse them */
 __FORTIFY_INLINE char *strncat(char *p, const char *q, __kernel_size_t count)
 {
-- 
2.20.1

[RFC][PATCH v3 4/5] Add new file in LKDTM to test fortified strscpy.

[laniel_francis]

From: Francis Laniel <laniel_francis@privacyrequired.com>

This new test generates a crash at runtime because there is a write overflow in
destination string.

Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
---
 drivers/misc/lkdtm/Makefile             |  1 +
 drivers/misc/lkdtm/core.c               |  1 +
 drivers/misc/lkdtm/fortify.c            | 47 +++++++++++++++++++++++++
 drivers/misc/lkdtm/lkdtm.h              |  3 ++
 tools/testing/selftests/lkdtm/tests.txt |  1 +
 5 files changed, 53 insertions(+)
 create mode 100644 drivers/misc/lkdtm/fortify.c

diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
index c70b3822013f..d898f7b22045 100644
--- a/drivers/misc/lkdtm/Makefile
+++ b/drivers/misc/lkdtm/Makefile
@@ -10,6 +10,7 @@ lkdtm-$(CONFIG_LKDTM)		+= rodata_objcopy.o
 lkdtm-$(CONFIG_LKDTM)		+= usercopy.o
 lkdtm-$(CONFIG_LKDTM)		+= stackleak.o
 lkdtm-$(CONFIG_LKDTM)		+= cfi.o
+lkdtm-$(CONFIG_LKDTM)		+= fortify.o
 
 KASAN_SANITIZE_stackleak.o	:= n
 KCOV_INSTRUMENT_rodata.o	:= n
diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index a002f39a5964..4326e2d09870 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -180,6 +180,7 @@ static const struct crashtype crashtypes[] = {
 #ifdef CONFIG_X86_32
 	CRASHTYPE(DOUBLE_FAULT),
 #endif
+	CRASHTYPE(FORTIFIED_STRSCPY),
 };
 
 
diff --git a/drivers/misc/lkdtm/fortify.c b/drivers/misc/lkdtm/fortify.c
new file mode 100644
index 000000000000..cecdfbb8ba55
--- /dev/null
+++ b/drivers/misc/lkdtm/fortify.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 Francis Laniel <laniel_francis@privacyrequired.com>
+ *
+ * Add tests related to fortified functions in this file.
+ */
+#include <linux/string.h>
+#include <linux/slab.h>
+#include "lkdtm.h"
+
+
+/*
+ * Calls fortified strscpy to test that it returns the same result as vanilla
+ * strscpy and generate a panic because there is a write overflow (i.e. src
+ * length is greater than dst length).
+ */
+void lkdtm_FORTIFIED_STRSCPY(void)
+{
+#if !defined(__NO_FORTIFY) && defined(__OPTIMIZE__) && defined(CONFIG_FORTIFY_SOURCE)
+	char *src;
+	char dst[3];
+
+	src = kstrdup("foobar", GFP_KERNEL);
+
+	if (src == NULL)
+		return;
+
+	/* Vanilla strscpy returns -E2BIG if size is 0. */
+	WARN_ON(strscpy(dst, src, 0) != -E2BIG);
+
+	/* Vanilla strscpy returns -E2BIG if src is truncated. */
+	WARN_ON(strscpy(dst, src, sizeof(dst)) != -E2BIG);
+
+	/* After above call, dst must contain "fo" because src was truncated. */
+	WARN_ON(strncmp(dst, "fo", sizeof(dst)) != 0);
+
+	/*
+	 * Use strlen here so size cannot be known at compile time and there is
+	 * a runtime overflow.
+	 */
+	strscpy(dst, src, strlen(src));
+
+	pr_info("Fail: No overflow in above strscpy call!\n");
+
+	kfree(src);
+#endif
+}
diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index 70c8b7c9460f..29c12dcdeab1 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -106,4 +106,7 @@ void lkdtm_STACKLEAK_ERASING(void);
 /* cfi.c */
 void lkdtm_CFI_FORWARD_PROTO(void);
 
+/* fortify.c */
+void lkdtm_FORTIFIED_STRSCPY(void);
+
 #endif
diff --git a/tools/testing/selftests/lkdtm/tests.txt b/tools/testing/selftests/lkdtm/tests.txt
index 9d266e79c6a2..4234109579eb 100644
--- a/tools/testing/selftests/lkdtm/tests.txt
+++ b/tools/testing/selftests/lkdtm/tests.txt
@@ -70,3 +70,4 @@ USERCOPY_KERNEL
 USERCOPY_KERNEL_DS
 STACKLEAK_ERASING OK: the rest of the thread stack is properly erased
 CFI_FORWARD_PROTO
+FORTIFIED_STRSCPY
\ No newline at end of file
-- 
2.20.1

[RFC][PATCH v3 5/5] Correct wrong filenames in comment.

[laniel_francis]

From: Francis Laniel <laniel_francis@privacyrequired.com>

In lkdtm.h, files targeted in comments are name "lkdtm_file.c" while there are
named "file.c" in directory.

Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
---
 drivers/misc/lkdtm/lkdtm.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index 29c12dcdeab1..1f7521f60565 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -6,7 +6,7 @@
 
 #include <linux/kernel.h>
 
-/* lkdtm_bugs.c */
+/* bugs.c */
 void __init lkdtm_bugs_init(int *recur_param);
 void lkdtm_PANIC(void);
 void lkdtm_BUG(void);
@@ -36,7 +36,7 @@ void lkdtm_CORRUPT_PAC(void);
 void lkdtm_FORTIFY_OBJECT(void);
 void lkdtm_FORTIFY_SUBOBJECT(void);
 
-/* lkdtm_heap.c */
+/* heap.c */
 void __init lkdtm_heap_init(void);
 void __exit lkdtm_heap_exit(void);
 void lkdtm_OVERWRITE_ALLOCATION(void);
@@ -48,7 +48,7 @@ void lkdtm_SLAB_FREE_DOUBLE(void);
 void lkdtm_SLAB_FREE_CROSS(void);
 void lkdtm_SLAB_FREE_PAGE(void);
 
-/* lkdtm_perms.c */
+/* perms.c */
 void __init lkdtm_perms_init(void);
 void lkdtm_WRITE_RO(void);
 void lkdtm_WRITE_RO_AFTER_INIT(void);
@@ -63,7 +63,7 @@ void lkdtm_EXEC_NULL(void);
 void lkdtm_ACCESS_USERSPACE(void);
 void lkdtm_ACCESS_NULL(void);
 
-/* lkdtm_refcount.c */
+/* refcount.c */
 void lkdtm_REFCOUNT_INC_OVERFLOW(void);
 void lkdtm_REFCOUNT_ADD_OVERFLOW(void);
 void lkdtm_REFCOUNT_INC_NOT_ZERO_OVERFLOW(void);
@@ -84,10 +84,10 @@ void lkdtm_REFCOUNT_SUB_AND_TEST_SATURATED(void);
 void lkdtm_REFCOUNT_TIMING(void);
 void lkdtm_ATOMIC_TIMING(void);
 
-/* lkdtm_rodata.c */
+/* rodata.c */
 void lkdtm_rodata_do_nothing(void);
 
-/* lkdtm_usercopy.c */
+/* usercopy.c */
 void __init lkdtm_usercopy_init(void);
 void __exit lkdtm_usercopy_exit(void);
 void lkdtm_USERCOPY_HEAP_SIZE_TO(void);
@@ -100,7 +100,7 @@ void lkdtm_USERCOPY_STACK_BEYOND(void);
 void lkdtm_USERCOPY_KERNEL(void);
 void lkdtm_USERCOPY_KERNEL_DS(void);
 
-/* lkdtm_stackleak.c */
+/* stackleak.c */
 void lkdtm_STACKLEAK_ERASING(void);
 
 /* cfi.c */
-- 
2.20.1

Re: [RFC][PATCH v3 3/5] Fortify string function strscpy.

[Kees Cook]

On Wed, Oct 21, 2020 at 05:06:06PM +0200, laniel_francis@privacyrequired.com wrote:
> From: Francis Laniel <laniel_francis@privacyrequired.com>
> 
> Fortified strscpy detects write overflows to dest.

This commit log needs to be expanded to explain the "why" of the change
with some detail.

Also, please look at the git commit history of include/linux/string.h
for a sense of the kinds of Subject prefixes being used. I think this
Subject should be something like:

	Subject: string.h: Add FORTIFY coverage for strscpy()

> 
> Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
> ---
>  include/linux/string.h | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 46e91d684c47..add7426ff718 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -6,6 +6,8 @@
>  #include <linux/compiler.h>	/* for inline */
>  #include <linux/types.h>	/* for size_t */
>  #include <linux/stddef.h>	/* for NULL */
> +#include <linux/bug.h>		/* for WARN_ON_ONCE */

^^^ no longer needed

> +#include <linux/errno.h>	/* for E2BIG */
>  #include <stdarg.h>
>  #include <uapi/linux/string.h>
>  
> @@ -357,6 +359,42 @@ __FORTIFY_INLINE size_t strlcpy(char *p, const char *q, size_t size)
>  	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 1 as second argument to guess only p size even and not the

This isn't a "guess". Perhaps just say:

/* Use string size rather than possible enclosing struct size. */

> +	 * surrounding struct size (in case it is embedded inside a struct).
> +	 */
> +	size_t p_size = __builtin_object_size(p, 1);

A short-circuit path should be added here:

	size_t q_size = __builtin_object_size(q, 1);
        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();

Compile-time size > p_size is caught here.

Compile-time over-read on strings isn't catchable.

> +
> +	len = strnlen(q, size);

Runtime over-read is caught here.

> +	/*
> +	 * strscpy handles read overflows by stop reading q when '\0' is
> +	 * met.
> +	 * We stick to this behavior here.
> +	 */
> +	len = (len >= size) ? size : len;

This test isn't needed: strnlen(q, size) will never return larger than
size. (i.e. len has already been reduced to size.)

> +
> +	/* Otherwise generate a runtime write overflow error. */
> +	if (len > p_size)
> +		fortify_panic(__func__);

Runtime over-write is caught here, though I think this needs to be:

	if (len + 1 > p_size)
		fortify_panic(__func__);

since we need to fit "len" plus NUL in p_size. len + 1 == p_size is ok.

> +	/*
> +	 * Still use size as third argument to correctly compute max
> +	 * inside strscpy.
> +	 */

If we do this, then we run the risk of doing the over-read anyway. For
this call, we need to use len in most cases.

> +	return __real_strscpy(p, q, size);

In the unfortified case, -E2BIG will happen when strlen(q) >= size. When
fortified, strnlen is capped at min(size, q_size), so -E2BIG could only
happen when len == size. If len < size, there will never be -E2BIG.

So I think this means we need the __real_strscpy() call to look like this:

	// All safe from over-read  (len + 1 <= min(size, q_size): strnlen() above).
	// All safe from over-write (len + 1 <= p_size: explicit test above).
	if (len < size)
		return __real_strscpy(p, q, len + 1); // not E2BIG
	else if (len + 1 == size)
		return __real_strscpy(p, q, len + 1); // not E2BIG
	else if (len == size) // therefore size < min(size, q_size, p_size)
		return __real_strscpy(p, q, size);    // E2BIG
	else //  len + 1 > size
		return __real_strscpy(p, q, len + 1); // E2BIG

or simply:

	return __real_strscpy(p, q, len == size ? size : len + 1);

But the complexity here clearly calls for test cases.

> +}
> +
>  /* defined after fortified strlen and strnlen to reuse them */
>  __FORTIFY_INLINE char *strncat(char *p, const char *q, __kernel_size_t count)
>  {
> -- 
> 2.20.1
> 

-- 
Kees Cook

Re: [RFC][PATCH v3 4/5] Add new file in LKDTM to test fortified strscpy.

[Kees Cook]

On Wed, Oct 21, 2020 at 05:06:07PM +0200, laniel_francis@privacyrequired.com wrote:
> From: Francis Laniel <laniel_francis@privacyrequired.com>
> 
> This new test generates a crash at runtime because there is a write overflow in
> destination string.
> 
> Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
> ---
>  drivers/misc/lkdtm/Makefile             |  1 +
>  drivers/misc/lkdtm/core.c               |  1 +
>  drivers/misc/lkdtm/fortify.c            | 47 +++++++++++++++++++++++++
>  drivers/misc/lkdtm/lkdtm.h              |  3 ++
>  tools/testing/selftests/lkdtm/tests.txt |  1 +
>  5 files changed, 53 insertions(+)
>  create mode 100644 drivers/misc/lkdtm/fortify.c
> 
> diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
> index c70b3822013f..d898f7b22045 100644
> --- a/drivers/misc/lkdtm/Makefile
> +++ b/drivers/misc/lkdtm/Makefile
> @@ -10,6 +10,7 @@ lkdtm-$(CONFIG_LKDTM)		+= rodata_objcopy.o
>  lkdtm-$(CONFIG_LKDTM)		+= usercopy.o
>  lkdtm-$(CONFIG_LKDTM)		+= stackleak.o
>  lkdtm-$(CONFIG_LKDTM)		+= cfi.o
> +lkdtm-$(CONFIG_LKDTM)		+= fortify.o
>  
>  KASAN_SANITIZE_stackleak.o	:= n
>  KCOV_INSTRUMENT_rodata.o	:= n
> diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
> index a002f39a5964..4326e2d09870 100644
> --- a/drivers/misc/lkdtm/core.c
> +++ b/drivers/misc/lkdtm/core.c
> @@ -180,6 +180,7 @@ static const struct crashtype crashtypes[] = {
>  #ifdef CONFIG_X86_32
>  	CRASHTYPE(DOUBLE_FAULT),
>  #endif
> +	CRASHTYPE(FORTIFIED_STRSCPY),
>  };
>  
>  
> diff --git a/drivers/misc/lkdtm/fortify.c b/drivers/misc/lkdtm/fortify.c
> new file mode 100644
> index 000000000000..cecdfbb8ba55
> --- /dev/null
> +++ b/drivers/misc/lkdtm/fortify.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020 Francis Laniel <laniel_francis@privacyrequired.com>
> + *
> + * Add tests related to fortified functions in this file.
> + */
> +#include <linux/string.h>
> +#include <linux/slab.h>
> +#include "lkdtm.h"
> +
> +
> +/*
> + * Calls fortified strscpy to test that it returns the same result as vanilla
> + * strscpy and generate a panic because there is a write overflow (i.e. src
> + * length is greater than dst length).
> + */
> +void lkdtm_FORTIFIED_STRSCPY(void)
> +{
> +#if !defined(__NO_FORTIFY) && defined(__OPTIMIZE__) && defined(CONFIG_FORTIFY_SOURCE)

I would drop the #if: just let it run and freak out on non-fortified
kernels.

> +	char *src;
> +	char dst[3];
> +
> +	src = kstrdup("foobar", GFP_KERNEL);
> +
> +	if (src == NULL)
> +		return;
> +
> +	/* Vanilla strscpy returns -E2BIG if size is 0. */
> +	WARN_ON(strscpy(dst, src, 0) != -E2BIG);

For LKDTM, we have different reporting that "normal", in the sense that
usually the WARN/BUG outcomes are _desirable_ (i.e. "freak out on stack
overflow"). So, I would write this as:

	if (strscpy(dst, src, 0) != -E2BIG)
		pr_warn("FAIL: strscpy() of 0 length did not return -E2BIG\n");

> +
> +	/* Vanilla strscpy returns -E2BIG if src is truncated. */
> +	WARN_ON(strscpy(dst, src, sizeof(dst)) != -E2BIG);

Same.

> +
> +	/* After above call, dst must contain "fo" because src was truncated. */
> +	WARN_ON(strncmp(dst, "fo", sizeof(dst)) != 0);

Same.

> +
> +	/*
> +	 * Use strlen here so size cannot be known at compile time and there is
> +	 * a runtime overflow.
> +	 */
> +	strscpy(dst, src, strlen(src));

I think we'll need a couple more corner cases, and any that need to Oops
separately can be separate functions. Here's a corner case to test to
strnlen():

struct {
	union {
	        char big[10];
	        char src[5];
	};
} weird = { .big = "hello!" };
char dst[sizeof(weird.src) + 1];

strscpy(dst, weird.src, sizeof(dst));

if (strcmp(dst, "hello") != 0)
	pr_warn("FAIL ...

But each of the cases being tested in the fortified strscpy() should be
exercised.

> +
> +	pr_info("Fail: No overflow in above strscpy call!\n");

pr_warn("FAIL: ...

> +
> +	kfree(src);
> +#endif
> +}
> diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
> index 70c8b7c9460f..29c12dcdeab1 100644
> --- a/drivers/misc/lkdtm/lkdtm.h
> +++ b/drivers/misc/lkdtm/lkdtm.h
> @@ -106,4 +106,7 @@ void lkdtm_STACKLEAK_ERASING(void);
>  /* cfi.c */
>  void lkdtm_CFI_FORWARD_PROTO(void);
>  
> +/* fortify.c */
> +void lkdtm_FORTIFIED_STRSCPY(void);
> +
>  #endif
> diff --git a/tools/testing/selftests/lkdtm/tests.txt b/tools/testing/selftests/lkdtm/tests.txt
> index 9d266e79c6a2..4234109579eb 100644
> --- a/tools/testing/selftests/lkdtm/tests.txt
> +++ b/tools/testing/selftests/lkdtm/tests.txt
> @@ -70,3 +70,4 @@ USERCOPY_KERNEL
>  USERCOPY_KERNEL_DS
>  STACKLEAK_ERASING OK: the rest of the thread stack is properly erased
>  CFI_FORWARD_PROTO
> +FORTIFIED_STRSCPY
> \ No newline at end of file
> -- 
> 2.20.1
> 

-- 
Kees Cook

Re: [RFC][PATCH v3 5/5] Correct wrong filenames in comment.

[Kees Cook]

On Wed, Oct 21, 2020 at 05:06:08PM +0200, laniel_francis@privacyrequired.com wrote:
> From: Francis Laniel <laniel_francis@privacyrequired.com>
> 
> In lkdtm.h, files targeted in comments are name "lkdtm_file.c" while there are
> named "file.c" in directory.
> 
> Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>

Acked-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [RFC][PATCH v3 4/5] Add new file in LKDTM to test fortified strscpy.

[Francis Laniel]

Le samedi 24 octobre 2020, 07:23:01 CEST Kees Cook a écrit :
> On Wed, Oct 21, 2020 at 05:06:07PM +0200, laniel_francis@privacyrequired.com 
wrote:
> > From: Francis Laniel <laniel_francis@privacyrequired.com>
> > 
> > This new test generates a crash at runtime because there is a write
> > overflow in destination string.
> > 
> > Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
> > ---
> > 
> >  drivers/misc/lkdtm/Makefile             |  1 +
> >  drivers/misc/lkdtm/core.c               |  1 +
> >  drivers/misc/lkdtm/fortify.c            | 47 +++++++++++++++++++++++++
> >  drivers/misc/lkdtm/lkdtm.h              |  3 ++
> >  tools/testing/selftests/lkdtm/tests.txt |  1 +
> >  5 files changed, 53 insertions(+)
> >  create mode 100644 drivers/misc/lkdtm/fortify.c
> > 
> > diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
> > index c70b3822013f..d898f7b22045 100644
> > --- a/drivers/misc/lkdtm/Makefile
> > +++ b/drivers/misc/lkdtm/Makefile
> > @@ -10,6 +10,7 @@ lkdtm-$(CONFIG_LKDTM)		+= rodata_objcopy.o
> > 
> >  lkdtm-$(CONFIG_LKDTM)		+= usercopy.o
> >  lkdtm-$(CONFIG_LKDTM)		+= stackleak.o
> >  lkdtm-$(CONFIG_LKDTM)		+= cfi.o
> > 
> > +lkdtm-$(CONFIG_LKDTM)		+= fortify.o
> > 
> >  KASAN_SANITIZE_stackleak.o	:= n
> >  KCOV_INSTRUMENT_rodata.o	:= n
> > 
> > diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
> > index a002f39a5964..4326e2d09870 100644
> > --- a/drivers/misc/lkdtm/core.c
> > +++ b/drivers/misc/lkdtm/core.c
> > @@ -180,6 +180,7 @@ static const struct crashtype crashtypes[] = {
> > 
> >  #ifdef CONFIG_X86_32
> >  
> >  	CRASHTYPE(DOUBLE_FAULT),
> >  
> >  #endif
> > 
> > +	CRASHTYPE(FORTIFIED_STRSCPY),
> > 
> >  };
> > 
> > diff --git a/drivers/misc/lkdtm/fortify.c b/drivers/misc/lkdtm/fortify.c
> > new file mode 100644
> > index 000000000000..cecdfbb8ba55
> > --- /dev/null
> > +++ b/drivers/misc/lkdtm/fortify.c
> > @@ -0,0 +1,47 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2020 Francis Laniel <laniel_francis@privacyrequired.com>
> > + *
> > + * Add tests related to fortified functions in this file.
> > + */
> > +#include <linux/string.h>
> > +#include <linux/slab.h>
> > +#include "lkdtm.h"
> > +
> > +
> > +/*
> > + * Calls fortified strscpy to test that it returns the same result as
> > vanilla + * strscpy and generate a panic because there is a write
> > overflow (i.e. src + * length is greater than dst length).
> > + */
> > +void lkdtm_FORTIFIED_STRSCPY(void)
> > +{
> > +#if !defined(__NO_FORTIFY) && defined(__OPTIMIZE__) &&
> > defined(CONFIG_FORTIFY_SOURCE)
> I would drop the #if: just let it run and freak out on non-fortified
> kernels.

I will drop it for v4.

> > +	char *src;
> > +	char dst[3];
> > +
> > +	src = kstrdup("foobar", GFP_KERNEL);
> > +
> > +	if (src == NULL)
> > +		return;
> > +
> > +	/* Vanilla strscpy returns -E2BIG if size is 0. */
> > +	WARN_ON(strscpy(dst, src, 0) != -E2BIG);
> 
> For LKDTM, we have different reporting that "normal", in the sense that
> usually the WARN/BUG outcomes are _desirable_ (i.e. "freak out on stack
> overflow"). So, I would write this as:
> 
> 	if (strscpy(dst, src, 0) != -E2BIG)
> 		pr_warn("FAIL: strscpy() of 0 length did not return -E2BIG\n");

I was not sure if using WARN_* lile macro was a good idea in LKDTM since if I 
understood correctly the goal of a LKDTM test is to fail.
I will rewrite it the way you suggested it for the v4.

> > +
> > +	/* Vanilla strscpy returns -E2BIG if src is truncated. */
> > +	WARN_ON(strscpy(dst, src, sizeof(dst)) != -E2BIG);
> 
> Same.
> 
> > +
> > +	/* After above call, dst must contain "fo" because src was truncated. */
> > +	WARN_ON(strncmp(dst, "fo", sizeof(dst)) != 0);
> 
> Same.
> 
> > +
> > +	/*
> > +	 * Use strlen here so size cannot be known at compile time and there is
> > +	 * a runtime overflow.
> > +	 */
> > +	strscpy(dst, src, strlen(src));
> 
> I think we'll need a couple more corner cases, and any that need to Oops
> separately can be separate functions. Here's a corner case to test to
> strnlen():
> 
> struct {
> 	union {
> 	        char big[10];
> 	        char src[5];
> 	};
> } weird = { .big = "hello!" };
> char dst[sizeof(weird.src) + 1];
> 
> strscpy(dst, weird.src, sizeof(dst));
> 
> if (strcmp(dst, "hello") != 0)
> 	pr_warn("FAIL ...
> 
> But each of the cases being tested in the fortified strscpy() should be
> exercised.

I will try to add these cases and other tests, if I think to more, for the 
next version so the tests cover as most case as possible.

> > +
> > +	pr_info("Fail: No overflow in above strscpy call!\n");
> 
> pr_warn("FAIL: ...
> 

Will be modified for next release!

> > +
> > +	kfree(src);
> > +#endif
> > +}
> > diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
> > index 70c8b7c9460f..29c12dcdeab1 100644
> > --- a/drivers/misc/lkdtm/lkdtm.h
> > +++ b/drivers/misc/lkdtm/lkdtm.h
> > @@ -106,4 +106,7 @@ void lkdtm_STACKLEAK_ERASING(void);
> > 
> >  /* cfi.c */
> >  void lkdtm_CFI_FORWARD_PROTO(void);
> > 
> > +/* fortify.c */
> > +void lkdtm_FORTIFIED_STRSCPY(void);
> > +
> > 
> >  #endif
> > 
> > diff --git a/tools/testing/selftests/lkdtm/tests.txt
> > b/tools/testing/selftests/lkdtm/tests.txt index
> > 9d266e79c6a2..4234109579eb 100644
> > --- a/tools/testing/selftests/lkdtm/tests.txt
> > +++ b/tools/testing/selftests/lkdtm/tests.txt
> > @@ -70,3 +70,4 @@ USERCOPY_KERNEL
> > 
> >  USERCOPY_KERNEL_DS
> >  STACKLEAK_ERASING OK: the rest of the thread stack is properly erased
> >  CFI_FORWARD_PROTO
> > 
> > +FORTIFIED_STRSCPY
> > \ No newline at end of file

Re: [RFC][PATCH v3 3/5] Fortify string function strscpy.

[Francis Laniel]

Le samedi 24 octobre 2020, 07:04:04 CEST Kees Cook a écrit :
> On Wed, Oct 21, 2020 at 05:06:06PM +0200, laniel_francis@privacyrequired.com 
wrote:
> > From: Francis Laniel <laniel_francis@privacyrequired.com>
> > 
> > Fortified strscpy detects write overflows to dest.
> 
> This commit log needs to be expanded to explain the "why" of the change
> with some detail.
> 
> Also, please look at the git commit history of include/linux/string.h
> for a sense of the kinds of Subject prefixes being used. I think this
> Subject should be something like:
> 
> 	Subject: string.h: Add FORTIFY coverage for strscpy()
> 

I will expand the commit message for the next version and adapt the title to 
follow the history.

> > Signed-off-by: Francis Laniel <laniel_francis@privacyrequired.com>
> > ---
> > 
> >  include/linux/string.h | 38 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> > 
> > diff --git a/include/linux/string.h b/include/linux/string.h
> > index 46e91d684c47..add7426ff718 100644
> > --- a/include/linux/string.h
> > +++ b/include/linux/string.h
> > @@ -6,6 +6,8 @@
> > 
> >  #include <linux/compiler.h>	/* for inline */
> >  #include <linux/types.h>	/* for size_t */
> >  #include <linux/stddef.h>	/* for NULL */
> > 
> > +#include <linux/bug.h>		/* for WARN_ON_ONCE */
> 
> ^^^ no longer needed

I will remove it!

> 
> > +#include <linux/errno.h>	/* for E2BIG */
> > 
> >  #include <stdarg.h>
> >  #include <uapi/linux/string.h>
> > 
> > @@ -357,6 +359,42 @@ __FORTIFY_INLINE size_t strlcpy(char *p, const char
> > *q, size_t size)> 
> >  	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 1 as second argument to guess only p size even and not the
> 
> This isn't a "guess". Perhaps just say:
> 
> /* Use string size rather than possible enclosing struct size. */
> 

Your comment is clearer than mine, I will add it for the next release.

> > +	 * surrounding struct size (in case it is embedded inside a struct).
> > +	 */
> > +	size_t p_size = __builtin_object_size(p, 1);
> 
> A short-circuit path should be added here:
> 
> 	size_t q_size = __builtin_object_size(q, 1);
>         if (p_size == (size_t)-1 && q_size == (size_t)-1)
>                 return __real_strscpy(p, q, size);
> 

I thought I misunderstood one of your passed message because I understood that 
I should remove this snippet.
I will readd it and sorry for the misunderstanding.

> > +	/*
> > +	 * 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();
> 
> Compile-time size > p_size is caught here.
> 
> Compile-time over-read on strings isn't catchable.
> 
> > +
> > +	len = strnlen(q, size);
> 
> Runtime over-read is caught here.
> 
> > +	/*
> > +	 * strscpy handles read overflows by stop reading q when '\0' is
> > +	 * met.
> > +	 * We stick to this behavior here.
> > +	 */
> > +	len = (len >= size) ? size : len;
> 
> This test isn't needed: strnlen(q, size) will never return larger than
> size. (i.e. len has already been reduced to size.)

Nice catch! I will remove this branch then.

> 
> > +
> > +	/* Otherwise generate a runtime write overflow error. */
> > +	if (len > p_size)
> > +		fortify_panic(__func__);
> 
> Runtime over-write is caught here, though I think this needs to be:
> 
> 	if (len + 1 > p_size)
> 		fortify_panic(__func__);
> 
> since we need to fit "len" plus NUL in p_size. len + 1 == p_size is ok.
> 

I am not really sure about this but I need to think more about it after coding 
a new version and tested it.

> > +	/*
> > +	 * Still use size as third argument to correctly compute max
> > +	 * inside strscpy.
> > +	 */
> 
> If we do this, then we run the risk of doing the over-read anyway. For
> this call, we need to use len in most cases.

I had trouble when I used len as argument in previous version of this patch 
set.
But you are right we totally need to use len, moreover it avoids having 
write_overflow.
I will then switch to len and test the code with LKDTM and gdb.

> 
> > +	return __real_strscpy(p, q, size);
> 
> In the unfortified case, -E2BIG will happen when strlen(q) >= size. When
> fortified, strnlen is capped at min(size, q_size), so -E2BIG could only
> happen when len == size. If len < size, there will never be -E2BIG.
> 
> So I think this means we need the __real_strscpy() call to look like this:
> 
> 	// All safe from over-read  (len + 1 <= min(size, q_size): strnlen()
> above). // All safe from over-write (len + 1 <= p_size: explicit test
> above). if (len < size)
> 		return __real_strscpy(p, q, len + 1); // not E2BIG
> 	else if (len + 1 == size)
> 		return __real_strscpy(p, q, len + 1); // not E2BIG
> 	else if (len == size) // therefore size < min(size, q_size, p_size)
> 		return __real_strscpy(p, q, size);    // E2BIG
> 	else //  len + 1 > size
> 		return __real_strscpy(p, q, len + 1); // E2BIG
> 
> or simply:
> 
> 	return __real_strscpy(p, q, len == size ? size : len + 1);
> 
> But the complexity here clearly calls for test cases.
> 

Modifying the last argument of __real_strscpy() can modify what it returns so 
fortified strscpy will not return the same as __real_strscpy().
I will stick for the behavior of __real__strscpy() for next release.
Maybe I will first write the test then the fortified function so I am sure there 
is no problem with the new version.

Thank you for the complete review though!

> > +}
> > +
> > 
> >  /* defined after fortified strlen and strnlen to reuse them */
> >  __FORTIFY_INLINE char *strncat(char *p, const char *q, __kernel_size_t
> >  count) {