* [PATCH v2 00/10] fortify: Add KUnit tests for runtime overflows
@ 2023-04-07 19:27 Kees Cook
2023-04-07 19:27 ` [PATCH v2 01/10] kunit: tool: Enable CONFIG_FORTIFY_SOURCE under UML Kees Cook
` (9 more replies)
0 siblings, 10 replies; 27+ messages in thread
From: Kees Cook @ 2023-04-07 19:27 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Nick Desaulniers, Liam Howlett,
Vlastimil Babka, Dan Williams, Rasmus Villemoes, Yury Norov,
Jason A. Donenfeld, Sander Vanheule, Eric Biggers,
Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
Hi,
This series adds KUnit tests for the CONFIG_FORTIFY_SOURCE behavior of the
standard C string functions, and for the strcat() family of functions,
as those were updated during refactoring. Finally, fortification error
messages are improved to give more context for the failure condition.
-Kees
v2:
- fix From/SoB
- strcat: force non-const length arguments (lkp)
- fix x86 and arm fortify_panic prototypes (lkp)
- move test-skip to init function (dlatypov)
- constify p_size, q_size everywhere (miguel)
- enum-ify, string-ify, bit-ify function name passing (aleksander & andy)
v1: https://lore.kernel.org/lkml/20230405235832.never.487-kees@kernel.org/
Kees Cook (10):
kunit: tool: Enable CONFIG_FORTIFY_SOURCE under UML
fortify: Allow KUnit test to build without FORTIFY
string: Add Kunit tests for strcat() family
fortify: Use const variables for __member_size tracking
fortify: Add protection for strlcat()
fortify: strcat: Move definition to use fortified strlcat()
fortify: Split reporting and avoid passing string pointer
fortify: Provide KUnit counters for failure testing
fortify: Add KUnit tests for runtime overflows
fortify: Improve buffer overflow reporting
MAINTAINERS | 1 +
arch/arm/boot/compressed/misc.c | 2 +-
arch/x86/boot/compressed/misc.c | 2 +-
include/linux/fortify-string.h | 257 +++++--
lib/Kconfig.debug | 7 +-
lib/Makefile | 1 +
lib/fortify_kunit.c | 731 +++++++++++++++++++
lib/strcat_kunit.c | 104 +++
lib/string_helpers.c | 26 +-
tools/objtool/check.c | 2 +-
tools/testing/kunit/configs/all_tests.config | 2 +
tools/testing/kunit/configs/arch_uml.config | 3 +
12 files changed, 1059 insertions(+), 79 deletions(-)
create mode 100644 lib/strcat_kunit.c
--
2.34.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 01/10] kunit: tool: Enable CONFIG_FORTIFY_SOURCE under UML
2023-04-07 19:27 [PATCH v2 00/10] fortify: Add KUnit tests for runtime overflows Kees Cook
@ 2023-04-07 19:27 ` Kees Cook
2023-04-07 23:33 ` Nick Desaulniers
2023-04-07 19:27 ` [PATCH v2 02/10] fortify: Allow KUnit test to build without FORTIFY Kees Cook
` (8 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Kees Cook @ 2023-04-07 19:27 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Nick Desaulniers, Liam Howlett,
Vlastimil Babka, Dan Williams, Rasmus Villemoes, Yury Norov,
Jason A. Donenfeld, Sander Vanheule, Eric Biggers,
Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
Since commit ba38961a069b ("um: Enable FORTIFY_SOURCE"), it's possible
to run the FORTIFY tests under UML. Enable CONFIG_FORTIFY_SOURCE when
running with --altests to gain additional coverage, and by default under
UML.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
tools/testing/kunit/configs/all_tests.config | 2 ++
tools/testing/kunit/configs/arch_uml.config | 3 +++
2 files changed, 5 insertions(+)
diff --git a/tools/testing/kunit/configs/all_tests.config b/tools/testing/kunit/configs/all_tests.config
index f990cbb73250..0393940c706a 100644
--- a/tools/testing/kunit/configs/all_tests.config
+++ b/tools/testing/kunit/configs/all_tests.config
@@ -9,6 +9,8 @@ CONFIG_KUNIT=y
CONFIG_KUNIT_EXAMPLE_TEST=y
CONFIG_KUNIT_ALL_TESTS=y
+CONFIG_FORTIFY_SOURCE=y
+
CONFIG_IIO=y
CONFIG_EXT4_FS=y
diff --git a/tools/testing/kunit/configs/arch_uml.config b/tools/testing/kunit/configs/arch_uml.config
index e824ce43b05a..54ad8972681a 100644
--- a/tools/testing/kunit/configs/arch_uml.config
+++ b/tools/testing/kunit/configs/arch_uml.config
@@ -3,3 +3,6 @@
# Enable virtio/pci, as a lot of tests require it.
CONFIG_VIRTIO_UML=y
CONFIG_UML_PCI_OVER_VIRTIO=y
+
+# Enable FORTIFY_SOURCE for wider checking.
+CONFIG_FORTIFY_SOURCE=y
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 02/10] fortify: Allow KUnit test to build without FORTIFY
2023-04-07 19:27 [PATCH v2 00/10] fortify: Add KUnit tests for runtime overflows Kees Cook
2023-04-07 19:27 ` [PATCH v2 01/10] kunit: tool: Enable CONFIG_FORTIFY_SOURCE under UML Kees Cook
@ 2023-04-07 19:27 ` Kees Cook
2023-07-02 15:07 ` Geert Uytterhoeven
2023-04-07 19:27 ` [PATCH v2 03/10] string: Add Kunit tests for strcat() family Kees Cook
` (7 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Kees Cook @ 2023-04-07 19:27 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Nick Desaulniers, Liam Howlett,
Vlastimil Babka, Dan Williams, Rasmus Villemoes, Yury Norov,
Jason A. Donenfeld, Sander Vanheule, Eric Biggers,
Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
In order for CI systems to notice all the skipped tests related to
CONFIG_FORTIFY_SOURCE, allow the FORTIFY_SOURCE KUnit tests to build
with or without CONFIG_FORTIFY_SOURCE.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
lib/Kconfig.debug | 2 +-
lib/fortify_kunit.c | 14 ++++++++++++++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index c8b379e2e9ad..d48a5f4b471e 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2614,7 +2614,7 @@ config STACKINIT_KUNIT_TEST
config FORTIFY_KUNIT_TEST
tristate "Test fortified str*() and mem*() function internals at runtime" if !KUNIT_ALL_TESTS
- depends on KUNIT && FORTIFY_SOURCE
+ depends on KUNIT
default KUNIT_ALL_TESTS
help
Builds unit tests for checking internals of FORTIFY_SOURCE as used
diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c
index c8c33cbaae9e..524132f33cf0 100644
--- a/lib/fortify_kunit.c
+++ b/lib/fortify_kunit.c
@@ -25,6 +25,11 @@ static const char array_of_10[] = "this is 10";
static const char *ptr_of_11 = "this is 11!";
static char array_unknown[] = "compiler thinks I might change";
+/* Handle being built without CONFIG_FORTIFY_SOURCE */
+#ifndef __compiletime_strlen
+# define __compiletime_strlen __builtin_strlen
+#endif
+
static void known_sizes_test(struct kunit *test)
{
KUNIT_EXPECT_EQ(test, __compiletime_strlen("88888888"), 8);
@@ -307,6 +312,14 @@ DEFINE_ALLOC_SIZE_TEST_PAIR(kvmalloc)
} while (0)
DEFINE_ALLOC_SIZE_TEST_PAIR(devm_kmalloc)
+static int fortify_test_init(struct kunit *test)
+{
+ if (!IS_ENABLED(CONFIG_FORTIFY_SOURCE))
+ kunit_skip(test, "Not built with CONFIG_FORTIFY_SOURCE=y");
+
+ return 0;
+}
+
static struct kunit_case fortify_test_cases[] = {
KUNIT_CASE(known_sizes_test),
KUNIT_CASE(control_flow_split_test),
@@ -323,6 +336,7 @@ static struct kunit_case fortify_test_cases[] = {
static struct kunit_suite fortify_test_suite = {
.name = "fortify",
+ .init = fortify_test_init,
.test_cases = fortify_test_cases,
};
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 03/10] string: Add Kunit tests for strcat() family
2023-04-07 19:27 [PATCH v2 00/10] fortify: Add KUnit tests for runtime overflows Kees Cook
2023-04-07 19:27 ` [PATCH v2 01/10] kunit: tool: Enable CONFIG_FORTIFY_SOURCE under UML Kees Cook
2023-04-07 19:27 ` [PATCH v2 02/10] fortify: Allow KUnit test to build without FORTIFY Kees Cook
@ 2023-04-07 19:27 ` Kees Cook
2023-04-07 19:27 ` [PATCH v2 04/10] fortify: Use const variables for __member_size tracking Kees Cook
` (6 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Kees Cook @ 2023-04-07 19:27 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Nick Desaulniers, Liam Howlett,
Vlastimil Babka, Dan Williams, Rasmus Villemoes, Yury Norov,
Jason A. Donenfeld, Sander Vanheule, Eric Biggers,
Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
Add tests to make sure the strcat() family of functions behave
correctly.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
MAINTAINERS | 1 +
lib/Kconfig.debug | 5 +++
lib/Makefile | 1 +
lib/strcat_kunit.c | 104 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 111 insertions(+)
create mode 100644 lib/strcat_kunit.c
diff --git a/MAINTAINERS b/MAINTAINERS
index ec57c42ed544..86c0012b5130 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8021,6 +8021,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/har
F: include/linux/fortify-string.h
F: lib/fortify_kunit.c
F: lib/memcpy_kunit.c
+F: lib/strcat_kunit.c
F: lib/strscpy_kunit.c
F: lib/test_fortify/*
F: scripts/test_fortify.sh
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index d48a5f4b471e..86157aa5e979 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2631,6 +2631,11 @@ config HW_BREAKPOINT_KUNIT_TEST
If unsure, say N.
+config STRCAT_KUNIT_TEST
+ tristate "Test strcat() family of functions at runtime" if !KUNIT_ALL_TESTS
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+
config STRSCPY_KUNIT_TEST
tristate "Test strscpy*() family of functions at runtime" if !KUNIT_ALL_TESTS
depends on KUNIT
diff --git a/lib/Makefile b/lib/Makefile
index baf2821f7a00..6582d8fe1a77 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -389,6 +389,7 @@ obj-$(CONFIG_STACKINIT_KUNIT_TEST) += stackinit_kunit.o
CFLAGS_fortify_kunit.o += $(call cc-disable-warning, unsequenced)
CFLAGS_fortify_kunit.o += $(DISABLE_STRUCTLEAK_PLUGIN)
obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
+obj-$(CONFIG_STRCAT_KUNIT_TEST) += strcat_kunit.o
obj-$(CONFIG_STRSCPY_KUNIT_TEST) += strscpy_kunit.o
obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
diff --git a/lib/strcat_kunit.c b/lib/strcat_kunit.c
new file mode 100644
index 000000000000..e21be95514af
--- /dev/null
+++ b/lib/strcat_kunit.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Kernel module for testing 'strcat' family of functions.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <kunit/test.h>
+#include <linux/string.h>
+
+static volatile int unconst;
+
+static void strcat_test(struct kunit *test)
+{
+ char dest[8];
+
+ /* Destination is terminated. */
+ memset(dest, 0, sizeof(dest));
+ KUNIT_EXPECT_EQ(test, strlen(dest), 0);
+ /* Empty copy does nothing. */
+ KUNIT_EXPECT_TRUE(test, strcat(dest, "") == dest);
+ KUNIT_EXPECT_STREQ(test, dest, "");
+ /* 4 characters copied in, stops at %NUL. */
+ KUNIT_EXPECT_TRUE(test, strcat(dest, "four\000123") == dest);
+ KUNIT_EXPECT_STREQ(test, dest, "four");
+ KUNIT_EXPECT_EQ(test, dest[5], '\0');
+ /* 2 more characters copied in okay. */
+ KUNIT_EXPECT_TRUE(test, strcat(dest, "AB") == dest);
+ KUNIT_EXPECT_STREQ(test, dest, "fourAB");
+}
+
+static void strncat_test(struct kunit *test)
+{
+ char dest[8];
+
+ /* Destination is terminated. */
+ memset(dest, 0, sizeof(dest));
+ KUNIT_EXPECT_EQ(test, strlen(dest), 0);
+ /* Empty copy of size 0 does nothing. */
+ KUNIT_EXPECT_TRUE(test, strncat(dest, "", 0 + unconst) == dest);
+ KUNIT_EXPECT_STREQ(test, dest, "");
+ /* Empty copy of size 1 does nothing too. */
+ KUNIT_EXPECT_TRUE(test, strncat(dest, "", 1 + unconst) == dest);
+ KUNIT_EXPECT_STREQ(test, dest, "");
+ /* Copy of max 0 characters should do nothing. */
+ KUNIT_EXPECT_TRUE(test, strncat(dest, "asdf", 0 + unconst) == dest);
+ KUNIT_EXPECT_STREQ(test, dest, "");
+
+ /* 4 characters copied in, even if max is 8. */
+ KUNIT_EXPECT_TRUE(test, strncat(dest, "four\000123", 8 + unconst) == dest);
+ KUNIT_EXPECT_STREQ(test, dest, "four");
+ KUNIT_EXPECT_EQ(test, dest[5], '\0');
+ KUNIT_EXPECT_EQ(test, dest[6], '\0');
+ /* 2 characters copied in okay, 2 ignored. */
+ KUNIT_EXPECT_TRUE(test, strncat(dest, "ABCD", 2 + unconst) == dest);
+ KUNIT_EXPECT_STREQ(test, dest, "fourAB");
+}
+
+static void strlcat_test(struct kunit *test)
+{
+ char dest[8] = "";
+ int len = sizeof(dest) + unconst;
+
+ /* Destination is terminated. */
+ KUNIT_EXPECT_EQ(test, strlen(dest), 0);
+ /* Empty copy is size 0. */
+ KUNIT_EXPECT_EQ(test, strlcat(dest, "", len), 0);
+ KUNIT_EXPECT_STREQ(test, dest, "");
+ /* Size 1 should keep buffer terminated, report size of source only. */
+ KUNIT_EXPECT_EQ(test, strlcat(dest, "four", 1 + unconst), 4);
+ KUNIT_EXPECT_STREQ(test, dest, "");
+
+ /* 4 characters copied in. */
+ KUNIT_EXPECT_EQ(test, strlcat(dest, "four", len), 4);
+ KUNIT_EXPECT_STREQ(test, dest, "four");
+ /* 2 characters copied in okay, gets to 6 total. */
+ KUNIT_EXPECT_EQ(test, strlcat(dest, "AB", len), 6);
+ KUNIT_EXPECT_STREQ(test, dest, "fourAB");
+ /* 2 characters ignored if max size (7) reached. */
+ KUNIT_EXPECT_EQ(test, strlcat(dest, "CD", 7 + unconst), 8);
+ KUNIT_EXPECT_STREQ(test, dest, "fourAB");
+ /* 1 of 2 characters skipped, now at true max size. */
+ KUNIT_EXPECT_EQ(test, strlcat(dest, "EFG", len), 9);
+ KUNIT_EXPECT_STREQ(test, dest, "fourABE");
+ /* Everything else ignored, now at full size. */
+ KUNIT_EXPECT_EQ(test, strlcat(dest, "1234", len), 11);
+ KUNIT_EXPECT_STREQ(test, dest, "fourABE");
+}
+
+static struct kunit_case strcat_test_cases[] = {
+ KUNIT_CASE(strcat_test),
+ KUNIT_CASE(strncat_test),
+ KUNIT_CASE(strlcat_test),
+ {}
+};
+
+static struct kunit_suite strcat_test_suite = {
+ .name = "strcat",
+ .test_cases = strcat_test_cases,
+};
+
+kunit_test_suite(strcat_test_suite);
+
+MODULE_LICENSE("GPL");
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 04/10] fortify: Use const variables for __member_size tracking
2023-04-07 19:27 [PATCH v2 00/10] fortify: Add KUnit tests for runtime overflows Kees Cook
` (2 preceding siblings ...)
2023-04-07 19:27 ` [PATCH v2 03/10] string: Add Kunit tests for strcat() family Kees Cook
@ 2023-04-07 19:27 ` Kees Cook
2023-04-18 17:58 ` Nick Desaulniers
2023-04-07 19:27 ` [PATCH v2 05/10] fortify: Add protection for strlcat() Kees Cook
` (5 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Kees Cook @ 2023-04-07 19:27 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Miguel Ojeda, Andy Shevchenko, Cezary Rojewski,
Puyou Lu, Mark Brown, Josh Poimboeuf, Peter Zijlstra,
Brendan Higgins, David Gow, Andrew Morton, Nathan Chancellor,
Alexander Potapenko, Zhaoyang Huang, Randy Dunlap,
Geert Uytterhoeven, Miguel Ojeda, Alexander Lobakin,
Nick Desaulniers, Liam Howlett, Vlastimil Babka, Dan Williams,
Rasmus Villemoes, Yury Norov, Jason A. Donenfeld,
Sander Vanheule, Eric Biggers, Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
The sizes reported by __member_size should never change in a given
function. Mark them as such.
Suggested-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/linux/fortify-string.h | 42 +++++++++++++++++-----------------
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index c9de1f59ee80..fae1bf4bd543 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -20,7 +20,7 @@ void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning("
({ \
char *__p = (char *)(p); \
size_t __ret = SIZE_MAX; \
- size_t __p_size = __member_size(p); \
+ const size_t __p_size = __member_size(p); \
if (__p_size != SIZE_MAX && \
__builtin_constant_p(*__p)) { \
size_t __p_len = __p_size - 1; \
@@ -142,7 +142,7 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size)
__FORTIFY_INLINE __diagnose_as(__builtin_strncpy, 1, 2, 3)
char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
{
- size_t p_size = __member_size(p);
+ const size_t p_size = __member_size(p);
if (__compiletime_lessthan(p_size, size))
__write_overflow();
@@ -169,7 +169,7 @@ char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
__FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
char *strcat(char * const POS p, const char *q)
{
- size_t p_size = __member_size(p);
+ const size_t p_size = __member_size(p);
if (p_size == SIZE_MAX)
return __underlying_strcat(p, q);
@@ -191,8 +191,8 @@ extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(st
*/
__FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size_t maxlen)
{
- size_t p_size = __member_size(p);
- size_t p_len = __compiletime_strlen(p);
+ const size_t p_size = __member_size(p);
+ const size_t p_len = __compiletime_strlen(p);
size_t ret;
/* We can take compile-time actions when maxlen is const. */
@@ -233,8 +233,8 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size
__FORTIFY_INLINE __diagnose_as(__builtin_strlen, 1)
__kernel_size_t __fortify_strlen(const char * const POS p)
{
+ const size_t p_size = __member_size(p);
__kernel_size_t ret;
- size_t p_size = __member_size(p);
/* Give up if we don't know how large p is. */
if (p_size == SIZE_MAX)
@@ -267,8 +267,8 @@ extern size_t __real_strlcpy(char *, const char *, size_t) __RENAME(strlcpy);
*/
__FORTIFY_INLINE size_t strlcpy(char * const POS p, const char * const POS q, size_t size)
{
- size_t p_size = __member_size(p);
- size_t q_size = __member_size(q);
+ const size_t p_size = __member_size(p);
+ const size_t q_size = __member_size(q);
size_t q_len; /* Full count of source string length. */
size_t len; /* Count of characters going into destination. */
@@ -318,10 +318,10 @@ extern ssize_t __real_strscpy(char *, const char *, size_t) __RENAME(strscpy);
*/
__FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, size_t size)
{
- size_t len;
/* Use string size rather than possible enclosing struct size. */
- size_t p_size = __member_size(p);
- size_t q_size = __member_size(q);
+ const size_t p_size = __member_size(p);
+ const size_t q_size = __member_size(q);
+ size_t len;
/* If we cannot get size of p and q default to call strscpy. */
if (p_size == SIZE_MAX && q_size == SIZE_MAX)
@@ -394,9 +394,9 @@ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, s
__FORTIFY_INLINE __diagnose_as(__builtin_strncat, 1, 2, 3)
char *strncat(char * const POS p, const char * const POS q, __kernel_size_t count)
{
+ const size_t p_size = __member_size(p);
+ const size_t q_size = __member_size(q);
size_t p_len, copy_len;
- size_t p_size = __member_size(p);
- size_t q_size = __member_size(q);
if (p_size == SIZE_MAX && q_size == SIZE_MAX)
return __underlying_strncat(p, q, count);
@@ -639,7 +639,7 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
extern void *__real_memscan(void *, int, __kernel_size_t) __RENAME(memscan);
__FORTIFY_INLINE void *memscan(void * const POS0 p, int c, __kernel_size_t size)
{
- size_t p_size = __struct_size(p);
+ const size_t p_size = __struct_size(p);
if (__compiletime_lessthan(p_size, size))
__read_overflow();
@@ -651,8 +651,8 @@ __FORTIFY_INLINE void *memscan(void * const POS0 p, int c, __kernel_size_t size)
__FORTIFY_INLINE __diagnose_as(__builtin_memcmp, 1, 2, 3)
int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t size)
{
- size_t p_size = __struct_size(p);
- size_t q_size = __struct_size(q);
+ const size_t p_size = __struct_size(p);
+ const size_t q_size = __struct_size(q);
if (__builtin_constant_p(size)) {
if (__compiletime_lessthan(p_size, size))
@@ -668,7 +668,7 @@ int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t
__FORTIFY_INLINE __diagnose_as(__builtin_memchr, 1, 2, 3)
void *memchr(const void * const POS0 p, int c, __kernel_size_t size)
{
- size_t p_size = __struct_size(p);
+ const size_t p_size = __struct_size(p);
if (__compiletime_lessthan(p_size, size))
__read_overflow();
@@ -680,7 +680,7 @@ void *memchr(const void * const POS0 p, int c, __kernel_size_t size)
void *__real_memchr_inv(const void *s, int c, size_t n) __RENAME(memchr_inv);
__FORTIFY_INLINE void *memchr_inv(const void * const POS0 p, int c, size_t size)
{
- size_t p_size = __struct_size(p);
+ const size_t p_size = __struct_size(p);
if (__compiletime_lessthan(p_size, size))
__read_overflow();
@@ -693,7 +693,7 @@ extern void *__real_kmemdup(const void *src, size_t len, gfp_t gfp) __RENAME(kme
__realloc_size(2);
__FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp)
{
- size_t p_size = __struct_size(p);
+ const size_t p_size = __struct_size(p);
if (__compiletime_lessthan(p_size, size))
__read_overflow();
@@ -720,8 +720,8 @@ __FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp
__FORTIFY_INLINE __diagnose_as(__builtin_strcpy, 1, 2)
char *strcpy(char * const POS p, const char * const POS q)
{
- size_t p_size = __member_size(p);
- size_t q_size = __member_size(q);
+ const size_t p_size = __member_size(p);
+ const size_t q_size = __member_size(q);
size_t size;
/* If neither buffer size is known, immediately give up. */
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 05/10] fortify: Add protection for strlcat()
2023-04-07 19:27 [PATCH v2 00/10] fortify: Add KUnit tests for runtime overflows Kees Cook
` (3 preceding siblings ...)
2023-04-07 19:27 ` [PATCH v2 04/10] fortify: Use const variables for __member_size tracking Kees Cook
@ 2023-04-07 19:27 ` Kees Cook
2023-04-07 19:27 ` [PATCH v2 06/10] fortify: strcat: Move definition to use fortified strlcat() Kees Cook
` (4 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Kees Cook @ 2023-04-07 19:27 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Nick Desaulniers, Liam Howlett,
Vlastimil Babka, Dan Williams, Rasmus Villemoes, Yury Norov,
Jason A. Donenfeld, Sander Vanheule, Eric Biggers,
Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
The definition of strcat() was defined in terms of unfortified strlcat(),
but that meant there was no bounds checking done on the internal strlen()
calls, and the (bounded) copy would be performed before reporting a
failure. Additionally, pathological cases (i.e. unterminated destination
buffer) did not make calls to fortify_panic(), which will make future unit
testing more difficult. Instead, explicitly define a fortified strlcat()
wrapper for strcat() to use.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/linux/fortify-string.h | 64 ++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index fae1bf4bd543..8cf17ef81905 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -371,6 +371,70 @@ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, s
return __real_strscpy(p, q, len);
}
+/* Defined after fortified strlen() to reuse it. */
+extern size_t __real_strlcat(char *p, const char *q, size_t avail) __RENAME(strlcat);
+/**
+ * strlcat - Append a string to an existing string
+ *
+ * @p: pointer to %NUL-terminated string to append to
+ * @q: pointer to %NUL-terminated string to append from
+ * @avail: Maximum bytes available in @p
+ *
+ * Appends %NUL-terminated string @q after the %NUL-terminated
+ * string at @p, but will not write beyond @avail bytes total,
+ * potentially truncating the copy from @q. @p will stay
+ * %NUL-terminated only if a %NUL already existed within
+ * the @avail bytes of @p. If so, the resulting number of
+ * bytes copied from @q will be at most "@avail - strlen(@p) - 1".
+ *
+ * Do not use this function. While FORTIFY_SOURCE tries to avoid
+ * read and write overflows, this is only possible when the sizes
+ * of @p and @q are known to the compiler. Prefer building the
+ * string with formatting, via scnprintf(), seq_buf, or similar.
+ *
+ * Returns total bytes that _would_ have been contained by @p
+ * regardless of truncation, similar to snprintf(). If return
+ * value is >= @avail, the string has been truncated.
+ *
+ */
+__FORTIFY_INLINE
+size_t strlcat(char * const POS p, const char * const POS q, size_t avail)
+{
+ const size_t p_size = __member_size(p);
+ const size_t q_size = __member_size(q);
+ size_t p_len, copy_len;
+ size_t actual, wanted;
+
+ /* Give up immediately if both buffer sizes are unknown. */
+ if (p_size == SIZE_MAX && q_size == SIZE_MAX)
+ return __real_strlcat(p, q, avail);
+
+ p_len = strnlen(p, avail);
+ copy_len = strlen(q);
+ wanted = actual = p_len + copy_len;
+
+ /* Cannot append any more: report truncation. */
+ if (avail <= p_len)
+ return wanted;
+
+ /* Give up if string is already overflowed. */
+ if (p_size <= p_len)
+ fortify_panic(__func__);
+
+ if (actual >= avail) {
+ copy_len = avail - p_len - 1;
+ actual = p_len + copy_len;
+ }
+
+ /* Give up if copy will overflow. */
+ if (p_size <= actual)
+ fortify_panic(__func__);
+ __underlying_memcpy(p + p_len, q, copy_len);
+ p[actual] = '\0';
+
+ return wanted;
+}
+
/**
* strncat - Append a string to an existing string
*
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 06/10] fortify: strcat: Move definition to use fortified strlcat()
2023-04-07 19:27 [PATCH v2 00/10] fortify: Add KUnit tests for runtime overflows Kees Cook
` (4 preceding siblings ...)
2023-04-07 19:27 ` [PATCH v2 05/10] fortify: Add protection for strlcat() Kees Cook
@ 2023-04-07 19:27 ` Kees Cook
2023-04-18 18:09 ` Nick Desaulniers
2023-04-07 19:27 ` [PATCH v2 07/10] fortify: Split reporting and avoid passing string pointer Kees Cook
` (3 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Kees Cook @ 2023-04-07 19:27 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Nick Desaulniers, Liam Howlett,
Vlastimil Babka, Dan Williams, Rasmus Villemoes, Yury Norov,
Jason A. Donenfeld, Sander Vanheule, Eric Biggers,
Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
Move the definition of fortified strcat() to after strlcat() to use it
for bounds checking.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/linux/fortify-string.h | 53 +++++++++++++++++-----------------
1 file changed, 26 insertions(+), 27 deletions(-)
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index 8cf17ef81905..ab058d092817 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -151,33 +151,6 @@ char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
return __underlying_strncpy(p, q, size);
}
-/**
- * strcat - Append a string to an existing string
- *
- * @p: pointer to NUL-terminated string to append to
- * @q: pointer to NUL-terminated source string to append from
- *
- * Do not use this function. While FORTIFY_SOURCE tries to avoid
- * read and write overflows, this is only possible when the
- * destination buffer size is known to the compiler. Prefer
- * building the string with formatting, via scnprintf() or similar.
- * At the very least, use strncat().
- *
- * Returns @p.
- *
- */
-__FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
-char *strcat(char * const POS p, const char *q)
-{
- const size_t p_size = __member_size(p);
-
- if (p_size == SIZE_MAX)
- return __underlying_strcat(p, q);
- if (strlcat(p, q, p_size) >= p_size)
- fortify_panic(__func__);
- return p;
-}
-
extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(strnlen);
/**
* strnlen - Return bounded count of characters in a NUL-terminated string
@@ -435,6 +408,32 @@ size_t strlcat(char * const POS p, const char * const POS q, size_t avail)
return wanted;
}
+/* Defined after fortified strlcat() to reuse it. */
+/**
+ * strcat - Append a string to an existing string
+ *
+ * @p: pointer to NUL-terminated string to append to
+ * @q: pointer to NUL-terminated source string to append from
+ *
+ * Do not use this function. While FORTIFY_SOURCE tries to avoid
+ * read and write overflows, this is only possible when the
+ * destination buffer size is known to the compiler. Prefer
+ * building the string with formatting, via scnprintf() or similar.
+ * At the very least, use strncat().
+ *
+ * Returns @p.
+ *
+ */
+__FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
+char *strcat(char * const POS p, const char *q)
+{
+ const size_t p_size = __member_size(p);
+
+ if (strlcat(p, q, p_size) >= p_size)
+ fortify_panic(__func__);
+ return p;
+}
+
/**
* strncat - Append a string to an existing string
*
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 07/10] fortify: Split reporting and avoid passing string pointer
2023-04-07 19:27 [PATCH v2 00/10] fortify: Add KUnit tests for runtime overflows Kees Cook
` (5 preceding siblings ...)
2023-04-07 19:27 ` [PATCH v2 06/10] fortify: strcat: Move definition to use fortified strlcat() Kees Cook
@ 2023-04-07 19:27 ` Kees Cook
2023-04-08 2:26 ` kernel test robot
2023-04-20 15:52 ` Alexander Lobakin
2023-04-07 19:27 ` [PATCH v2 08/10] fortify: Provide KUnit counters for failure testing Kees Cook
` (2 subsequent siblings)
9 siblings, 2 replies; 27+ messages in thread
From: Kees Cook @ 2023-04-07 19:27 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Nick Desaulniers, Liam Howlett,
Vlastimil Babka, Dan Williams, Rasmus Villemoes, Yury Norov,
Jason A. Donenfeld, Sander Vanheule, Eric Biggers,
Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
In preparation for KUnit testing and further improvements in fortify
failure reporting, split out the report and encode the function and
access failure (read or write overflow) into a single u8 argument. This
mainly ends up saving some space in the data segment. For a defconfig
with FORTIFY_SOURCE enabled:
$ size gcc/vmlinux.before gcc/vmlinux.after
text data bss dec hex filename
26132309 9760658 2195460 38088427 2452eeb gcc/vmlinux.before
26132386 9748382 2195460 38076228 244ff44 gcc/vmlinux.after
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Cezary Rojewski <cezary.rojewski@intel.com>
Cc: Puyou Lu <puyou.lu@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/arm/boot/compressed/misc.c | 2 +-
arch/x86/boot/compressed/misc.c | 2 +-
include/linux/fortify-string.h | 84 ++++++++++++++++++++++++---------
lib/string_helpers.c | 23 +++++++--
tools/objtool/check.c | 2 +-
5 files changed, 85 insertions(+), 28 deletions(-)
diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
index abfed1aa2baa..f31e2c949089 100644
--- a/arch/arm/boot/compressed/misc.c
+++ b/arch/arm/boot/compressed/misc.c
@@ -160,7 +160,7 @@ decompress_kernel(unsigned long output_start, unsigned long free_mem_ptr_p,
putstr(" done, booting the kernel.\n");
}
-void fortify_panic(const char *name)
+void __fortify_panic(const u8 reason)
{
error("detected buffer overflow");
}
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 014ff222bf4b..aa45e7529a40 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -470,7 +470,7 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
return output + entry_offset;
}
-void fortify_panic(const char *name)
+void __fortify_panic(const u8 reason)
{
error("detected buffer overflow");
}
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index ab058d092817..19906b45fb98 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -2,6 +2,7 @@
#ifndef _LINUX_FORTIFY_STRING_H_
#define _LINUX_FORTIFY_STRING_H_
+#include <linux/bitfield.h>
#include <linux/bug.h>
#include <linux/const.h>
#include <linux/limits.h>
@@ -9,7 +10,45 @@
#define __FORTIFY_INLINE extern __always_inline __gnu_inline __overloadable
#define __RENAME(x) __asm__(#x)
-void fortify_panic(const char *name) __noreturn __cold;
+#define FORTIFY_REASON_DIR(r) FIELD_GET(BIT(0), r)
+#define FORTIFY_REASON_FUNC(r) FIELD_GET(GENMASK(7, 1), r)
+#define FORTIFY_REASON(func, write) (FIELD_PREP(BIT(0), write) | \
+ FIELD_PREP(GENMASK(7, 1), func))
+
+#define fortify_panic(func, write) \
+ __fortify_panic(FORTIFY_REASON(func, write))
+
+#define FORTIFY_READ 0
+#define FORTIFY_WRITE 1
+
+#define EACH_FORTIFY_FUNC(macro) \
+ macro(strncpy), \
+ macro(strnlen), \
+ macro(strlen), \
+ macro(strlcpy), \
+ macro(strscpy), \
+ macro(strlcat), \
+ macro(strcat), \
+ macro(strncat), \
+ macro(memset), \
+ macro(memcpy), \
+ macro(memmove), \
+ macro(memscan), \
+ macro(memcmp), \
+ macro(memchr), \
+ macro(memchr_inv), \
+ macro(kmemdup), \
+ macro(strcpy), \
+ macro(UNKNOWN),
+
+#define MAKE_FORTIFY_FUNC(func) FORTIFY_FUNC_##func
+
+enum fortify_func {
+ EACH_FORTIFY_FUNC(MAKE_FORTIFY_FUNC)
+};
+
+void __fortify_report(const u8 reason);
+void __fortify_panic(const u8 reason) __cold __noreturn;
void __read_overflow(void) __compiletime_error("detected read beyond size of object (1st parameter)");
void __read_overflow2(void) __compiletime_error("detected read beyond size of object (2nd parameter)");
void __read_overflow2_field(size_t avail, size_t wanted) __compiletime_warning("detected read beyond size of field (2nd parameter); maybe use struct_group()?");
@@ -147,7 +186,7 @@ char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
if (__compiletime_lessthan(p_size, size))
__write_overflow();
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strncpy, FORTIFY_WRITE);
return __underlying_strncpy(p, q, size);
}
@@ -178,7 +217,7 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size
/* Do not check characters beyond the end of p. */
ret = __real_strnlen(p, maxlen < p_size ? maxlen : p_size);
if (p_size <= ret && maxlen != ret)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strnlen, FORTIFY_READ);
return ret;
}
@@ -214,7 +253,7 @@ __kernel_size_t __fortify_strlen(const char * const POS p)
return __underlying_strlen(p);
ret = strnlen(p, p_size);
if (p_size <= ret)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strlen, FORTIFY_READ);
return ret;
}
@@ -256,7 +295,7 @@ __FORTIFY_INLINE size_t strlcpy(char * const POS p, const char * const POS q, si
}
if (size) {
if (len >= p_size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strlcpy, FORTIFY_WRITE);
__underlying_memcpy(p, q, len);
p[len] = '\0';
}
@@ -334,7 +373,7 @@ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, s
* p_size.
*/
if (len > p_size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strscpy, FORTIFY_WRITE);
/*
* We can now safely call vanilla strscpy because we are protected from:
@@ -392,7 +431,7 @@ size_t strlcat(char * const POS p, const char * const POS q, size_t avail)
/* Give up if string is already overflowed. */
if (p_size <= p_len)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_READ);
if (actual >= avail) {
copy_len = avail - p_len - 1;
@@ -401,7 +440,7 @@ size_t strlcat(char * const POS p, const char * const POS q, size_t avail)
/* Give up if copy will overflow. */
if (p_size <= actual)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_WRITE);
__underlying_memcpy(p + p_len, q, copy_len);
p[actual] = '\0';
@@ -430,7 +469,7 @@ char *strcat(char * const POS p, const char *q)
const size_t p_size = __member_size(p);
if (strlcat(p, q, p_size) >= p_size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strcat, FORTIFY_WRITE);
return p;
}
@@ -466,7 +505,7 @@ char *strncat(char * const POS p, const char * const POS q, __kernel_size_t coun
p_len = strlen(p);
copy_len = strnlen(q, count);
if (p_size < p_len + copy_len + 1)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strncat, FORTIFY_WRITE);
__underlying_memcpy(p + p_len, q, copy_len);
p[p_len + copy_len] = '\0';
return p;
@@ -507,7 +546,7 @@ __FORTIFY_INLINE void fortify_memset_chk(__kernel_size_t size,
* lengths are unknown.)
*/
if (p_size != SIZE_MAX && p_size < size)
- fortify_panic("memset");
+ fortify_panic(FORTIFY_FUNC_memset, FORTIFY_WRITE);
}
#define __fortify_memset_chk(p, c, size, p_size, p_size_field) ({ \
@@ -561,7 +600,7 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
const size_t q_size,
const size_t p_size_field,
const size_t q_size_field,
- const char *func)
+ const u8 func)
{
if (__builtin_constant_p(size)) {
/*
@@ -605,9 +644,10 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
* (The SIZE_MAX test is to optimize away checks where the buffer
* lengths are unknown.)
*/
- if ((p_size != SIZE_MAX && p_size < size) ||
- (q_size != SIZE_MAX && q_size < size))
- fortify_panic(func);
+ if (p_size != SIZE_MAX && p_size < size)
+ fortify_panic(func, FORTIFY_WRITE);
+ else if (q_size != SIZE_MAX && q_size < size)
+ fortify_panic(func, FORTIFY_READ);
/*
* Warn when writing beyond destination field size.
@@ -640,7 +680,7 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
const size_t __q_size_field = (q_size_field); \
WARN_ONCE(fortify_memcpy_chk(__fortify_size, __p_size, \
__q_size, __p_size_field, \
- __q_size_field, #op), \
+ __q_size_field, FORTIFY_FUNC_ ##op), \
#op ": detected field-spanning write (size %zu) of single %s (size %zu)\n", \
__fortify_size, \
"field \"" #p "\" at " __FILE__ ":" __stringify(__LINE__), \
@@ -707,7 +747,7 @@ __FORTIFY_INLINE void *memscan(void * const POS0 p, int c, __kernel_size_t size)
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_memscan, FORTIFY_READ);
return __real_memscan(p, c, size);
}
@@ -724,7 +764,7 @@ int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t
__read_overflow2();
}
if (p_size < size || q_size < size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_memcmp, FORTIFY_READ);
return __underlying_memcmp(p, q, size);
}
@@ -736,7 +776,7 @@ void *memchr(const void * const POS0 p, int c, __kernel_size_t size)
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_memchr, FORTIFY_READ);
return __underlying_memchr(p, c, size);
}
@@ -748,7 +788,7 @@ __FORTIFY_INLINE void *memchr_inv(const void * const POS0 p, int c, size_t size)
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_memchr_inv, FORTIFY_READ);
return __real_memchr_inv(p, c, size);
}
@@ -761,7 +801,7 @@ __FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_kmemdup, FORTIFY_READ);
return __real_kmemdup(p, size, gfp);
}
@@ -798,7 +838,7 @@ char *strcpy(char * const POS p, const char * const POS q)
__write_overflow();
/* Run-time check for dynamic size overflow. */
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strcpy, FORTIFY_WRITE);
__underlying_memcpy(p, q, size);
return p;
}
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 230020a2e076..96d502e1e578 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -1021,10 +1021,27 @@ EXPORT_SYMBOL(__read_overflow2_field);
void __write_overflow_field(size_t avail, size_t wanted) { }
EXPORT_SYMBOL(__write_overflow_field);
-void fortify_panic(const char *name)
+#define MAKE_FORTIFY_FUNC_NAME(func) [MAKE_FORTIFY_FUNC(func)] = #func
+
+static const char * const fortify_func_name[] = {
+ EACH_FORTIFY_FUNC(MAKE_FORTIFY_FUNC_NAME)
+};
+
+void __fortify_report(const u8 reason)
+{
+ const u8 func = FORTIFY_REASON_FUNC(reason);
+ const bool write = FORTIFY_REASON_DIR(reason);
+ const char *name;
+
+ name = fortify_func_name[min_t(u8, func, FORTIFY_FUNC_UNKNOWN)];
+ WARN(1, "%s: detected buffer %s overflow\n", name, str_read_write(!write));
+}
+EXPORT_SYMBOL(__fortify_report);
+
+void __fortify_panic(const u8 reason)
{
- pr_emerg("detected buffer overflow in %s\n", name);
+ __fortify_report(reason);
BUG();
}
-EXPORT_SYMBOL(fortify_panic);
+EXPORT_SYMBOL(__fortify_panic);
#endif /* CONFIG_FORTIFY_SOURCE */
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index f937be1afe65..2d0a67ce1c51 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -197,6 +197,7 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
* attribute isn't provided in ELF data. Keep 'em sorted.
*/
static const char * const global_noreturns[] = {
+ "__fortify_panic",
"__invalid_creds",
"__module_put_and_kthread_exit",
"__reiserfs_panic",
@@ -208,7 +209,6 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
"do_group_exit",
"do_task_dead",
"ex_handler_msr_mce",
- "fortify_panic",
"kthread_complete_and_exit",
"kthread_exit",
"kunit_try_catch_throw",
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 08/10] fortify: Provide KUnit counters for failure testing
2023-04-07 19:27 [PATCH v2 00/10] fortify: Add KUnit tests for runtime overflows Kees Cook
` (6 preceding siblings ...)
2023-04-07 19:27 ` [PATCH v2 07/10] fortify: Split reporting and avoid passing string pointer Kees Cook
@ 2023-04-07 19:27 ` Kees Cook
2023-04-18 18:20 ` Nick Desaulniers
2023-04-07 19:27 ` [PATCH v2 09/10] fortify: Add KUnit tests for runtime overflows Kees Cook
2023-04-07 19:27 ` [PATCH v2 10/10] fortify: Improve buffer overflow reporting Kees Cook
9 siblings, 1 reply; 27+ messages in thread
From: Kees Cook @ 2023-04-07 19:27 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Nick Desaulniers, Liam Howlett,
Vlastimil Babka, Dan Williams, Rasmus Villemoes, Yury Norov,
Jason A. Donenfeld, Sander Vanheule, Eric Biggers,
Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
The standard C string APIs were not designed to have a failure mode;
they were expected to always succeed without memory safety issues.
Normally, CONFIG_FORTIFY_SOURCE will use fortify_panic() to stop
processing, as truncating a read or write may provide an even worse
system state. However, this creates a problem for testing under things
like KUnit, which needs a way to survive failures.
When building with CONFIG_KUNIT, provide a failure path for all users
for fortify_panic, and track whether the failure was a read overflow or
a write overflow, for KUnit tests to examine. Inspired by similar logic
in the slab tests.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/linux/fortify-string.h | 45 +++++++++++++++++++---------------
lib/fortify_kunit.c | 44 +++++++++++++++++++++++++++++++++
lib/string_helpers.c | 2 ++
3 files changed, 71 insertions(+), 20 deletions(-)
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index 19906b45fb98..5d04c0e95854 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -15,8 +15,12 @@
#define FORTIFY_REASON(func, write) (FIELD_PREP(BIT(0), write) | \
FIELD_PREP(GENMASK(7, 1), func))
-#define fortify_panic(func, write) \
+#ifdef FORTIFY_KUNIT_OVERRIDE
+# define fortify_panic kunit_fortify_panic
+#else
+# define fortify_panic(func, write, retfail) \
__fortify_panic(FORTIFY_REASON(func, write))
+#endif
#define FORTIFY_READ 0
#define FORTIFY_WRITE 1
@@ -186,7 +190,7 @@ char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
if (__compiletime_lessthan(p_size, size))
__write_overflow();
if (p_size < size)
- fortify_panic(FORTIFY_FUNC_strncpy, FORTIFY_WRITE);
+ fortify_panic(FORTIFY_FUNC_strncpy, FORTIFY_WRITE, p);
return __underlying_strncpy(p, q, size);
}
@@ -217,7 +221,7 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size
/* Do not check characters beyond the end of p. */
ret = __real_strnlen(p, maxlen < p_size ? maxlen : p_size);
if (p_size <= ret && maxlen != ret)
- fortify_panic(FORTIFY_FUNC_strnlen, FORTIFY_READ);
+ fortify_panic(FORTIFY_FUNC_strnlen, FORTIFY_READ, ret);
return ret;
}
@@ -253,7 +257,7 @@ __kernel_size_t __fortify_strlen(const char * const POS p)
return __underlying_strlen(p);
ret = strnlen(p, p_size);
if (p_size <= ret)
- fortify_panic(FORTIFY_FUNC_strlen, FORTIFY_READ);
+ fortify_panic(FORTIFY_FUNC_strlen, FORTIFY_READ, ret);
return ret;
}
@@ -295,7 +299,7 @@ __FORTIFY_INLINE size_t strlcpy(char * const POS p, const char * const POS q, si
}
if (size) {
if (len >= p_size)
- fortify_panic(FORTIFY_FUNC_strlcpy, FORTIFY_WRITE);
+ fortify_panic(FORTIFY_FUNC_strlcpy, FORTIFY_WRITE, q_len);
__underlying_memcpy(p, q, len);
p[len] = '\0';
}
@@ -373,7 +377,7 @@ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, s
* p_size.
*/
if (len > p_size)
- fortify_panic(FORTIFY_FUNC_strscpy, FORTIFY_WRITE);
+ fortify_panic(FORTIFY_FUNC_strscpy, FORTIFY_WRITE, -E2BIG);
/*
* We can now safely call vanilla strscpy because we are protected from:
@@ -431,7 +435,7 @@ size_t strlcat(char * const POS p, const char * const POS q, size_t avail)
/* Give up if string is already overflowed. */
if (p_size <= p_len)
- fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_READ);
+ fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_READ, wanted);
if (actual >= avail) {
copy_len = avail - p_len - 1;
@@ -440,7 +444,7 @@ size_t strlcat(char * const POS p, const char * const POS q, size_t avail)
/* Give up if copy will overflow. */
if (p_size <= actual)
- fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_WRITE);
+ fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_WRITE, wanted);
__underlying_memcpy(p + p_len, q, copy_len);
p[actual] = '\0';
@@ -469,7 +473,7 @@ char *strcat(char * const POS p, const char *q)
const size_t p_size = __member_size(p);
if (strlcat(p, q, p_size) >= p_size)
- fortify_panic(FORTIFY_FUNC_strcat, FORTIFY_WRITE);
+ fortify_panic(FORTIFY_FUNC_strcat, FORTIFY_WRITE, p);
return p;
}
@@ -505,13 +509,13 @@ char *strncat(char * const POS p, const char * const POS q, __kernel_size_t coun
p_len = strlen(p);
copy_len = strnlen(q, count);
if (p_size < p_len + copy_len + 1)
- fortify_panic(FORTIFY_FUNC_strncat, FORTIFY_WRITE);
+ fortify_panic(FORTIFY_FUNC_strncat, FORTIFY_WRITE, p);
__underlying_memcpy(p + p_len, q, copy_len);
p[p_len + copy_len] = '\0';
return p;
}
-__FORTIFY_INLINE void fortify_memset_chk(__kernel_size_t size,
+__FORTIFY_INLINE bool fortify_memset_chk(__kernel_size_t size,
const size_t p_size,
const size_t p_size_field)
{
@@ -546,7 +550,8 @@ __FORTIFY_INLINE void fortify_memset_chk(__kernel_size_t size,
* lengths are unknown.)
*/
if (p_size != SIZE_MAX && p_size < size)
- fortify_panic(FORTIFY_FUNC_memset, FORTIFY_WRITE);
+ fortify_panic(FORTIFY_FUNC_memset, FORTIFY_WRITE, true);
+ return false;
}
#define __fortify_memset_chk(p, c, size, p_size, p_size_field) ({ \
@@ -645,9 +650,9 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
* lengths are unknown.)
*/
if (p_size != SIZE_MAX && p_size < size)
- fortify_panic(func, FORTIFY_WRITE);
+ fortify_panic(func, FORTIFY_WRITE, true);
else if (q_size != SIZE_MAX && q_size < size)
- fortify_panic(func, FORTIFY_READ);
+ fortify_panic(func, FORTIFY_READ, true);
/*
* Warn when writing beyond destination field size.
@@ -747,7 +752,7 @@ __FORTIFY_INLINE void *memscan(void * const POS0 p, int c, __kernel_size_t size)
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(FORTIFY_FUNC_memscan, FORTIFY_READ);
+ fortify_panic(FORTIFY_FUNC_memscan, FORTIFY_READ, NULL);
return __real_memscan(p, c, size);
}
@@ -764,7 +769,7 @@ int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t
__read_overflow2();
}
if (p_size < size || q_size < size)
- fortify_panic(FORTIFY_FUNC_memcmp, FORTIFY_READ);
+ fortify_panic(FORTIFY_FUNC_memcmp, FORTIFY_READ, INT_MIN);
return __underlying_memcmp(p, q, size);
}
@@ -776,7 +781,7 @@ void *memchr(const void * const POS0 p, int c, __kernel_size_t size)
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(FORTIFY_FUNC_memchr, FORTIFY_READ);
+ fortify_panic(FORTIFY_FUNC_memchr, FORTIFY_READ, NULL);
return __underlying_memchr(p, c, size);
}
@@ -788,7 +793,7 @@ __FORTIFY_INLINE void *memchr_inv(const void * const POS0 p, int c, size_t size)
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(FORTIFY_FUNC_memchr_inv, FORTIFY_READ);
+ fortify_panic(FORTIFY_FUNC_memchr_inv, FORTIFY_READ, NULL);
return __real_memchr_inv(p, c, size);
}
@@ -801,7 +806,7 @@ __FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(FORTIFY_FUNC_kmemdup, FORTIFY_READ);
+ fortify_panic(FORTIFY_FUNC_kmemdup, FORTIFY_READ, NULL);
return __real_kmemdup(p, size, gfp);
}
@@ -838,7 +843,7 @@ char *strcpy(char * const POS p, const char * const POS q)
__write_overflow();
/* Run-time check for dynamic size overflow. */
if (p_size < size)
- fortify_panic(FORTIFY_FUNC_strcpy, FORTIFY_WRITE);
+ fortify_panic(FORTIFY_FUNC_strcpy, FORTIFY_WRITE, p);
__underlying_memcpy(p, q, size);
return p;
}
diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c
index 524132f33cf0..ea2b39f279c2 100644
--- a/lib/fortify_kunit.c
+++ b/lib/fortify_kunit.c
@@ -15,12 +15,28 @@
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+/* Call kunit_fortify_panic() instead of fortify_panic() */
+#define FORTIFY_KUNIT_OVERRIDE
+void fortify_add_kunit_error(int write);
+#define kunit_fortify_panic(func, write, retfail) \
+ do { \
+ __fortify_report(FORTIFY_REASON(func, write)); \
+ fortify_add_kunit_error(write); \
+ return (retfail); \
+ } while (0)
+
#include <kunit/test.h>
+#include <kunit/test-bug.h>
#include <linux/device.h>
#include <linux/slab.h>
#include <linux/string.h>
#include <linux/vmalloc.h>
+static struct kunit_resource read_resource;
+static struct kunit_resource write_resource;
+static int fortify_read_overflows;
+static int fortify_write_overflows;
+
static const char array_of_10[] = "this is 10";
static const char *ptr_of_11 = "this is 11!";
static char array_unknown[] = "compiler thinks I might change";
@@ -30,6 +46,25 @@ static char array_unknown[] = "compiler thinks I might change";
# define __compiletime_strlen __builtin_strlen
#endif
+void fortify_add_kunit_error(int write)
+{
+ struct kunit_resource *resource;
+ struct kunit *current_test;
+
+ current_test = kunit_get_current_test();
+ if (!current_test)
+ return;
+
+ resource = kunit_find_named_resource(current_test,
+ write ? "fortify_write_overflows"
+ : "fortify_read_overflows");
+ if (!resource)
+ return;
+
+ (*(int *)resource->data)++;
+ kunit_put_resource(resource);
+}
+
static void known_sizes_test(struct kunit *test)
{
KUNIT_EXPECT_EQ(test, __compiletime_strlen("88888888"), 8);
@@ -317,6 +352,15 @@ static int fortify_test_init(struct kunit *test)
if (!IS_ENABLED(CONFIG_FORTIFY_SOURCE))
kunit_skip(test, "Not built with CONFIG_FORTIFY_SOURCE=y");
+ fortify_read_overflows = 0;
+ kunit_add_named_resource(test, NULL, NULL, &read_resource,
+ "fortify_read_overflows",
+ &fortify_read_overflows);
+ fortify_write_overflows = 0;
+ kunit_add_named_resource(test, NULL, NULL, &write_resource,
+ "fortify_write_overflows",
+ &fortify_write_overflows);
+
return 0;
}
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 96d502e1e578..38edde20e61b 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -18,6 +18,8 @@
#include <linux/slab.h>
#include <linux/string.h>
#include <linux/string_helpers.h>
+#include <kunit/test.h>
+#include <kunit/test-bug.h>
/**
* string_get_size - get the size in the specified units
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 09/10] fortify: Add KUnit tests for runtime overflows
2023-04-07 19:27 [PATCH v2 00/10] fortify: Add KUnit tests for runtime overflows Kees Cook
` (7 preceding siblings ...)
2023-04-07 19:27 ` [PATCH v2 08/10] fortify: Provide KUnit counters for failure testing Kees Cook
@ 2023-04-07 19:27 ` Kees Cook
2023-04-08 0:33 ` kernel test robot
2023-04-07 19:27 ` [PATCH v2 10/10] fortify: Improve buffer overflow reporting Kees Cook
9 siblings, 1 reply; 27+ messages in thread
From: Kees Cook @ 2023-04-07 19:27 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Nick Desaulniers, Liam Howlett,
Vlastimil Babka, Dan Williams, Rasmus Villemoes, Yury Norov,
Jason A. Donenfeld, Sander Vanheule, Eric Biggers,
Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
With fortify overflows able to be redirected, we can use KUnit to
exercise the overflow conditions. Add tests for every API covered by
CONFIG_FORTIFY_SOURCE.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
lib/fortify_kunit.c | 673 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 673 insertions(+)
diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c
index ea2b39f279c2..3206fe723110 100644
--- a/lib/fortify_kunit.c
+++ b/lib/fortify_kunit.c
@@ -347,6 +347,663 @@ DEFINE_ALLOC_SIZE_TEST_PAIR(kvmalloc)
} while (0)
DEFINE_ALLOC_SIZE_TEST_PAIR(devm_kmalloc)
+/*
+ * We can't have an array at the end of a structure or else
+ * builds without -fstrict-flex-arrays=3 will report them as
+ * being an unknown length. Additionally, add bytes before
+ * and after the string to catch over/underflows if tests
+ * fail.
+ */
+struct fortify_padding {
+ unsigned long bytes_before;
+ char buf[32];
+ unsigned long bytes_after;
+};
+/* Force compiler into not being able to resolve size at compile-time. */
+static volatile int unconst;
+
+static void strlen_test(struct kunit *test)
+{
+ struct fortify_padding pad = { };
+ int i, end = sizeof(pad.buf) - 1;
+
+ /* Fill 31 bytes with valid characters. */
+ for (i = 0; i < sizeof(pad.buf) - 1; i++)
+ pad.buf[i] = i + '0';
+ /* Trailing bytes are still %NUL. */
+ KUNIT_EXPECT_EQ(test, pad.buf[end], '\0');
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* String is terminated, so strlen() is valid. */
+ KUNIT_EXPECT_EQ(test, strlen(pad.buf), end);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+
+ /* Make string unterminated, and recount. */
+ pad.buf[end] = 'A';
+ end = sizeof(pad.buf);
+ KUNIT_EXPECT_EQ(test, strlen(pad.buf), end);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1);
+}
+
+static void strnlen_test(struct kunit *test)
+{
+ struct fortify_padding pad = { };
+ int i, end = sizeof(pad.buf) - 1;
+
+ /* Fill 31 bytes with valid characters. */
+ for (i = 0; i < sizeof(pad.buf) - 1; i++)
+ pad.buf[i] = i + '0';
+ /* Trailing bytes are still %NUL. */
+ KUNIT_EXPECT_EQ(test, pad.buf[end], '\0');
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* String is terminated, so strnlen() is valid. */
+ KUNIT_EXPECT_EQ(test, strnlen(pad.buf, sizeof(pad.buf)), end);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ /* A truncated strnlen() will be safe, too. */
+ KUNIT_EXPECT_EQ(test, strnlen(pad.buf, sizeof(pad.buf) / 2),
+ sizeof(pad.buf) / 2);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+
+ /* Make string unterminated, and recount. */
+ pad.buf[end] = 'A';
+ end = sizeof(pad.buf);
+ /* Reading beyond with strncpy() will fail. */
+ KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 1), end);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1);
+ KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 2), end);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
+
+ /* Early-truncated is safe still, though. */
+ KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end), end);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
+
+ end = sizeof(pad.buf) / 2;
+ KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end), end);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
+}
+
+static void strcpy_test(struct kunit *test)
+{
+ struct fortify_padding pad = { };
+ char src[sizeof(pad.buf) + 1] = { };
+ int i;
+
+ /* Fill 31 bytes with valid characters. */
+ for (i = 0; i < sizeof(src) - 2; i++)
+ src[i] = i + '0';
+
+ /* Destination is %NUL-filled to start with. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_before, 0);
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* Legitimate strcpy() 1 less than of max size. */
+ KUNIT_ASSERT_TRUE(test, strcpy(pad.buf, src)
+ == pad.buf);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* Only last byte should be %NUL */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+
+ src[sizeof(src) - 2] = 'A';
+ /* But now we trip the overflow checking. */
+ KUNIT_ASSERT_TRUE(test, strcpy(pad.buf, src)
+ == pad.buf);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 1);
+ /* Trailing %NUL -- thanks to FORTIFY. */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ /* And we will not have gone beyond. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ src[sizeof(src) - 1] = 'A';
+ /* And for sure now, two bytes past. */
+ KUNIT_ASSERT_TRUE(test, strcpy(pad.buf, src)
+ == pad.buf);
+ /*
+ * Which trips both the strlen() on the unterminated src,
+ * and the resulting copy attempt.
+ */
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 2);
+ /* Trailing %NUL -- thanks to FORTIFY. */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ /* And we will not have gone beyond. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+}
+
+static void strncpy_test(struct kunit *test)
+{
+ struct fortify_padding pad = { };
+ char src[] = "Copy me fully into a small buffer and I will overflow!";
+
+ /* Destination is %NUL-filled to start with. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_before, 0);
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* Legitimate strncpy() 1 less than of max size. */
+ KUNIT_ASSERT_TRUE(test, strncpy(pad.buf, src,
+ sizeof(pad.buf) + unconst - 1)
+ == pad.buf);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* Only last byte should be %NUL */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+
+ /* Legitimate (though unterminated) max-size strncpy. */
+ KUNIT_ASSERT_TRUE(test, strncpy(pad.buf, src,
+ sizeof(pad.buf) + unconst)
+ == pad.buf);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* No trailing %NUL -- thanks strncpy API. */
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ /* But we will not have gone beyond. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* Now verify that FORTIFY is working... */
+ KUNIT_ASSERT_TRUE(test, strncpy(pad.buf, src,
+ sizeof(pad.buf) + unconst + 1)
+ == pad.buf);
+ /* Should catch the overflow. */
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 1);
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ /* And we will not have gone beyond. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* And further... */
+ KUNIT_ASSERT_TRUE(test, strncpy(pad.buf, src,
+ sizeof(pad.buf) + unconst + 2)
+ == pad.buf);
+ /* Should catch the overflow. */
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 2);
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ /* And we will not have gone beyond. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+}
+
+static void strlcpy_test(struct kunit *test)
+{
+ struct fortify_padding pad = { };
+ char src[] = "Copy me fully into a small buffer and I will overflow!";
+ char tiny[4] = "abcd";
+
+ /* Destination is %NUL-filled to start with. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_before, 0);
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* Legitimate strlcpy() 1 less than of max size. */
+ KUNIT_ASSERT_EQ(test, strlcpy(pad.buf, src,
+ sizeof(pad.buf) + unconst - 1),
+ sizeof(src) - 1);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* Keeping space for %NUL, last two bytes should be %NUL */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+
+ /* Legitimate max-size strlcpy. */
+ KUNIT_ASSERT_EQ(test, strlcpy(pad.buf, src,
+ sizeof(pad.buf) + unconst),
+ sizeof(src) - 1);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* A trailing %NUL will exist. */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+
+ /* Now verify that FORTIFY is working... */
+ KUNIT_ASSERT_EQ(test, strlcpy(pad.buf, src,
+ sizeof(pad.buf) + unconst + 1),
+ sizeof(src) - 1);
+ /* Should catch the overflow. */
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 1);
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ /* And we will not have gone beyond. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* And much further... */
+ KUNIT_ASSERT_EQ(test, strlcpy(pad.buf, src,
+ sizeof(src) * 2 + unconst),
+ sizeof(src) - 1);
+ /* Should catch the overflow. */
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 2);
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ /* And we will not have gone beyond. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* Catch over-read. */
+ KUNIT_ASSERT_EQ(test, strlcpy(pad.buf, tiny,
+ sizeof(pad.buf) + unconst),
+ sizeof(tiny));
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 2);
+}
+
+static void strscpy_test(struct kunit *test)
+{
+ struct fortify_padding pad = { };
+ char src[] = "Copy me fully into a small buffer and I will overflow!";
+
+ /* Destination is %NUL-filled to start with. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_before, 0);
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* Legitimate strscpy() 1 less than of max size. */
+ KUNIT_ASSERT_EQ(test, strscpy(pad.buf, src,
+ sizeof(pad.buf) + unconst - 1),
+ -E2BIG);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* Keeping space for %NUL, last two bytes should be %NUL */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+
+ /* Legitimate max-size strscpy. */
+ KUNIT_ASSERT_EQ(test, strscpy(pad.buf, src,
+ sizeof(pad.buf) + unconst),
+ -E2BIG);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* A trailing %NUL will exist. */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+
+ /* Now verify that FORTIFY is working... */
+ KUNIT_ASSERT_EQ(test, strscpy(pad.buf, src,
+ sizeof(pad.buf) + unconst + 1),
+ -E2BIG);
+ /* Should catch the overflow. */
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 1);
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ /* And we will not have gone beyond. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* And much further... */
+ KUNIT_ASSERT_EQ(test, strscpy(pad.buf, src,
+ sizeof(src) * 2 + unconst),
+ -E2BIG);
+ /* Should catch the overflow. */
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 2);
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ /* And we will not have gone beyond. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+}
+
+static void strcat_test(struct kunit *test)
+{
+ struct fortify_padding pad = { };
+ char src[sizeof(pad.buf) / 2] = { };
+ char one[] = "A";
+ char two[] = "BC";
+ int i;
+
+ /* Fill 15 bytes with valid characters. */
+ for (i = 0; i < sizeof(src) - 1; i++)
+ src[i] = i + 'A';
+
+ /* Destination is %NUL-filled to start with. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_before, 0);
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* Legitimate strcat() using less than half max size. */
+ KUNIT_ASSERT_TRUE(test, strcat(pad.buf, src) == pad.buf);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* Legitimate strcat() now 2 bytes shy of end. */
+ KUNIT_ASSERT_TRUE(test, strcat(pad.buf, src) == pad.buf);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* Last two bytes should be %NUL */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+
+ /* Add one more character to the end. */
+ KUNIT_ASSERT_TRUE(test, strcat(pad.buf, one) == pad.buf);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* Last byte should be %NUL */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+
+ /* And this one char will overflow. */
+ KUNIT_ASSERT_TRUE(test, strcat(pad.buf, one) == pad.buf);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 1);
+ /* Last byte should be %NUL thanks to FORTIFY. */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* And adding two will overflow more. */
+ KUNIT_ASSERT_TRUE(test, strcat(pad.buf, two) == pad.buf);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 2);
+ /* Last byte should be %NUL thanks to FORTIFY. */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+}
+
+static void strncat_test(struct kunit *test)
+{
+ struct fortify_padding pad = { };
+ char src[sizeof(pad.buf)] = { };
+ int i, partial;
+
+ /* Fill 31 bytes with valid characters. */
+ partial = sizeof(src) / 2 - 1;
+ for (i = 0; i < partial; i++)
+ src[i] = i + 'A';
+
+ /* Destination is %NUL-filled to start with. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_before, 0);
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* Legitimate strncat() using less than half max size. */
+ KUNIT_ASSERT_TRUE(test, strncat(pad.buf, src, partial) == pad.buf);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* Legitimate strncat() now 2 bytes shy of end. */
+ KUNIT_ASSERT_TRUE(test, strncat(pad.buf, src, partial) == pad.buf);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* Last two bytes should be %NUL */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+
+ /* Add one more character to the end. */
+ KUNIT_ASSERT_TRUE(test, strncat(pad.buf, src, 1) == pad.buf);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* Last byte should be %NUL */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+
+ /* And this one char will overflow. */
+ KUNIT_ASSERT_TRUE(test, strncat(pad.buf, src, 1) == pad.buf);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 1);
+ /* Last byte should be %NUL thanks to FORTIFY. */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* And adding two will overflow more. */
+ KUNIT_ASSERT_TRUE(test, strncat(pad.buf, src, 2) == pad.buf);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 2);
+ /* Last byte should be %NUL thanks to FORTIFY. */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* Force an unterminated destination, and overflow. */
+ pad.buf[sizeof(pad.buf) - 1] = 'A';
+ KUNIT_ASSERT_TRUE(test, strncat(pad.buf, src, 1) == pad.buf);
+ /* This will have tripped both strlen() and strcat(). */
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 3);
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+ /* But we should not go beyond the end. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+}
+
+static void strlcat_test(struct kunit *test)
+{
+ struct fortify_padding pad = { };
+ char src[sizeof(pad.buf)] = { };
+ int i, partial;
+ int len = sizeof(pad.buf) + unconst;
+
+ /* Fill 15 bytes with valid characters. */
+ partial = sizeof(src) / 2 - 1;
+ for (i = 0; i < partial; i++)
+ src[i] = i + 'A';
+
+ /* Destination is %NUL-filled to start with. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_before, 0);
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* Legitimate strlcat() using less than half max size. */
+ KUNIT_ASSERT_EQ(test, strlcat(pad.buf, src, len), partial);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* Legitimate strlcat() now 2 bytes shy of end. */
+ KUNIT_ASSERT_EQ(test, strlcat(pad.buf, src, len), partial * 2);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* Last two bytes should be %NUL */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+
+ /* Add one more character to the end. */
+ KUNIT_ASSERT_EQ(test, strlcat(pad.buf, "Q", len), partial * 2 + 1);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0);
+ /* Last byte should be %NUL */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+
+ /* And this one char will overflow. */
+ KUNIT_ASSERT_EQ(test, strlcat(pad.buf, "V", len * 2), len);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 1);
+ /* Last byte should be %NUL thanks to FORTIFY. */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* And adding two will overflow more. */
+ KUNIT_ASSERT_EQ(test, strlcat(pad.buf, "QQ", len * 2), len + 1);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 2);
+ /* Last byte should be %NUL thanks to FORTIFY. */
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* Force an unterminated destination, and overflow. */
+ pad.buf[sizeof(pad.buf) - 1] = 'A';
+ KUNIT_ASSERT_EQ(test, strlcat(pad.buf, "TT", len * 2), len + 2);
+ /* This will have tripped both strlen() and strlcat(). */
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 2);
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 2], '\0');
+ KUNIT_EXPECT_NE(test, pad.buf[sizeof(pad.buf) - 3], '\0');
+ /* But we should not go beyond the end. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+
+ /* Force an unterminated source, and overflow. */
+ memset(src, 'B', sizeof(src));
+ pad.buf[sizeof(pad.buf) - 1] = '\0';
+ KUNIT_ASSERT_EQ(test, strlcat(pad.buf, src, len * 3), len - 1 + sizeof(src));
+ /* This will have tripped both strlen() and strlcat(). */
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 3);
+ KUNIT_EXPECT_EQ(test, fortify_write_overflows, 3);
+ KUNIT_EXPECT_EQ(test, pad.buf[sizeof(pad.buf) - 1], '\0');
+ /* But we should not go beyond the end. */
+ KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
+}
+
+static void memscan_test(struct kunit *test)
+{
+ char haystack[] = "Where oh where is my memory range?";
+ char *mem = haystack + strlen("Where oh where is ");
+ char needle = 'm';
+ size_t len = sizeof(haystack) + unconst;
+
+ KUNIT_ASSERT_PTR_EQ(test, memscan(haystack, needle, len),
+ mem);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ /* Catch too-large range. */
+ KUNIT_ASSERT_PTR_EQ(test, memscan(haystack, needle, len + 1),
+ NULL);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1);
+ KUNIT_ASSERT_PTR_EQ(test, memscan(haystack, needle, len * 2),
+ NULL);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
+}
+
+static void memchr_test(struct kunit *test)
+{
+ char haystack[] = "Where oh where is my memory range?";
+ char *mem = haystack + strlen("Where oh where is ");
+ char needle = 'm';
+ size_t len = sizeof(haystack) + unconst;
+
+ KUNIT_ASSERT_PTR_EQ(test, memchr(haystack, needle, len),
+ mem);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ /* Catch too-large range. */
+ KUNIT_ASSERT_PTR_EQ(test, memchr(haystack, needle, len + 1),
+ NULL);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1);
+ KUNIT_ASSERT_PTR_EQ(test, memchr(haystack, needle, len * 2),
+ NULL);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
+}
+
+static void memchr_inv_test(struct kunit *test)
+{
+ char haystack[] = "Where oh where is my memory range?";
+ char *mem = haystack + 1;
+ char needle = 'W';
+ size_t len = sizeof(haystack) + unconst;
+
+ /* Normal search is okay. */
+ KUNIT_ASSERT_PTR_EQ(test, memchr_inv(haystack, needle, len),
+ mem);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ /* Catch too-large range. */
+ KUNIT_ASSERT_PTR_EQ(test, memchr_inv(haystack, needle, len + 1),
+ NULL);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1);
+ KUNIT_ASSERT_PTR_EQ(test, memchr_inv(haystack, needle, len * 2),
+ NULL);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
+}
+
+static void memcmp_test(struct kunit *test)
+{
+ char one[] = "My mind is going ...";
+ char two[] = "My mind is going ... I can feel it.";
+ size_t one_len = sizeof(one) + unconst - 1;
+ size_t two_len = sizeof(two) + unconst - 1;
+
+ /* We match the first string (ignoring the %NUL). */
+ KUNIT_ASSERT_EQ(test, memcmp(one, two, one_len), 0);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ /* Still in bounds, but no longer matching. */
+ KUNIT_ASSERT_EQ(test, memcmp(one, two, one_len + 1), -32);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+
+ /* Catch too-large ranges. */
+ KUNIT_ASSERT_EQ(test, memcmp(one, two, one_len + 2), INT_MIN);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1);
+
+ KUNIT_ASSERT_EQ(test, memcmp(two, one, two_len + 2), INT_MIN);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
+}
+
+static void kmemdup_test(struct kunit *test)
+{
+ char src[] = "I got Doom running on it!";
+ char *copy;
+ size_t len = sizeof(src) + unconst;
+
+ /* Copy is within bounds. */
+ copy = kmemdup(src, len, GFP_KERNEL);
+ KUNIT_EXPECT_NOT_NULL(test, copy);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ kfree(copy);
+
+ /* Without %NUL. */
+ copy = kmemdup(src, len - 1, GFP_KERNEL);
+ KUNIT_EXPECT_NOT_NULL(test, copy);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ kfree(copy);
+
+ /* Tiny bounds. */
+ copy = kmemdup(src, 1, GFP_KERNEL);
+ KUNIT_EXPECT_NOT_NULL(test, copy);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
+ kfree(copy);
+
+ /* Out of bounds by 1 byte. */
+ copy = kmemdup(src, len + 1, GFP_KERNEL);
+ KUNIT_EXPECT_NULL(test, copy);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1);
+ kfree(copy);
+
+ /* Way out of bounds. */
+ copy = kmemdup(src, len * 2, GFP_KERNEL);
+ KUNIT_EXPECT_NULL(test, copy);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
+ kfree(copy);
+
+ /* Starting offset causing out of bounds. */
+ copy = kmemdup(src + 1, len, GFP_KERNEL);
+ KUNIT_EXPECT_NULL(test, copy);
+ KUNIT_EXPECT_EQ(test, fortify_read_overflows, 3);
+ kfree(copy);
+}
+
static int fortify_test_init(struct kunit *test)
{
if (!IS_ENABLED(CONFIG_FORTIFY_SOURCE))
@@ -375,6 +1032,22 @@ static struct kunit_case fortify_test_cases[] = {
KUNIT_CASE(alloc_size_kvmalloc_dynamic_test),
KUNIT_CASE(alloc_size_devm_kmalloc_const_test),
KUNIT_CASE(alloc_size_devm_kmalloc_dynamic_test),
+ KUNIT_CASE(strlen_test),
+ KUNIT_CASE(strnlen_test),
+ KUNIT_CASE(strcpy_test),
+ KUNIT_CASE(strncpy_test),
+ KUNIT_CASE(strlcpy_test),
+ KUNIT_CASE(strscpy_test),
+ KUNIT_CASE(strcat_test),
+ KUNIT_CASE(strncat_test),
+ KUNIT_CASE(strlcat_test),
+ /* skip memset: performs bounds checking on whole structs */
+ /* skip memcpy: still using warn-and-clobber instead of hard-fail */
+ KUNIT_CASE(memscan_test),
+ KUNIT_CASE(memchr_test),
+ KUNIT_CASE(memchr_inv_test),
+ KUNIT_CASE(memcmp_test),
+ KUNIT_CASE(kmemdup_test),
{}
};
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 10/10] fortify: Improve buffer overflow reporting
2023-04-07 19:27 [PATCH v2 00/10] fortify: Add KUnit tests for runtime overflows Kees Cook
` (8 preceding siblings ...)
2023-04-07 19:27 ` [PATCH v2 09/10] fortify: Add KUnit tests for runtime overflows Kees Cook
@ 2023-04-07 19:27 ` Kees Cook
9 siblings, 0 replies; 27+ messages in thread
From: Kees Cook @ 2023-04-07 19:27 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Nick Desaulniers, Liam Howlett,
Vlastimil Babka, Dan Williams, Rasmus Villemoes, Yury Norov,
Jason A. Donenfeld, Sander Vanheule, Eric Biggers,
Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
Improve the reporting of buffer overflows under CONFIG_FORTIFY_SOURCE to
help accelerate debugging efforts. The calculations are all just sitting
in registers anyway, so pass them along to the function to be reported.
For example, before:
detected buffer overflow in memcpy
and after:
memcpy: detected buffer overflow: 4096 byte read from buffer of size 1
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/arm/boot/compressed/misc.c | 2 +-
arch/x86/boot/compressed/misc.c | 2 +-
include/linux/fortify-string.h | 61 ++++++++++++++++++---------------
lib/fortify_kunit.c | 4 +--
lib/string_helpers.c | 9 ++---
5 files changed, 42 insertions(+), 36 deletions(-)
diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
index f31e2c949089..5487f64d8c3d 100644
--- a/arch/arm/boot/compressed/misc.c
+++ b/arch/arm/boot/compressed/misc.c
@@ -160,7 +160,7 @@ decompress_kernel(unsigned long output_start, unsigned long free_mem_ptr_p,
putstr(" done, booting the kernel.\n");
}
-void __fortify_panic(const u8 reason)
+void __fortify_panic(const u8 reason, size_t avail, size_t size)
{
error("detected buffer overflow");
}
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index aa45e7529a40..c1dc12abd6d9 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -470,7 +470,7 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
return output + entry_offset;
}
-void __fortify_panic(const u8 reason)
+void __fortify_panic(const u8 reason, size_t avail, size_t size)
{
error("detected buffer overflow");
}
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index 5d04c0e95854..a0002740d2a7 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -18,8 +18,8 @@
#ifdef FORTIFY_KUNIT_OVERRIDE
# define fortify_panic kunit_fortify_panic
#else
-# define fortify_panic(func, write, retfail) \
- __fortify_panic(FORTIFY_REASON(func, write))
+# define fortify_panic(func, write, avail, size, retfail) \
+ __fortify_panic(FORTIFY_REASON(func, write), avail, size)
#endif
#define FORTIFY_READ 0
@@ -51,8 +51,8 @@ enum fortify_func {
EACH_FORTIFY_FUNC(MAKE_FORTIFY_FUNC)
};
-void __fortify_report(const u8 reason);
-void __fortify_panic(const u8 reason) __cold __noreturn;
+void __fortify_report(const u8 reason, const size_t avail, const size_t size);
+void __fortify_panic(const u8 reason, const size_t avail, const size_t size) __cold __noreturn;
void __read_overflow(void) __compiletime_error("detected read beyond size of object (1st parameter)");
void __read_overflow2(void) __compiletime_error("detected read beyond size of object (2nd parameter)");
void __read_overflow2_field(size_t avail, size_t wanted) __compiletime_warning("detected read beyond size of field (2nd parameter); maybe use struct_group()?");
@@ -190,7 +190,7 @@ char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
if (__compiletime_lessthan(p_size, size))
__write_overflow();
if (p_size < size)
- fortify_panic(FORTIFY_FUNC_strncpy, FORTIFY_WRITE, p);
+ fortify_panic(FORTIFY_FUNC_strncpy, FORTIFY_WRITE, p_size, size, p);
return __underlying_strncpy(p, q, size);
}
@@ -221,7 +221,7 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size
/* Do not check characters beyond the end of p. */
ret = __real_strnlen(p, maxlen < p_size ? maxlen : p_size);
if (p_size <= ret && maxlen != ret)
- fortify_panic(FORTIFY_FUNC_strnlen, FORTIFY_READ, ret);
+ fortify_panic(FORTIFY_FUNC_strnlen, FORTIFY_READ, p_size, ret + 1, ret);
return ret;
}
@@ -257,7 +257,7 @@ __kernel_size_t __fortify_strlen(const char * const POS p)
return __underlying_strlen(p);
ret = strnlen(p, p_size);
if (p_size <= ret)
- fortify_panic(FORTIFY_FUNC_strlen, FORTIFY_READ, ret);
+ fortify_panic(FORTIFY_FUNC_strlen, FORTIFY_READ, p_size, ret + 1, ret);
return ret;
}
@@ -298,8 +298,8 @@ __FORTIFY_INLINE size_t strlcpy(char * const POS p, const char * const POS q, si
__write_overflow();
}
if (size) {
- if (len >= p_size)
- fortify_panic(FORTIFY_FUNC_strlcpy, FORTIFY_WRITE, q_len);
+ if (p_size <= len)
+ fortify_panic(FORTIFY_FUNC_strlcpy, FORTIFY_WRITE, p_size, len + 1, q_len);
__underlying_memcpy(p, q, len);
p[len] = '\0';
}
@@ -376,8 +376,8 @@ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, s
* Generate a runtime write overflow error if len is greater than
* p_size.
*/
- if (len > p_size)
- fortify_panic(FORTIFY_FUNC_strscpy, FORTIFY_WRITE, -E2BIG);
+ if (p_size < len)
+ fortify_panic(FORTIFY_FUNC_strscpy, FORTIFY_WRITE, p_size, len, -E2BIG);
/*
* We can now safely call vanilla strscpy because we are protected from:
@@ -435,7 +435,7 @@ size_t strlcat(char * const POS p, const char * const POS q, size_t avail)
/* Give up if string is already overflowed. */
if (p_size <= p_len)
- fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_READ, wanted);
+ fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_READ, p_size, p_len + 1, wanted);
if (actual >= avail) {
copy_len = avail - p_len - 1;
@@ -444,7 +444,7 @@ size_t strlcat(char * const POS p, const char * const POS q, size_t avail)
/* Give up if copy will overflow. */
if (p_size <= actual)
- fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_WRITE, wanted);
+ fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_WRITE, p_size, actual + 1, wanted);
__underlying_memcpy(p + p_len, q, copy_len);
p[actual] = '\0';
@@ -471,9 +471,11 @@ __FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
char *strcat(char * const POS p, const char *q)
{
const size_t p_size = __member_size(p);
+ size_t wanted;
- if (strlcat(p, q, p_size) >= p_size)
- fortify_panic(FORTIFY_FUNC_strcat, FORTIFY_WRITE, p);
+ wanted = strlcat(p, q, p_size);
+ if (p_size <= wanted)
+ fortify_panic(FORTIFY_FUNC_strcat, FORTIFY_WRITE, p_size, wanted + 1, p);
return p;
}
@@ -502,14 +504,15 @@ char *strncat(char * const POS p, const char * const POS q, __kernel_size_t coun
{
const size_t p_size = __member_size(p);
const size_t q_size = __member_size(q);
- size_t p_len, copy_len;
+ size_t p_len, copy_len, total;
if (p_size == SIZE_MAX && q_size == SIZE_MAX)
return __underlying_strncat(p, q, count);
p_len = strlen(p);
copy_len = strnlen(q, count);
- if (p_size < p_len + copy_len + 1)
- fortify_panic(FORTIFY_FUNC_strncat, FORTIFY_WRITE, p);
+ total = p_len + copy_len + 1;
+ if (p_size < total)
+ fortify_panic(FORTIFY_FUNC_strncat, FORTIFY_WRITE, p_size, total, p);
__underlying_memcpy(p + p_len, q, copy_len);
p[p_len + copy_len] = '\0';
return p;
@@ -550,7 +553,7 @@ __FORTIFY_INLINE bool fortify_memset_chk(__kernel_size_t size,
* lengths are unknown.)
*/
if (p_size != SIZE_MAX && p_size < size)
- fortify_panic(FORTIFY_FUNC_memset, FORTIFY_WRITE, true);
+ fortify_panic(FORTIFY_FUNC_memset, FORTIFY_WRITE, p_size, size, true);
return false;
}
@@ -650,9 +653,9 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
* lengths are unknown.)
*/
if (p_size != SIZE_MAX && p_size < size)
- fortify_panic(func, FORTIFY_WRITE, true);
+ fortify_panic(func, FORTIFY_WRITE, p_size, size, true);
else if (q_size != SIZE_MAX && q_size < size)
- fortify_panic(func, FORTIFY_READ, true);
+ fortify_panic(func, FORTIFY_READ, p_size, size, true);
/*
* Warn when writing beyond destination field size.
@@ -752,7 +755,7 @@ __FORTIFY_INLINE void *memscan(void * const POS0 p, int c, __kernel_size_t size)
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(FORTIFY_FUNC_memscan, FORTIFY_READ, NULL);
+ fortify_panic(FORTIFY_FUNC_memscan, FORTIFY_READ, p_size, size, NULL);
return __real_memscan(p, c, size);
}
@@ -768,8 +771,10 @@ int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t
if (__compiletime_lessthan(q_size, size))
__read_overflow2();
}
- if (p_size < size || q_size < size)
- fortify_panic(FORTIFY_FUNC_memcmp, FORTIFY_READ, INT_MIN);
+ if (p_size < size)
+ fortify_panic(FORTIFY_FUNC_memcmp, FORTIFY_READ, p_size, size, INT_MIN);
+ else if (q_size < size)
+ fortify_panic(FORTIFY_FUNC_memcmp, FORTIFY_READ, q_size, size, INT_MIN);
return __underlying_memcmp(p, q, size);
}
@@ -781,7 +786,7 @@ void *memchr(const void * const POS0 p, int c, __kernel_size_t size)
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(FORTIFY_FUNC_memchr, FORTIFY_READ, NULL);
+ fortify_panic(FORTIFY_FUNC_memchr, FORTIFY_READ, p_size, size, NULL);
return __underlying_memchr(p, c, size);
}
@@ -793,7 +798,7 @@ __FORTIFY_INLINE void *memchr_inv(const void * const POS0 p, int c, size_t size)
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(FORTIFY_FUNC_memchr_inv, FORTIFY_READ, NULL);
+ fortify_panic(FORTIFY_FUNC_memchr_inv, FORTIFY_READ, p_size, size, NULL);
return __real_memchr_inv(p, c, size);
}
@@ -806,7 +811,7 @@ __FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(FORTIFY_FUNC_kmemdup, FORTIFY_READ, NULL);
+ fortify_panic(FORTIFY_FUNC_kmemdup, FORTIFY_READ, p_size, size, NULL);
return __real_kmemdup(p, size, gfp);
}
@@ -843,7 +848,7 @@ char *strcpy(char * const POS p, const char * const POS q)
__write_overflow();
/* Run-time check for dynamic size overflow. */
if (p_size < size)
- fortify_panic(FORTIFY_FUNC_strcpy, FORTIFY_WRITE, p);
+ fortify_panic(FORTIFY_FUNC_strcpy, FORTIFY_WRITE, p_size, size, p);
__underlying_memcpy(p, q, size);
return p;
}
diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c
index 3206fe723110..6aac58eb6eb6 100644
--- a/lib/fortify_kunit.c
+++ b/lib/fortify_kunit.c
@@ -18,9 +18,9 @@
/* Call kunit_fortify_panic() instead of fortify_panic() */
#define FORTIFY_KUNIT_OVERRIDE
void fortify_add_kunit_error(int write);
-#define kunit_fortify_panic(func, write, retfail) \
+#define kunit_fortify_panic(func, write, avail, size, retfail) \
do { \
- __fortify_report(FORTIFY_REASON(func, write)); \
+ __fortify_report(FORTIFY_REASON(func, write), avail, size); \
fortify_add_kunit_error(write); \
return (retfail); \
} while (0)
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 38edde20e61b..9a8167535e1f 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -1029,20 +1029,21 @@ static const char * const fortify_func_name[] = {
EACH_FORTIFY_FUNC(MAKE_FORTIFY_FUNC_NAME)
};
-void __fortify_report(const u8 reason)
+void __fortify_report(const u8 reason, const size_t avail, const size_t size)
{
const u8 func = FORTIFY_REASON_FUNC(reason);
const bool write = FORTIFY_REASON_DIR(reason);
const char *name;
name = fortify_func_name[min_t(u8, func, FORTIFY_FUNC_UNKNOWN)];
- WARN(1, "%s: detected buffer %s overflow\n", name, str_read_write(!write));
+ WARN(1, "%s: detected buffer overflow: %zu byte %s buffer of size %zu\n",
+ name, size, str_read_write(!write), avail);
}
EXPORT_SYMBOL(__fortify_report);
-void __fortify_panic(const u8 reason)
+void __fortify_panic(const u8 reason, const size_t avail, const size_t size)
{
- __fortify_report(reason);
+ __fortify_report(reason, avail, size);
BUG();
}
EXPORT_SYMBOL(__fortify_panic);
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v2 01/10] kunit: tool: Enable CONFIG_FORTIFY_SOURCE under UML
2023-04-07 19:27 ` [PATCH v2 01/10] kunit: tool: Enable CONFIG_FORTIFY_SOURCE under UML Kees Cook
@ 2023-04-07 23:33 ` Nick Desaulniers
2023-04-07 23:42 ` Nick Desaulniers
2023-05-07 15:20 ` Kees Cook
0 siblings, 2 replies; 27+ messages in thread
From: Nick Desaulniers @ 2023-04-07 23:33 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Liam Howlett, Vlastimil Babka, Dan Williams,
Rasmus Villemoes, Yury Norov, Jason A. Donenfeld,
Sander Vanheule, Eric Biggers, Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
On Fri, Apr 7, 2023 at 12:27 PM Kees Cook <keescook@chromium.org> wrote:
>
> Since commit ba38961a069b ("um: Enable FORTIFY_SOURCE"), it's possible
> to run the FORTIFY tests under UML. Enable CONFIG_FORTIFY_SOURCE when
> running with --altests to gain additional coverage, and by default under
two L's in alltest?
> UML.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> tools/testing/kunit/configs/all_tests.config | 2 ++
> tools/testing/kunit/configs/arch_uml.config | 3 +++
> 2 files changed, 5 insertions(+)
>
> diff --git a/tools/testing/kunit/configs/all_tests.config b/tools/testing/kunit/configs/all_tests.config
> index f990cbb73250..0393940c706a 100644
> --- a/tools/testing/kunit/configs/all_tests.config
> +++ b/tools/testing/kunit/configs/all_tests.config
> @@ -9,6 +9,8 @@ CONFIG_KUNIT=y
> CONFIG_KUNIT_EXAMPLE_TEST=y
> CONFIG_KUNIT_ALL_TESTS=y
>
> +CONFIG_FORTIFY_SOURCE=y
> +
> CONFIG_IIO=y
>
> CONFIG_EXT4_FS=y
> diff --git a/tools/testing/kunit/configs/arch_uml.config b/tools/testing/kunit/configs/arch_uml.config
> index e824ce43b05a..54ad8972681a 100644
> --- a/tools/testing/kunit/configs/arch_uml.config
> +++ b/tools/testing/kunit/configs/arch_uml.config
> @@ -3,3 +3,6 @@
> # Enable virtio/pci, as a lot of tests require it.
> CONFIG_VIRTIO_UML=y
> CONFIG_UML_PCI_OVER_VIRTIO=y
> +
> +# Enable FORTIFY_SOURCE for wider checking.
> +CONFIG_FORTIFY_SOURCE=y
> --
> 2.34.1
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 01/10] kunit: tool: Enable CONFIG_FORTIFY_SOURCE under UML
2023-04-07 23:33 ` Nick Desaulniers
@ 2023-04-07 23:42 ` Nick Desaulniers
2023-05-10 19:24 ` Kees Cook
2023-05-07 15:20 ` Kees Cook
1 sibling, 1 reply; 27+ messages in thread
From: Nick Desaulniers @ 2023-04-07 23:42 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Liam Howlett, Vlastimil Babka, Dan Williams,
Rasmus Villemoes, Yury Norov, Jason A. Donenfeld,
Sander Vanheule, Eric Biggers, Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
On Fri, Apr 7, 2023 at 4:33 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Fri, Apr 7, 2023 at 12:27 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > Since commit ba38961a069b ("um: Enable FORTIFY_SOURCE"), it's possible
> > to run the FORTIFY tests under UML. Enable CONFIG_FORTIFY_SOURCE when
> > running with --altests to gain additional coverage, and by default under
>
> two L's in alltest?
Also, while testing this series:
```
$ LLVM=1 ./tools/testing/kunit/kunit.py run
...
[16:40:09] ================== fortify (24 subtests) ===================
[16:40:09] [PASSED] known_sizes_test
[16:40:09] [PASSED] control_flow_split_test
[16:40:09] [PASSED] alloc_size_kmalloc_const_test
[16:40:09] # alloc_size_kmalloc_dynamic_test: EXPECTATION FAILED
at lib/fortify_kunit.c:249
[16:40:09] Expected __builtin_dynamic_object_size(p, 1) == expected, but
[16:40:09] __builtin_dynamic_object_size(p, 1) == -1
(0xffffffffffffffff)
[16:40:09] expected == 11 (0xb)
[16:40:09] __alloc_size() not working with __bdos on kmemdup("hello
there", len, gfp)
[16:40:09] [FAILED] alloc_size_kmalloc_dynamic_test
[16:40:09] [PASSED] alloc_size_vmalloc_const_test
[16:40:09] [PASSED] alloc_size_vmalloc_dynamic_test
[16:40:09] [PASSED] alloc_size_kvmalloc_const_test
[16:40:09] [PASSED] alloc_size_kvmalloc_dynamic_test
[16:40:09] [PASSED] alloc_size_devm_kmalloc_const_test
[16:40:09] [PASSED] alloc_size_devm_kmalloc_dynamic_test
[16:40:09] [PASSED] strlen_test
[16:40:09] [PASSED] strnlen_test
[16:40:09] [PASSED] strcpy_test
[16:40:09] [PASSED] strncpy_test
[16:40:09] [PASSED] strlcpy_test
[16:40:09] [PASSED] strscpy_test
[16:40:09] [PASSED] strcat_test
[16:40:09] [PASSED] strncat_test
[16:40:09] [PASSED] strlcat_test
[16:40:09] [PASSED] memscan_test
[16:40:09] [PASSED] memchr_test
[16:40:09] [PASSED] memchr_inv_test
[16:40:09] [PASSED] memcmp_test
[16:40:09] [PASSED] kmemdup_test
[16:40:09] # fortify: pass:23 fail:1 skip:0 total:24
[16:40:09] # Totals: pass:23 fail:1 skip:0 total:24
[16:40:09] ===================== [FAILED] fortify =====================
```
It would be cool to understand that failure in BDOS BEFORE turning on
these tests by default.
>
> > UML.
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > tools/testing/kunit/configs/all_tests.config | 2 ++
> > tools/testing/kunit/configs/arch_uml.config | 3 +++
> > 2 files changed, 5 insertions(+)
> >
> > diff --git a/tools/testing/kunit/configs/all_tests.config b/tools/testing/kunit/configs/all_tests.config
> > index f990cbb73250..0393940c706a 100644
> > --- a/tools/testing/kunit/configs/all_tests.config
> > +++ b/tools/testing/kunit/configs/all_tests.config
> > @@ -9,6 +9,8 @@ CONFIG_KUNIT=y
> > CONFIG_KUNIT_EXAMPLE_TEST=y
> > CONFIG_KUNIT_ALL_TESTS=y
> >
> > +CONFIG_FORTIFY_SOURCE=y
> > +
> > CONFIG_IIO=y
> >
> > CONFIG_EXT4_FS=y
> > diff --git a/tools/testing/kunit/configs/arch_uml.config b/tools/testing/kunit/configs/arch_uml.config
> > index e824ce43b05a..54ad8972681a 100644
> > --- a/tools/testing/kunit/configs/arch_uml.config
> > +++ b/tools/testing/kunit/configs/arch_uml.config
> > @@ -3,3 +3,6 @@
> > # Enable virtio/pci, as a lot of tests require it.
> > CONFIG_VIRTIO_UML=y
> > CONFIG_UML_PCI_OVER_VIRTIO=y
> > +
> > +# Enable FORTIFY_SOURCE for wider checking.
> > +CONFIG_FORTIFY_SOURCE=y
> > --
> > 2.34.1
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 09/10] fortify: Add KUnit tests for runtime overflows
2023-04-07 19:27 ` [PATCH v2 09/10] fortify: Add KUnit tests for runtime overflows Kees Cook
@ 2023-04-08 0:33 ` kernel test robot
2023-04-18 18:27 ` Nick Desaulniers
0 siblings, 1 reply; 27+ messages in thread
From: kernel test robot @ 2023-04-08 0:33 UTC (permalink / raw)
To: Kees Cook, linux-hardening
Cc: oe-kbuild-all, Kees Cook, Andy Shevchenko, Cezary Rojewski,
Puyou Lu, Mark Brown, Josh Poimboeuf, Peter Zijlstra,
Brendan Higgins, David Gow, Andrew Morton,
Linux Memory Management List, Nathan Chancellor,
Alexander Potapenko, Zhaoyang Huang, Randy Dunlap,
Geert Uytterhoeven, Miguel Ojeda, Alexander Lobakin,
Nick Desaulniers, Liam Howlett, Vlastimil Babka, Dan Williams,
Rasmus Villemoes, Yury Norov, Jason A. Donenfeld,
Sander Vanheule, Eric Biggers, Masami Hiramatsu (Google),
Andrey Konovalov
Hi Kees,
kernel test robot noticed the following build warnings:
[auto build test WARNING on kees/for-next/hardening]
[also build test WARNING on kees/for-next/pstore kees/for-next/kspp linus/master tip/x86/core v6.3-rc5 next-20230406]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kees-Cook/kunit-tool-Enable-CONFIG_FORTIFY_SOURCE-under-UML/20230408-032959
base: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/hardening
patch link: https://lore.kernel.org/r/20230407192717.636137-9-keescook%40chromium.org
patch subject: [PATCH v2 09/10] fortify: Add KUnit tests for runtime overflows
config: openrisc-randconfig-r034-20230405 (https://download.01.org/0day-ci/archive/20230408/202304080811.nYP4KpPZ-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/d212962ef7682ee160bf38fa455475558f031759
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Kees-Cook/kunit-tool-Enable-CONFIG_FORTIFY_SOURCE-under-UML/20230408-032959
git checkout d212962ef7682ee160bf38fa455475558f031759
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=openrisc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=openrisc SHELL=/bin/bash lib/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304080811.nYP4KpPZ-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from lib/fortify_kunit.c:28:
lib/fortify_kunit.c: In function 'strnlen_test':
>> lib/fortify_kunit.c:412:31: warning: 'strnlen' specified bound 33 exceeds source size 32 [-Wstringop-overread]
412 | KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 1), end);
include/kunit/test.h:584:38: note: in definition of macro 'KUNIT_BASE_BINARY_ASSERTION'
584 | const typeof(left) __left = (left); \
| ^~~~
include/kunit/test.h:776:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
776 | KUNIT_BINARY_INT_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:773:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
773 | KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
| ^~~~~~~~~~~~~~~~~~~
lib/fortify_kunit.c:412:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
412 | KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 1), end);
| ^~~~~~~~~~~~~~~
lib/fortify_kunit.c:359:14: note: source object allocated here
359 | char buf[32];
| ^~~
lib/fortify_kunit.c:414:31: warning: 'strnlen' specified bound 34 exceeds source size 32 [-Wstringop-overread]
414 | KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 2), end);
include/kunit/test.h:584:38: note: in definition of macro 'KUNIT_BASE_BINARY_ASSERTION'
584 | const typeof(left) __left = (left); \
| ^~~~
include/kunit/test.h:776:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
776 | KUNIT_BINARY_INT_ASSERTION(test, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:773:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
773 | KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
| ^~~~~~~~~~~~~~~~~~~
lib/fortify_kunit.c:414:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
414 | KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 2), end);
| ^~~~~~~~~~~~~~~
lib/fortify_kunit.c:359:14: note: source object allocated here
359 | char buf[32];
| ^~~
vim +/strnlen +412 lib/fortify_kunit.c
387
388 static void strnlen_test(struct kunit *test)
389 {
390 struct fortify_padding pad = { };
391 int i, end = sizeof(pad.buf) - 1;
392
393 /* Fill 31 bytes with valid characters. */
394 for (i = 0; i < sizeof(pad.buf) - 1; i++)
395 pad.buf[i] = i + '0';
396 /* Trailing bytes are still %NUL. */
397 KUNIT_EXPECT_EQ(test, pad.buf[end], '\0');
398 KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
399
400 /* String is terminated, so strnlen() is valid. */
401 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, sizeof(pad.buf)), end);
402 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
403 /* A truncated strnlen() will be safe, too. */
404 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, sizeof(pad.buf) / 2),
405 sizeof(pad.buf) / 2);
406 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
407
408 /* Make string unterminated, and recount. */
409 pad.buf[end] = 'A';
410 end = sizeof(pad.buf);
411 /* Reading beyond with strncpy() will fail. */
> 412 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 1), end);
413 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1);
414 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 2), end);
415 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
416
417 /* Early-truncated is safe still, though. */
418 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end), end);
419 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
420
421 end = sizeof(pad.buf) / 2;
422 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end), end);
423 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
424 }
425
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 07/10] fortify: Split reporting and avoid passing string pointer
2023-04-07 19:27 ` [PATCH v2 07/10] fortify: Split reporting and avoid passing string pointer Kees Cook
@ 2023-04-08 2:26 ` kernel test robot
2023-04-20 15:52 ` Alexander Lobakin
1 sibling, 0 replies; 27+ messages in thread
From: kernel test robot @ 2023-04-08 2:26 UTC (permalink / raw)
To: Kees Cook, linux-hardening
Cc: oe-kbuild-all, Kees Cook, Andy Shevchenko, Cezary Rojewski,
Puyou Lu, Mark Brown, Josh Poimboeuf, Brendan Higgins, David Gow,
Andrew Morton, Linux Memory Management List, Nathan Chancellor,
Alexander Potapenko, Zhaoyang Huang, Randy Dunlap,
Geert Uytterhoeven, Miguel Ojeda, Alexander Lobakin,
Nick Desaulniers, Liam Howlett, Vlastimil Babka, Dan Williams,
Rasmus Villemoes, Yury Norov, Jason A. Donenfeld,
Sander Vanheule, Eric Biggers, Masami Hiramatsu (Google),
Andrey Konovalov
Hi Kees,
kernel test robot noticed the following build warnings:
[auto build test WARNING on kees/for-next/hardening]
[also build test WARNING on kees/for-next/pstore kees/for-next/kspp linus/master tip/x86/core v6.3-rc5 next-20230406]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kees-Cook/kunit-tool-Enable-CONFIG_FORTIFY_SOURCE-under-UML/20230408-032959
base: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/hardening
patch link: https://lore.kernel.org/r/20230407192717.636137-7-keescook%40chromium.org
patch subject: [PATCH v2 07/10] fortify: Split reporting and avoid passing string pointer
config: arm-randconfig-r046-20230404 (https://download.01.org/0day-ci/archive/20230408/202304081026.0GjsJGNP-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/124d051177d6812a0b5bd445a9f89e0694d56f2d
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Kees-Cook/kunit-tool-Enable-CONFIG_FORTIFY_SOURCE-under-UML/20230408-032959
git checkout 124d051177d6812a0b5bd445a9f89e0694d56f2d
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304081026.0GjsJGNP-lkp@intel.com/
All warnings (new ones prefixed by >>):
arch/arm/boot/compressed/misc.c:129:17: warning: no previous prototype for '__div0' [-Wmissing-prototypes]
129 | asmlinkage void __div0(void)
| ^~~~~~
arch/arm/boot/compressed/misc.c:138:1: warning: no previous prototype for 'decompress_kernel' [-Wmissing-prototypes]
138 | decompress_kernel(unsigned long output_start, unsigned long free_mem_ptr_p,
| ^~~~~~~~~~~~~~~~~
>> arch/arm/boot/compressed/misc.c:163:6: warning: no previous prototype for '__fortify_panic' [-Wmissing-prototypes]
163 | void __fortify_panic(const u8 reason)
| ^~~~~~~~~~~~~~~
vim +/__fortify_panic +163 arch/arm/boot/compressed/misc.c
128
> 129 asmlinkage void __div0(void)
130 {
131 error("Attempting division by 0!");
132 }
133
134 extern int do_decompress(u8 *input, int len, u8 *output, void (*error)(char *x));
135
136
137 void
138 decompress_kernel(unsigned long output_start, unsigned long free_mem_ptr_p,
139 unsigned long free_mem_ptr_end_p,
140 int arch_id)
141 {
142 int ret;
143
144 output_data = (unsigned char *)output_start;
145 free_mem_ptr = free_mem_ptr_p;
146 free_mem_end_ptr = free_mem_ptr_end_p;
147 __machine_arch_type = arch_id;
148
149 #ifdef CONFIG_ARCH_EP93XX
150 ep93xx_decomp_setup();
151 #endif
152 arch_decomp_setup();
153
154 putstr("Uncompressing Linux...");
155 ret = do_decompress(input_data, input_data_end - input_data,
156 output_data, error);
157 if (ret)
158 error("decompressor returned an error");
159 else
160 putstr(" done, booting the kernel.\n");
161 }
162
> 163 void __fortify_panic(const u8 reason)
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 04/10] fortify: Use const variables for __member_size tracking
2023-04-07 19:27 ` [PATCH v2 04/10] fortify: Use const variables for __member_size tracking Kees Cook
@ 2023-04-18 17:58 ` Nick Desaulniers
0 siblings, 0 replies; 27+ messages in thread
From: Nick Desaulniers @ 2023-04-18 17:58 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, Miguel Ojeda, Andy Shevchenko, Cezary Rojewski,
Puyou Lu, Mark Brown, Josh Poimboeuf, Peter Zijlstra,
Brendan Higgins, David Gow, Andrew Morton, Nathan Chancellor,
Alexander Potapenko, Zhaoyang Huang, Randy Dunlap,
Geert Uytterhoeven, Miguel Ojeda, Alexander Lobakin,
Liam Howlett, Vlastimil Babka, Dan Williams, Rasmus Villemoes,
Yury Norov, Jason A. Donenfeld, Sander Vanheule, Eric Biggers,
Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
On Fri, Apr 7, 2023 at 12:27 PM Kees Cook <keescook@chromium.org> wrote:
>
> The sizes reported by __member_size should never change in a given
> function. Mark them as such.
>
> Suggested-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> include/linux/fortify-string.h | 42 +++++++++++++++++-----------------
> 1 file changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index c9de1f59ee80..fae1bf4bd543 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -20,7 +20,7 @@ void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning("
> ({ \
> char *__p = (char *)(p); \
> size_t __ret = SIZE_MAX; \
> - size_t __p_size = __member_size(p); \
> + const size_t __p_size = __member_size(p); \
> if (__p_size != SIZE_MAX && \
> __builtin_constant_p(*__p)) { \
> size_t __p_len = __p_size - 1; \
> @@ -142,7 +142,7 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size)
> __FORTIFY_INLINE __diagnose_as(__builtin_strncpy, 1, 2, 3)
> char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
> {
> - size_t p_size = __member_size(p);
> + const size_t p_size = __member_size(p);
>
> if (__compiletime_lessthan(p_size, size))
> __write_overflow();
> @@ -169,7 +169,7 @@ char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
> __FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
> char *strcat(char * const POS p, const char *q)
> {
> - size_t p_size = __member_size(p);
> + const size_t p_size = __member_size(p);
>
> if (p_size == SIZE_MAX)
> return __underlying_strcat(p, q);
> @@ -191,8 +191,8 @@ extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(st
> */
> __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size_t maxlen)
> {
> - size_t p_size = __member_size(p);
> - size_t p_len = __compiletime_strlen(p);
> + const size_t p_size = __member_size(p);
> + const size_t p_len = __compiletime_strlen(p);
> size_t ret;
>
> /* We can take compile-time actions when maxlen is const. */
> @@ -233,8 +233,8 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size
> __FORTIFY_INLINE __diagnose_as(__builtin_strlen, 1)
> __kernel_size_t __fortify_strlen(const char * const POS p)
> {
> + const size_t p_size = __member_size(p);
> __kernel_size_t ret;
> - size_t p_size = __member_size(p);
>
> /* Give up if we don't know how large p is. */
> if (p_size == SIZE_MAX)
> @@ -267,8 +267,8 @@ extern size_t __real_strlcpy(char *, const char *, size_t) __RENAME(strlcpy);
> */
> __FORTIFY_INLINE size_t strlcpy(char * const POS p, const char * const POS q, size_t size)
> {
> - size_t p_size = __member_size(p);
> - size_t q_size = __member_size(q);
> + const size_t p_size = __member_size(p);
> + const size_t q_size = __member_size(q);
> size_t q_len; /* Full count of source string length. */
> size_t len; /* Count of characters going into destination. */
>
> @@ -318,10 +318,10 @@ extern ssize_t __real_strscpy(char *, const char *, size_t) __RENAME(strscpy);
> */
> __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, size_t size)
> {
> - size_t len;
> /* Use string size rather than possible enclosing struct size. */
> - size_t p_size = __member_size(p);
> - size_t q_size = __member_size(q);
> + const size_t p_size = __member_size(p);
> + const size_t q_size = __member_size(q);
> + size_t len;
>
> /* If we cannot get size of p and q default to call strscpy. */
> if (p_size == SIZE_MAX && q_size == SIZE_MAX)
> @@ -394,9 +394,9 @@ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, s
> __FORTIFY_INLINE __diagnose_as(__builtin_strncat, 1, 2, 3)
> char *strncat(char * const POS p, const char * const POS q, __kernel_size_t count)
> {
> + const size_t p_size = __member_size(p);
> + const size_t q_size = __member_size(q);
> size_t p_len, copy_len;
> - size_t p_size = __member_size(p);
> - size_t q_size = __member_size(q);
>
> if (p_size == SIZE_MAX && q_size == SIZE_MAX)
> return __underlying_strncat(p, q, count);
> @@ -639,7 +639,7 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
> extern void *__real_memscan(void *, int, __kernel_size_t) __RENAME(memscan);
> __FORTIFY_INLINE void *memscan(void * const POS0 p, int c, __kernel_size_t size)
> {
> - size_t p_size = __struct_size(p);
> + const size_t p_size = __struct_size(p);
>
> if (__compiletime_lessthan(p_size, size))
> __read_overflow();
> @@ -651,8 +651,8 @@ __FORTIFY_INLINE void *memscan(void * const POS0 p, int c, __kernel_size_t size)
> __FORTIFY_INLINE __diagnose_as(__builtin_memcmp, 1, 2, 3)
> int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t size)
> {
> - size_t p_size = __struct_size(p);
> - size_t q_size = __struct_size(q);
> + const size_t p_size = __struct_size(p);
> + const size_t q_size = __struct_size(q);
>
> if (__builtin_constant_p(size)) {
> if (__compiletime_lessthan(p_size, size))
> @@ -668,7 +668,7 @@ int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t
> __FORTIFY_INLINE __diagnose_as(__builtin_memchr, 1, 2, 3)
> void *memchr(const void * const POS0 p, int c, __kernel_size_t size)
> {
> - size_t p_size = __struct_size(p);
> + const size_t p_size = __struct_size(p);
>
> if (__compiletime_lessthan(p_size, size))
> __read_overflow();
> @@ -680,7 +680,7 @@ void *memchr(const void * const POS0 p, int c, __kernel_size_t size)
> void *__real_memchr_inv(const void *s, int c, size_t n) __RENAME(memchr_inv);
> __FORTIFY_INLINE void *memchr_inv(const void * const POS0 p, int c, size_t size)
> {
> - size_t p_size = __struct_size(p);
> + const size_t p_size = __struct_size(p);
>
> if (__compiletime_lessthan(p_size, size))
> __read_overflow();
> @@ -693,7 +693,7 @@ extern void *__real_kmemdup(const void *src, size_t len, gfp_t gfp) __RENAME(kme
> __realloc_size(2);
> __FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp)
> {
> - size_t p_size = __struct_size(p);
> + const size_t p_size = __struct_size(p);
>
> if (__compiletime_lessthan(p_size, size))
> __read_overflow();
> @@ -720,8 +720,8 @@ __FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp
> __FORTIFY_INLINE __diagnose_as(__builtin_strcpy, 1, 2)
> char *strcpy(char * const POS p, const char * const POS q)
> {
> - size_t p_size = __member_size(p);
> - size_t q_size = __member_size(q);
> + const size_t p_size = __member_size(p);
> + const size_t q_size = __member_size(q);
> size_t size;
>
> /* If neither buffer size is known, immediately give up. */
> --
> 2.34.1
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 06/10] fortify: strcat: Move definition to use fortified strlcat()
2023-04-07 19:27 ` [PATCH v2 06/10] fortify: strcat: Move definition to use fortified strlcat() Kees Cook
@ 2023-04-18 18:09 ` Nick Desaulniers
2023-05-16 21:15 ` Kees Cook
0 siblings, 1 reply; 27+ messages in thread
From: Nick Desaulniers @ 2023-04-18 18:09 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Liam Howlett, Vlastimil Babka, Dan Williams,
Rasmus Villemoes, Yury Norov, Jason A. Donenfeld,
Sander Vanheule, Eric Biggers, Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
On Fri, Apr 7, 2023 at 12:27 PM Kees Cook <keescook@chromium.org> wrote:
>
> Move the definition of fortified strcat() to after strlcat() to use it
> for bounds checking.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> include/linux/fortify-string.h | 53 +++++++++++++++++-----------------
> 1 file changed, 26 insertions(+), 27 deletions(-)
>
> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index 8cf17ef81905..ab058d092817 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -151,33 +151,6 @@ char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
> return __underlying_strncpy(p, q, size);
> }
>
> -/**
> - * strcat - Append a string to an existing string
> - *
> - * @p: pointer to NUL-terminated string to append to
> - * @q: pointer to NUL-terminated source string to append from
> - *
> - * Do not use this function. While FORTIFY_SOURCE tries to avoid
> - * read and write overflows, this is only possible when the
> - * destination buffer size is known to the compiler. Prefer
> - * building the string with formatting, via scnprintf() or similar.
> - * At the very least, use strncat().
> - *
> - * Returns @p.
> - *
> - */
> -__FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
> -char *strcat(char * const POS p, const char *q)
> -{
> - const size_t p_size = __member_size(p);
> -
> - if (p_size == SIZE_MAX)
> - return __underlying_strcat(p, q);
> - if (strlcat(p, q, p_size) >= p_size)
> - fortify_panic(__func__);
> - return p;
> -}
> -
> extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(strnlen);
> /**
> * strnlen - Return bounded count of characters in a NUL-terminated string
> @@ -435,6 +408,32 @@ size_t strlcat(char * const POS p, const char * const POS q, size_t avail)
> return wanted;
> }
>
> +/* Defined after fortified strlcat() to reuse it. */
I don't follow; the previous location was already defined in terms of
calls to strlcat. Why is this patch necessary?
Could this be fixed in 5/10
https://lore.kernel.org/linux-hardening/20230407192717.636137-5-keescook@chromium.org/
by just putting strlcat in the expected place in the first place?
> +/**
> + * strcat - Append a string to an existing string
> + *
> + * @p: pointer to NUL-terminated string to append to
> + * @q: pointer to NUL-terminated source string to append from
> + *
> + * Do not use this function. While FORTIFY_SOURCE tries to avoid
> + * read and write overflows, this is only possible when the
> + * destination buffer size is known to the compiler. Prefer
> + * building the string with formatting, via scnprintf() or similar.
> + * At the very least, use strncat().
> + *
> + * Returns @p.
> + *
> + */
> +__FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
> +char *strcat(char * const POS p, const char *q)
> +{
> + const size_t p_size = __member_size(p);
> +
This drops the `p_size == SIZE_MAX` guard. Might it be faster at
runtime to dispatch to __underlying_strcat rather than __real_strlcat
in such cases?
What's the convention for __underlying_ vs __real_ prefixes in
include/linux/fortify-string.h?
> + if (strlcat(p, q, p_size) >= p_size)
> + fortify_panic(__func__);
> + return p;
> +}
> +
> /**
> * strncat - Append a string to an existing string
> *
> --
> 2.34.1
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 08/10] fortify: Provide KUnit counters for failure testing
2023-04-07 19:27 ` [PATCH v2 08/10] fortify: Provide KUnit counters for failure testing Kees Cook
@ 2023-04-18 18:20 ` Nick Desaulniers
0 siblings, 0 replies; 27+ messages in thread
From: Nick Desaulniers @ 2023-04-18 18:20 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Liam Howlett, Vlastimil Babka, Dan Williams,
Rasmus Villemoes, Yury Norov, Jason A. Donenfeld,
Sander Vanheule, Eric Biggers, Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
On Fri, Apr 7, 2023 at 12:27 PM Kees Cook <keescook@chromium.org> wrote:
>
> The standard C string APIs were not designed to have a failure mode;
> they were expected to always succeed without memory safety issues.
> Normally, CONFIG_FORTIFY_SOURCE will use fortify_panic() to stop
> processing, as truncating a read or write may provide an even worse
> system state. However, this creates a problem for testing under things
> like KUnit, which needs a way to survive failures.
>
> When building with CONFIG_KUNIT, provide a failure path for all users
> for fortify_panic, and track whether the failure was a read overflow or
> a write overflow, for KUnit tests to examine. Inspired by similar logic
> in the slab tests.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> include/linux/fortify-string.h | 45 +++++++++++++++++++---------------
> lib/fortify_kunit.c | 44 +++++++++++++++++++++++++++++++++
> lib/string_helpers.c | 2 ++
> 3 files changed, 71 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index 19906b45fb98..5d04c0e95854 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -15,8 +15,12 @@
> #define FORTIFY_REASON(func, write) (FIELD_PREP(BIT(0), write) | \
> FIELD_PREP(GENMASK(7, 1), func))
>
> -#define fortify_panic(func, write) \
> +#ifdef FORTIFY_KUNIT_OVERRIDE
> +# define fortify_panic kunit_fortify_panic
> +#else
> +# define fortify_panic(func, write, retfail) \
> __fortify_panic(FORTIFY_REASON(func, write))
> +#endif
Could we provide a different definition of fortify_panic in
lib/string_helpers.c rather than this macro indirection?
>
> #define FORTIFY_READ 0
> #define FORTIFY_WRITE 1
> @@ -186,7 +190,7 @@ char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
> if (__compiletime_lessthan(p_size, size))
> __write_overflow();
> if (p_size < size)
> - fortify_panic(FORTIFY_FUNC_strncpy, FORTIFY_WRITE);
> + fortify_panic(FORTIFY_FUNC_strncpy, FORTIFY_WRITE, p);
> return __underlying_strncpy(p, q, size);
> }
>
> @@ -217,7 +221,7 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size
> /* Do not check characters beyond the end of p. */
> ret = __real_strnlen(p, maxlen < p_size ? maxlen : p_size);
> if (p_size <= ret && maxlen != ret)
> - fortify_panic(FORTIFY_FUNC_strnlen, FORTIFY_READ);
> + fortify_panic(FORTIFY_FUNC_strnlen, FORTIFY_READ, ret);
> return ret;
> }
>
> @@ -253,7 +257,7 @@ __kernel_size_t __fortify_strlen(const char * const POS p)
> return __underlying_strlen(p);
> ret = strnlen(p, p_size);
> if (p_size <= ret)
> - fortify_panic(FORTIFY_FUNC_strlen, FORTIFY_READ);
> + fortify_panic(FORTIFY_FUNC_strlen, FORTIFY_READ, ret);
> return ret;
> }
>
> @@ -295,7 +299,7 @@ __FORTIFY_INLINE size_t strlcpy(char * const POS p, const char * const POS q, si
> }
> if (size) {
> if (len >= p_size)
> - fortify_panic(FORTIFY_FUNC_strlcpy, FORTIFY_WRITE);
> + fortify_panic(FORTIFY_FUNC_strlcpy, FORTIFY_WRITE, q_len);
> __underlying_memcpy(p, q, len);
> p[len] = '\0';
> }
> @@ -373,7 +377,7 @@ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, s
> * p_size.
> */
> if (len > p_size)
> - fortify_panic(FORTIFY_FUNC_strscpy, FORTIFY_WRITE);
> + fortify_panic(FORTIFY_FUNC_strscpy, FORTIFY_WRITE, -E2BIG);
>
> /*
> * We can now safely call vanilla strscpy because we are protected from:
> @@ -431,7 +435,7 @@ size_t strlcat(char * const POS p, const char * const POS q, size_t avail)
>
> /* Give up if string is already overflowed. */
> if (p_size <= p_len)
> - fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_READ);
> + fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_READ, wanted);
>
> if (actual >= avail) {
> copy_len = avail - p_len - 1;
> @@ -440,7 +444,7 @@ size_t strlcat(char * const POS p, const char * const POS q, size_t avail)
>
> /* Give up if copy will overflow. */
> if (p_size <= actual)
> - fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_WRITE);
> + fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_WRITE, wanted);
> __underlying_memcpy(p + p_len, q, copy_len);
> p[actual] = '\0';
>
> @@ -469,7 +473,7 @@ char *strcat(char * const POS p, const char *q)
> const size_t p_size = __member_size(p);
>
> if (strlcat(p, q, p_size) >= p_size)
> - fortify_panic(FORTIFY_FUNC_strcat, FORTIFY_WRITE);
> + fortify_panic(FORTIFY_FUNC_strcat, FORTIFY_WRITE, p);
> return p;
> }
>
> @@ -505,13 +509,13 @@ char *strncat(char * const POS p, const char * const POS q, __kernel_size_t coun
> p_len = strlen(p);
> copy_len = strnlen(q, count);
> if (p_size < p_len + copy_len + 1)
> - fortify_panic(FORTIFY_FUNC_strncat, FORTIFY_WRITE);
> + fortify_panic(FORTIFY_FUNC_strncat, FORTIFY_WRITE, p);
> __underlying_memcpy(p + p_len, q, copy_len);
> p[p_len + copy_len] = '\0';
> return p;
> }
>
> -__FORTIFY_INLINE void fortify_memset_chk(__kernel_size_t size,
> +__FORTIFY_INLINE bool fortify_memset_chk(__kernel_size_t size,
> const size_t p_size,
> const size_t p_size_field)
> {
> @@ -546,7 +550,8 @@ __FORTIFY_INLINE void fortify_memset_chk(__kernel_size_t size,
> * lengths are unknown.)
> */
> if (p_size != SIZE_MAX && p_size < size)
> - fortify_panic(FORTIFY_FUNC_memset, FORTIFY_WRITE);
> + fortify_panic(FORTIFY_FUNC_memset, FORTIFY_WRITE, true);
> + return false;
> }
>
> #define __fortify_memset_chk(p, c, size, p_size, p_size_field) ({ \
> @@ -645,9 +650,9 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
> * lengths are unknown.)
> */
> if (p_size != SIZE_MAX && p_size < size)
> - fortify_panic(func, FORTIFY_WRITE);
> + fortify_panic(func, FORTIFY_WRITE, true);
> else if (q_size != SIZE_MAX && q_size < size)
> - fortify_panic(func, FORTIFY_READ);
> + fortify_panic(func, FORTIFY_READ, true);
>
> /*
> * Warn when writing beyond destination field size.
> @@ -747,7 +752,7 @@ __FORTIFY_INLINE void *memscan(void * const POS0 p, int c, __kernel_size_t size)
> if (__compiletime_lessthan(p_size, size))
> __read_overflow();
> if (p_size < size)
> - fortify_panic(FORTIFY_FUNC_memscan, FORTIFY_READ);
> + fortify_panic(FORTIFY_FUNC_memscan, FORTIFY_READ, NULL);
> return __real_memscan(p, c, size);
> }
>
> @@ -764,7 +769,7 @@ int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t
> __read_overflow2();
> }
> if (p_size < size || q_size < size)
> - fortify_panic(FORTIFY_FUNC_memcmp, FORTIFY_READ);
> + fortify_panic(FORTIFY_FUNC_memcmp, FORTIFY_READ, INT_MIN);
> return __underlying_memcmp(p, q, size);
> }
>
> @@ -776,7 +781,7 @@ void *memchr(const void * const POS0 p, int c, __kernel_size_t size)
> if (__compiletime_lessthan(p_size, size))
> __read_overflow();
> if (p_size < size)
> - fortify_panic(FORTIFY_FUNC_memchr, FORTIFY_READ);
> + fortify_panic(FORTIFY_FUNC_memchr, FORTIFY_READ, NULL);
> return __underlying_memchr(p, c, size);
> }
>
> @@ -788,7 +793,7 @@ __FORTIFY_INLINE void *memchr_inv(const void * const POS0 p, int c, size_t size)
> if (__compiletime_lessthan(p_size, size))
> __read_overflow();
> if (p_size < size)
> - fortify_panic(FORTIFY_FUNC_memchr_inv, FORTIFY_READ);
> + fortify_panic(FORTIFY_FUNC_memchr_inv, FORTIFY_READ, NULL);
> return __real_memchr_inv(p, c, size);
> }
>
> @@ -801,7 +806,7 @@ __FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp
> if (__compiletime_lessthan(p_size, size))
> __read_overflow();
> if (p_size < size)
> - fortify_panic(FORTIFY_FUNC_kmemdup, FORTIFY_READ);
> + fortify_panic(FORTIFY_FUNC_kmemdup, FORTIFY_READ, NULL);
> return __real_kmemdup(p, size, gfp);
> }
>
> @@ -838,7 +843,7 @@ char *strcpy(char * const POS p, const char * const POS q)
> __write_overflow();
> /* Run-time check for dynamic size overflow. */
> if (p_size < size)
> - fortify_panic(FORTIFY_FUNC_strcpy, FORTIFY_WRITE);
> + fortify_panic(FORTIFY_FUNC_strcpy, FORTIFY_WRITE, p);
> __underlying_memcpy(p, q, size);
> return p;
> }
> diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c
> index 524132f33cf0..ea2b39f279c2 100644
> --- a/lib/fortify_kunit.c
> +++ b/lib/fortify_kunit.c
> @@ -15,12 +15,28 @@
> */
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +/* Call kunit_fortify_panic() instead of fortify_panic() */
> +#define FORTIFY_KUNIT_OVERRIDE
> +void fortify_add_kunit_error(int write);
> +#define kunit_fortify_panic(func, write, retfail) \
> + do { \
> + __fortify_report(FORTIFY_REASON(func, write)); \
> + fortify_add_kunit_error(write); \
> + return (retfail); \
^ Does this return value even matter? Could we just return -1 or some
other constant for all cases? Then you don't need an optional
parameter addition to fortify_panic.
> + } while (0)
> +
> #include <kunit/test.h>
> +#include <kunit/test-bug.h>
> #include <linux/device.h>
> #include <linux/slab.h>
> #include <linux/string.h>
> #include <linux/vmalloc.h>
>
> +static struct kunit_resource read_resource;
> +static struct kunit_resource write_resource;
> +static int fortify_read_overflows;
> +static int fortify_write_overflows;
> +
> static const char array_of_10[] = "this is 10";
> static const char *ptr_of_11 = "this is 11!";
> static char array_unknown[] = "compiler thinks I might change";
> @@ -30,6 +46,25 @@ static char array_unknown[] = "compiler thinks I might change";
> # define __compiletime_strlen __builtin_strlen
> #endif
>
> +void fortify_add_kunit_error(int write)
> +{
> + struct kunit_resource *resource;
> + struct kunit *current_test;
> +
> + current_test = kunit_get_current_test();
> + if (!current_test)
> + return;
> +
> + resource = kunit_find_named_resource(current_test,
> + write ? "fortify_write_overflows"
> + : "fortify_read_overflows");
> + if (!resource)
> + return;
> +
> + (*(int *)resource->data)++;
> + kunit_put_resource(resource);
> +}
> +
> static void known_sizes_test(struct kunit *test)
> {
> KUNIT_EXPECT_EQ(test, __compiletime_strlen("88888888"), 8);
> @@ -317,6 +352,15 @@ static int fortify_test_init(struct kunit *test)
> if (!IS_ENABLED(CONFIG_FORTIFY_SOURCE))
> kunit_skip(test, "Not built with CONFIG_FORTIFY_SOURCE=y");
>
> + fortify_read_overflows = 0;
> + kunit_add_named_resource(test, NULL, NULL, &read_resource,
> + "fortify_read_overflows",
> + &fortify_read_overflows);
> + fortify_write_overflows = 0;
> + kunit_add_named_resource(test, NULL, NULL, &write_resource,
> + "fortify_write_overflows",
> + &fortify_write_overflows);
> +
> return 0;
> }
>
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index 96d502e1e578..38edde20e61b 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -18,6 +18,8 @@
> #include <linux/slab.h>
> #include <linux/string.h>
> #include <linux/string_helpers.h>
> +#include <kunit/test.h>
> +#include <kunit/test-bug.h>
Why do we need these headers here?
>
> /**
> * string_get_size - get the size in the specified units
> --
> 2.34.1
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 09/10] fortify: Add KUnit tests for runtime overflows
2023-04-08 0:33 ` kernel test robot
@ 2023-04-18 18:27 ` Nick Desaulniers
0 siblings, 0 replies; 27+ messages in thread
From: Nick Desaulniers @ 2023-04-18 18:27 UTC (permalink / raw)
To: kernel test robot
Cc: Kees Cook, linux-hardening, oe-kbuild-all, Andy Shevchenko,
Cezary Rojewski, Puyou Lu, Mark Brown, Josh Poimboeuf,
Peter Zijlstra, Brendan Higgins, David Gow, Andrew Morton,
Linux Memory Management List, Nathan Chancellor,
Alexander Potapenko, Zhaoyang Huang, Randy Dunlap,
Geert Uytterhoeven, Miguel Ojeda, Alexander Lobakin,
Liam Howlett, Vlastimil Babka, Dan Williams, Rasmus Villemoes,
Yury Norov, Jason A. Donenfeld, Sander Vanheule, Eric Biggers,
Masami Hiramatsu (Google),
Andrey Konovalov
On Fri, Apr 7, 2023 at 5:33 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Kees,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on kees/for-next/hardening]
> [also build test WARNING on kees/for-next/pstore kees/for-next/kspp linus/master tip/x86/core v6.3-rc5 next-20230406]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Kees-Cook/kunit-tool-Enable-CONFIG_FORTIFY_SOURCE-under-UML/20230408-032959
> base: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/hardening
> patch link: https://lore.kernel.org/r/20230407192717.636137-9-keescook%40chromium.org
> patch subject: [PATCH v2 09/10] fortify: Add KUnit tests for runtime overflows
> config: openrisc-randconfig-r034-20230405 (https://download.01.org/0day-ci/archive/20230408/202304080811.nYP4KpPZ-lkp@intel.com/config)
> compiler: or1k-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/d212962ef7682ee160bf38fa455475558f031759
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Kees-Cook/kunit-tool-Enable-CONFIG_FORTIFY_SOURCE-under-UML/20230408-032959
> git checkout d212962ef7682ee160bf38fa455475558f031759
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=openrisc olddefconfig
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=openrisc SHELL=/bin/bash lib/
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202304080811.nYP4KpPZ-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
> In file included from lib/fortify_kunit.c:28:
> lib/fortify_kunit.c: In function 'strnlen_test':
> >> lib/fortify_kunit.c:412:31: warning: 'strnlen' specified bound 33 exceeds source size 32 [-Wstringop-overread]
If we expect to validate the runtime behavior of fortify, but using
constants that the compiler can check for readability in this test,
then we might need to use the
_Pragma/__diag infrastructure from include/linux/compiler_types.h to
disable -Wstringop-overread; or disable it at the makefile level.
> 412 | KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 1), end);
> include/kunit/test.h:584:38: note: in definition of macro 'KUNIT_BASE_BINARY_ASSERTION'
> 584 | const typeof(left) __left = (left); \
> | ^~~~
> include/kunit/test.h:776:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
> 776 | KUNIT_BINARY_INT_ASSERTION(test, \
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~
> include/kunit/test.h:773:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
> 773 | KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
> | ^~~~~~~~~~~~~~~~~~~
> lib/fortify_kunit.c:412:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
> 412 | KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 1), end);
> | ^~~~~~~~~~~~~~~
> lib/fortify_kunit.c:359:14: note: source object allocated here
> 359 | char buf[32];
> | ^~~
> lib/fortify_kunit.c:414:31: warning: 'strnlen' specified bound 34 exceeds source size 32 [-Wstringop-overread]
> 414 | KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 2), end);
> include/kunit/test.h:584:38: note: in definition of macro 'KUNIT_BASE_BINARY_ASSERTION'
> 584 | const typeof(left) __left = (left); \
> | ^~~~
> include/kunit/test.h:776:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
> 776 | KUNIT_BINARY_INT_ASSERTION(test, \
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~
> include/kunit/test.h:773:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
> 773 | KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
> | ^~~~~~~~~~~~~~~~~~~
> lib/fortify_kunit.c:414:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
> 414 | KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 2), end);
> | ^~~~~~~~~~~~~~~
> lib/fortify_kunit.c:359:14: note: source object allocated here
> 359 | char buf[32];
> | ^~~
>
>
> vim +/strnlen +412 lib/fortify_kunit.c
>
> 387
> 388 static void strnlen_test(struct kunit *test)
> 389 {
> 390 struct fortify_padding pad = { };
> 391 int i, end = sizeof(pad.buf) - 1;
> 392
> 393 /* Fill 31 bytes with valid characters. */
> 394 for (i = 0; i < sizeof(pad.buf) - 1; i++)
> 395 pad.buf[i] = i + '0';
> 396 /* Trailing bytes are still %NUL. */
> 397 KUNIT_EXPECT_EQ(test, pad.buf[end], '\0');
> 398 KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
> 399
> 400 /* String is terminated, so strnlen() is valid. */
> 401 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, sizeof(pad.buf)), end);
> 402 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
> 403 /* A truncated strnlen() will be safe, too. */
> 404 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, sizeof(pad.buf) / 2),
> 405 sizeof(pad.buf) / 2);
> 406 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
> 407
> 408 /* Make string unterminated, and recount. */
> 409 pad.buf[end] = 'A';
> 410 end = sizeof(pad.buf);
> 411 /* Reading beyond with strncpy() will fail. */
> > 412 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 1), end);
> 413 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1);
> 414 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 2), end);
> 415 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
> 416
> 417 /* Early-truncated is safe still, though. */
> 418 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end), end);
> 419 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
> 420
> 421 end = sizeof(pad.buf) / 2;
> 422 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end), end);
> 423 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
> 424 }
> 425
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 07/10] fortify: Split reporting and avoid passing string pointer
2023-04-07 19:27 ` [PATCH v2 07/10] fortify: Split reporting and avoid passing string pointer Kees Cook
2023-04-08 2:26 ` kernel test robot
@ 2023-04-20 15:52 ` Alexander Lobakin
1 sibling, 0 replies; 27+ messages in thread
From: Alexander Lobakin @ 2023-04-20 15:52 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Nick Desaulniers, Liam Howlett, Vlastimil Babka, Dan Williams,
Rasmus Villemoes, Yury Norov, Jason A. Donenfeld,
Sander Vanheule, Eric Biggers, Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
From: Kees Cook <keescook@chromium.org>
Date: Fri, 7 Apr 2023 12:27:13 -0700
> In preparation for KUnit testing and further improvements in fortify
> failure reporting, split out the report and encode the function and
> access failure (read or write overflow) into a single u8 argument. This
> mainly ends up saving some space in the data segment. For a defconfig
> with FORTIFY_SOURCE enabled:
[...]
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index 230020a2e076..96d502e1e578 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -1021,10 +1021,27 @@ EXPORT_SYMBOL(__read_overflow2_field);
> void __write_overflow_field(size_t avail, size_t wanted) { }
> EXPORT_SYMBOL(__write_overflow_field);
>
> -void fortify_panic(const char *name)
> +#define MAKE_FORTIFY_FUNC_NAME(func) [MAKE_FORTIFY_FUNC(func)] = #func
> +
> +static const char * const fortify_func_name[] = {
> + EACH_FORTIFY_FUNC(MAKE_FORTIFY_FUNC_NAME)
> +};
> +
> +void __fortify_report(const u8 reason)
> +{
> + const u8 func = FORTIFY_REASON_FUNC(reason);
> + const bool write = FORTIFY_REASON_DIR(reason);
Nano-nit: RCT style here :s
> + const char *name;
> +
> + name = fortify_func_name[min_t(u8, func, FORTIFY_FUNC_UNKNOWN)];
> + WARN(1, "%s: detected buffer %s overflow\n", name, str_read_write(!write));
> +}
> +EXPORT_SYMBOL(__fortify_report);
> +
> +void __fortify_panic(const u8 reason)
> {
> - pr_emerg("detected buffer overflow in %s\n", name);
> + __fortify_report(reason);
> BUG();
> }
> -EXPORT_SYMBOL(fortify_panic);
> +EXPORT_SYMBOL(__fortify_panic);
> #endif /* CONFIG_FORTIFY_SOURCE */
[...]
There's also a small build warning issue. Apart from that, I like how it
does look now, thanks!
Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>
Thanks,
Olek
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 01/10] kunit: tool: Enable CONFIG_FORTIFY_SOURCE under UML
2023-04-07 23:33 ` Nick Desaulniers
2023-04-07 23:42 ` Nick Desaulniers
@ 2023-05-07 15:20 ` Kees Cook
1 sibling, 0 replies; 27+ messages in thread
From: Kees Cook @ 2023-05-07 15:20 UTC (permalink / raw)
To: Nick Desaulniers
Cc: linux-hardening, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Liam Howlett, Vlastimil Babka, Dan Williams,
Rasmus Villemoes, Yury Norov, Jason A. Donenfeld,
Sander Vanheule, Eric Biggers, Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
On Fri, Apr 07, 2023 at 04:33:43PM -0700, Nick Desaulniers wrote:
> On Fri, Apr 7, 2023 at 12:27 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > Since commit ba38961a069b ("um: Enable FORTIFY_SOURCE"), it's possible
> > to run the FORTIFY tests under UML. Enable CONFIG_FORTIFY_SOURCE when
> > running with --altests to gain additional coverage, and by default under
>
> two L's in alltest?
Oops. Fixed. :)
--
Kees Cook
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 01/10] kunit: tool: Enable CONFIG_FORTIFY_SOURCE under UML
2023-04-07 23:42 ` Nick Desaulniers
@ 2023-05-10 19:24 ` Kees Cook
2023-05-22 19:43 ` Nick Desaulniers
0 siblings, 1 reply; 27+ messages in thread
From: Kees Cook @ 2023-05-10 19:24 UTC (permalink / raw)
To: Nick Desaulniers
Cc: linux-hardening, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Liam Howlett, Vlastimil Babka, Dan Williams,
Rasmus Villemoes, Yury Norov, Jason A. Donenfeld,
Sander Vanheule, Eric Biggers, Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
On Fri, Apr 07, 2023 at 04:42:27PM -0700, Nick Desaulniers wrote:
> On Fri, Apr 7, 2023 at 4:33 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > On Fri, Apr 7, 2023 at 12:27 PM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > Since commit ba38961a069b ("um: Enable FORTIFY_SOURCE"), it's possible
> > > to run the FORTIFY tests under UML. Enable CONFIG_FORTIFY_SOURCE when
> > > running with --altests to gain additional coverage, and by default under
> >
> > two L's in alltest?
>
> Also, while testing this series:
> ```
> $ LLVM=1 ./tools/testing/kunit/kunit.py run
> ...
> [16:40:09] ================== fortify (24 subtests) ===================
> [16:40:09] [PASSED] known_sizes_test
> [16:40:09] [PASSED] control_flow_split_test
> [16:40:09] [PASSED] alloc_size_kmalloc_const_test
> [16:40:09] # alloc_size_kmalloc_dynamic_test: EXPECTATION FAILED
> at lib/fortify_kunit.c:249
> [16:40:09] Expected __builtin_dynamic_object_size(p, 1) == expected, but
> [16:40:09] __builtin_dynamic_object_size(p, 1) == -1
> (0xffffffffffffffff)
> [16:40:09] expected == 11 (0xb)
> [16:40:09] __alloc_size() not working with __bdos on kmemdup("hello
> there", len, gfp)
I'm still tracking this down. I'm not sure what's happening here, but it
seems to be Clang-specific, and due to some interaction with the changes
I made for Kunit examination. WHY it happens I haven't found yet.
--
Kees Cook
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 06/10] fortify: strcat: Move definition to use fortified strlcat()
2023-04-18 18:09 ` Nick Desaulniers
@ 2023-05-16 21:15 ` Kees Cook
0 siblings, 0 replies; 27+ messages in thread
From: Kees Cook @ 2023-05-16 21:15 UTC (permalink / raw)
To: Nick Desaulniers
Cc: linux-hardening, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Liam Howlett, Vlastimil Babka, Dan Williams,
Rasmus Villemoes, Yury Norov, Jason A. Donenfeld,
Sander Vanheule, Eric Biggers, Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
On Tue, Apr 18, 2023 at 11:09:41AM -0700, Nick Desaulniers wrote:
> On Fri, Apr 7, 2023 at 12:27 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > Move the definition of fortified strcat() to after strlcat() to use it
> > for bounds checking.
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > include/linux/fortify-string.h | 53 +++++++++++++++++-----------------
> > 1 file changed, 26 insertions(+), 27 deletions(-)
> >
> > diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> > index 8cf17ef81905..ab058d092817 100644
> > --- a/include/linux/fortify-string.h
> > +++ b/include/linux/fortify-string.h
> > @@ -151,33 +151,6 @@ char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
> > return __underlying_strncpy(p, q, size);
> > }
> >
> > -/**
> > - * strcat - Append a string to an existing string
> > - *
> > - * @p: pointer to NUL-terminated string to append to
> > - * @q: pointer to NUL-terminated source string to append from
> > - *
> > - * Do not use this function. While FORTIFY_SOURCE tries to avoid
> > - * read and write overflows, this is only possible when the
> > - * destination buffer size is known to the compiler. Prefer
> > - * building the string with formatting, via scnprintf() or similar.
> > - * At the very least, use strncat().
> > - *
> > - * Returns @p.
> > - *
> > - */
> > -__FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
> > -char *strcat(char * const POS p, const char *q)
> > -{
> > - const size_t p_size = __member_size(p);
> > -
> > - if (p_size == SIZE_MAX)
> > - return __underlying_strcat(p, q);
> > - if (strlcat(p, q, p_size) >= p_size)
> > - fortify_panic(__func__);
> > - return p;
> > -}
> > -
> > extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(strnlen);
> > /**
> > * strnlen - Return bounded count of characters in a NUL-terminated string
> > @@ -435,6 +408,32 @@ size_t strlcat(char * const POS p, const char * const POS q, size_t avail)
> > return wanted;
> > }
> >
> > +/* Defined after fortified strlcat() to reuse it. */
>
> I don't follow; the previous location was already defined in terms of
> calls to strlcat. Why is this patch necessary?
>
> Could this be fixed in 5/10
> https://lore.kernel.org/linux-hardening/20230407192717.636137-5-keescook@chromium.org/
> by just putting strlcat in the expected place in the first place?
I wanted to collect all the str*cat functions together.
> > +/**
> > + * strcat - Append a string to an existing string
> > + *
> > + * @p: pointer to NUL-terminated string to append to
> > + * @q: pointer to NUL-terminated source string to append from
> > + *
> > + * Do not use this function. While FORTIFY_SOURCE tries to avoid
> > + * read and write overflows, this is only possible when the
> > + * destination buffer size is known to the compiler. Prefer
> > + * building the string with formatting, via scnprintf() or similar.
> > + * At the very least, use strncat().
> > + *
> > + * Returns @p.
> > + *
> > + */
> > +__FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
> > +char *strcat(char * const POS p, const char *q)
> > +{
> > + const size_t p_size = __member_size(p);
> > +
>
> This drops the `p_size == SIZE_MAX` guard. Might it be faster at
> runtime to dispatch to __underlying_strcat rather than __real_strlcat
> in such cases?
I wanted to avoid repeating the same checks, so since strlcat() already
does the right checking, I avoided repeating it here.
> What's the convention for __underlying_ vs __real_ prefixes in
> include/linux/fortify-string.h?
__underlying may be wrapped by K*SAN before being implemented via
__builtin, where as __real is used for things that aren't wrapped and/or
aren't available with a __builtin (e.g. strscpy).
--
Kees Cook
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 01/10] kunit: tool: Enable CONFIG_FORTIFY_SOURCE under UML
2023-05-10 19:24 ` Kees Cook
@ 2023-05-22 19:43 ` Nick Desaulniers
2023-05-22 20:14 ` Kees Cook
0 siblings, 1 reply; 27+ messages in thread
From: Nick Desaulniers @ 2023-05-22 19:43 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Liam Howlett, Vlastimil Babka, Dan Williams,
Rasmus Villemoes, Yury Norov, Jason A. Donenfeld,
Sander Vanheule, Eric Biggers, Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
On Wed, May 10, 2023 at 12:24 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Apr 07, 2023 at 04:42:27PM -0700, Nick Desaulniers wrote:
> > On Fri, Apr 7, 2023 at 4:33 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> > >
> > > On Fri, Apr 7, 2023 at 12:27 PM Kees Cook <keescook@chromium.org> wrote:
> > > >
> > > > Since commit ba38961a069b ("um: Enable FORTIFY_SOURCE"), it's possible
> > > > to run the FORTIFY tests under UML. Enable CONFIG_FORTIFY_SOURCE when
> > > > running with --altests to gain additional coverage, and by default under
> > >
> > > two L's in alltest?
> >
> > Also, while testing this series:
> > ```
> > $ LLVM=1 ./tools/testing/kunit/kunit.py run
> > ...
> > [16:40:09] ================== fortify (24 subtests) ===================
> > [16:40:09] [PASSED] known_sizes_test
> > [16:40:09] [PASSED] control_flow_split_test
> > [16:40:09] [PASSED] alloc_size_kmalloc_const_test
> > [16:40:09] # alloc_size_kmalloc_dynamic_test: EXPECTATION FAILED
> > at lib/fortify_kunit.c:249
> > [16:40:09] Expected __builtin_dynamic_object_size(p, 1) == expected, but
> > [16:40:09] __builtin_dynamic_object_size(p, 1) == -1
> > (0xffffffffffffffff)
> > [16:40:09] expected == 11 (0xb)
> > [16:40:09] __alloc_size() not working with __bdos on kmemdup("hello
> > there", len, gfp)
>
> I'm still tracking this down. I'm not sure what's happening here, but it
> seems to be Clang-specific, and due to some interaction with the changes
> I made for Kunit examination. WHY it happens I haven't found yet.
Was this what exposed https://github.com/llvm/llvm-project/issues/62789?
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 01/10] kunit: tool: Enable CONFIG_FORTIFY_SOURCE under UML
2023-05-22 19:43 ` Nick Desaulniers
@ 2023-05-22 20:14 ` Kees Cook
0 siblings, 0 replies; 27+ messages in thread
From: Kees Cook @ 2023-05-22 20:14 UTC (permalink / raw)
To: Nick Desaulniers
Cc: linux-hardening, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Liam Howlett, Vlastimil Babka, Dan Williams,
Rasmus Villemoes, Yury Norov, Jason A. Donenfeld,
Sander Vanheule, Eric Biggers, Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
On Mon, May 22, 2023 at 12:43:51PM -0700, Nick Desaulniers wrote:
> On Wed, May 10, 2023 at 12:24 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Fri, Apr 07, 2023 at 04:42:27PM -0700, Nick Desaulniers wrote:
> > > On Fri, Apr 7, 2023 at 4:33 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> > > >
> > > > On Fri, Apr 7, 2023 at 12:27 PM Kees Cook <keescook@chromium.org> wrote:
> > > > >
> > > > > Since commit ba38961a069b ("um: Enable FORTIFY_SOURCE"), it's possible
> > > > > to run the FORTIFY tests under UML. Enable CONFIG_FORTIFY_SOURCE when
> > > > > running with --altests to gain additional coverage, and by default under
> > > >
> > > > two L's in alltest?
> > >
> > > Also, while testing this series:
> > > ```
> > > $ LLVM=1 ./tools/testing/kunit/kunit.py run
> > > ...
> > > [16:40:09] ================== fortify (24 subtests) ===================
> > > [16:40:09] [PASSED] known_sizes_test
> > > [16:40:09] [PASSED] control_flow_split_test
> > > [16:40:09] [PASSED] alloc_size_kmalloc_const_test
> > > [16:40:09] # alloc_size_kmalloc_dynamic_test: EXPECTATION FAILED
> > > at lib/fortify_kunit.c:249
> > > [16:40:09] Expected __builtin_dynamic_object_size(p, 1) == expected, but
> > > [16:40:09] __builtin_dynamic_object_size(p, 1) == -1
> > > (0xffffffffffffffff)
> > > [16:40:09] expected == 11 (0xb)
> > > [16:40:09] __alloc_size() not working with __bdos on kmemdup("hello
> > > there", len, gfp)
> >
> > I'm still tracking this down. I'm not sure what's happening here, but it
> > seems to be Clang-specific, and due to some interaction with the changes
> > I made for Kunit examination. WHY it happens I haven't found yet.
>
> Was this what exposed https://github.com/llvm/llvm-project/issues/62789?
Nope -- I found this while working on:
https://lore.kernel.org/lkml/20230517225838.never.965-kees@kernel.org/
i.e. I was surprised I could use static initializers with a flexible
array, and then I went and verified various related behaviors between
GCC and Clang.
--
Kees Cook
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 02/10] fortify: Allow KUnit test to build without FORTIFY
2023-04-07 19:27 ` [PATCH v2 02/10] fortify: Allow KUnit test to build without FORTIFY Kees Cook
@ 2023-07-02 15:07 ` Geert Uytterhoeven
2023-07-03 19:47 ` Kees Cook
0 siblings, 1 reply; 27+ messages in thread
From: Geert Uytterhoeven @ 2023-07-02 15:07 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Nick Desaulniers, Liam Howlett,
Vlastimil Babka, Dan Williams, Rasmus Villemoes, Yury Norov,
Jason A. Donenfeld, Sander Vanheule, Eric Biggers,
Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
Hi Kees,
On Fri, Apr 7, 2023 at 9:27 PM Kees Cook <keescook@chromium.org> wrote:
> In order for CI systems to notice all the skipped tests related to
> CONFIG_FORTIFY_SOURCE, allow the FORTIFY_SOURCE KUnit tests to build
> with or without CONFIG_FORTIFY_SOURCE.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Thanks for your patch, which is now commit a9dc8d0442294b42
("fortify: Allow KUnit test to build without FORTIFY") upstream.
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2614,7 +2614,7 @@ config STACKINIT_KUNIT_TEST
>
> config FORTIFY_KUNIT_TEST
> tristate "Test fortified str*() and mem*() function internals at runtime" if !KUNIT_ALL_TESTS
> - depends on KUNIT && FORTIFY_SOURCE
> + depends on KUNIT
All other tests depend on the functionality they test.
Which makes sense, as you only want to test the functionality that is
available in the kernel you want to run.
> default KUNIT_ALL_TESTS
> help
> Builds unit tests for checking internals of FORTIFY_SOURCE as used
> diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c
> index c8c33cbaae9e..524132f33cf0 100644
> --- a/lib/fortify_kunit.c
> +++ b/lib/fortify_kunit.c
> @@ -307,6 +312,14 @@ DEFINE_ALLOC_SIZE_TEST_PAIR(kvmalloc)
> } while (0)
> DEFINE_ALLOC_SIZE_TEST_PAIR(devm_kmalloc)
>
> +static int fortify_test_init(struct kunit *test)
> +{
> + if (!IS_ENABLED(CONFIG_FORTIFY_SOURCE))
> + kunit_skip(test, "Not built with CONFIG_FORTIFY_SOURCE=y");
I was greeted by this message, which wasn't that helpful, as
CONFIG_FORTIFY_SOURCE depends on CONFIG_ARCH_HAS_FORTIFY_SOURCE,
which is not available yet on all architectures.
So I think the proper thing to do is to revert this patch.
Thanks!
> +
> + return 0;
> +}
> +
> static struct kunit_case fortify_test_cases[] = {
> KUNIT_CASE(known_sizes_test),
> KUNIT_CASE(control_flow_split_test),
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 02/10] fortify: Allow KUnit test to build without FORTIFY
2023-07-02 15:07 ` Geert Uytterhoeven
@ 2023-07-03 19:47 ` Kees Cook
0 siblings, 0 replies; 27+ messages in thread
From: Kees Cook @ 2023-07-03 19:47 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linux-hardening, Andy Shevchenko, Cezary Rojewski, Puyou Lu,
Mark Brown, Josh Poimboeuf, Peter Zijlstra, Brendan Higgins,
David Gow, Andrew Morton, Nathan Chancellor, Alexander Potapenko,
Zhaoyang Huang, Randy Dunlap, Geert Uytterhoeven, Miguel Ojeda,
Alexander Lobakin, Nick Desaulniers, Liam Howlett,
Vlastimil Babka, Dan Williams, Rasmus Villemoes, Yury Norov,
Jason A. Donenfeld, Sander Vanheule, Eric Biggers,
Masami Hiramatsu (Google),
Andrey Konovalov, Linus Walleij, Daniel Latypov,
José Expósito, linux-kernel, kunit-dev
On Sun, Jul 02, 2023 at 05:07:05PM +0200, Geert Uytterhoeven wrote:
> Hi Kees,
>
> On Fri, Apr 7, 2023 at 9:27 PM Kees Cook <keescook@chromium.org> wrote:
> > In order for CI systems to notice all the skipped tests related to
> > CONFIG_FORTIFY_SOURCE, allow the FORTIFY_SOURCE KUnit tests to build
> > with or without CONFIG_FORTIFY_SOURCE.
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
>
> Thanks for your patch, which is now commit a9dc8d0442294b42
> ("fortify: Allow KUnit test to build without FORTIFY") upstream.
>
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -2614,7 +2614,7 @@ config STACKINIT_KUNIT_TEST
> >
> > config FORTIFY_KUNIT_TEST
> > tristate "Test fortified str*() and mem*() function internals at runtime" if !KUNIT_ALL_TESTS
> > - depends on KUNIT && FORTIFY_SOURCE
> > + depends on KUNIT
>
> All other tests depend on the functionality they test.
> Which makes sense, as you only want to test the functionality that is
> available in the kernel you want to run.
Yeah, that is true for KUnit.
>
> > default KUNIT_ALL_TESTS
> > help
> > Builds unit tests for checking internals of FORTIFY_SOURCE as used
> > diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c
> > index c8c33cbaae9e..524132f33cf0 100644
> > --- a/lib/fortify_kunit.c
> > +++ b/lib/fortify_kunit.c
>
> > @@ -307,6 +312,14 @@ DEFINE_ALLOC_SIZE_TEST_PAIR(kvmalloc)
> > } while (0)
> > DEFINE_ALLOC_SIZE_TEST_PAIR(devm_kmalloc)
> >
> > +static int fortify_test_init(struct kunit *test)
> > +{
> > + if (!IS_ENABLED(CONFIG_FORTIFY_SOURCE))
> > + kunit_skip(test, "Not built with CONFIG_FORTIFY_SOURCE=y");
>
> I was greeted by this message, which wasn't that helpful, as
> CONFIG_FORTIFY_SOURCE depends on CONFIG_ARCH_HAS_FORTIFY_SOURCE,
> which is not available yet on all architectures.
>
> So I think the proper thing to do is to revert this patch.
> Thanks!
I created this patch so that I could add CONFIG_FORTIFY_SOURCE support
to UML, but you have a good point about other archs. I'll prepare a
revert.
--
Kees Cook
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2023-07-03 19:47 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-07 19:27 [PATCH v2 00/10] fortify: Add KUnit tests for runtime overflows Kees Cook
2023-04-07 19:27 ` [PATCH v2 01/10] kunit: tool: Enable CONFIG_FORTIFY_SOURCE under UML Kees Cook
2023-04-07 23:33 ` Nick Desaulniers
2023-04-07 23:42 ` Nick Desaulniers
2023-05-10 19:24 ` Kees Cook
2023-05-22 19:43 ` Nick Desaulniers
2023-05-22 20:14 ` Kees Cook
2023-05-07 15:20 ` Kees Cook
2023-04-07 19:27 ` [PATCH v2 02/10] fortify: Allow KUnit test to build without FORTIFY Kees Cook
2023-07-02 15:07 ` Geert Uytterhoeven
2023-07-03 19:47 ` Kees Cook
2023-04-07 19:27 ` [PATCH v2 03/10] string: Add Kunit tests for strcat() family Kees Cook
2023-04-07 19:27 ` [PATCH v2 04/10] fortify: Use const variables for __member_size tracking Kees Cook
2023-04-18 17:58 ` Nick Desaulniers
2023-04-07 19:27 ` [PATCH v2 05/10] fortify: Add protection for strlcat() Kees Cook
2023-04-07 19:27 ` [PATCH v2 06/10] fortify: strcat: Move definition to use fortified strlcat() Kees Cook
2023-04-18 18:09 ` Nick Desaulniers
2023-05-16 21:15 ` Kees Cook
2023-04-07 19:27 ` [PATCH v2 07/10] fortify: Split reporting and avoid passing string pointer Kees Cook
2023-04-08 2:26 ` kernel test robot
2023-04-20 15:52 ` Alexander Lobakin
2023-04-07 19:27 ` [PATCH v2 08/10] fortify: Provide KUnit counters for failure testing Kees Cook
2023-04-18 18:20 ` Nick Desaulniers
2023-04-07 19:27 ` [PATCH v2 09/10] fortify: Add KUnit tests for runtime overflows Kees Cook
2023-04-08 0:33 ` kernel test robot
2023-04-18 18:27 ` Nick Desaulniers
2023-04-07 19:27 ` [PATCH v2 10/10] fortify: Improve buffer overflow reporting Kees Cook
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.