From 66a5c40f60f5d88ad8d47ba6a4ba05892853fa1f Mon Sep 17 00:00:00 2001 From: Tanzir Hasan Date: Tue, 26 Dec 2023 18:00:00 +0000 Subject: kernel.h: removed REPEAT_BYTE from kernel.h This patch creates wordpart.h and includes it in asm/word-at-a-time.h for all architectures. WORD_AT_A_TIME_CONSTANTS depends on kernel.h because of REPEAT_BYTE. Moving this to another header and including it where necessary allows us to not include the bloated kernel.h. Making this implicit dependency on REPEAT_BYTE explicit allows for later improvements in the lib/string.c inclusion list. Suggested-by: Al Viro Suggested-by: Andy Shevchenko Signed-off-by: Tanzir Hasan Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20231226-libstringheader-v6-1-80aa08c7652c@google.com Signed-off-by: Kees Cook --- include/linux/kernel.h | 8 -------- include/linux/wordpart.h | 13 +++++++++++++ 2 files changed, 13 insertions(+), 8 deletions(-) create mode 100644 include/linux/wordpart.h (limited to 'include/linux') diff --git a/include/linux/kernel.h b/include/linux/kernel.h index d9ad21058eed..f4a1d582b79d 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -39,14 +39,6 @@ #define STACK_MAGIC 0xdeadbeef -/** - * REPEAT_BYTE - repeat the value @x multiple times as an unsigned long value - * @x: value to repeat - * - * NOTE: @x is not checked for > 0xff; larger values produce odd results. - */ -#define REPEAT_BYTE(x) ((~0ul / 0xff) * (x)) - /* generic data direction definitions */ #define READ 0 #define WRITE 1 diff --git a/include/linux/wordpart.h b/include/linux/wordpart.h new file mode 100644 index 000000000000..c9e6bd773ebd --- /dev/null +++ b/include/linux/wordpart.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef _LINUX_WORDPART_H +#define _LINUX_WORDPART_H +/** + * REPEAT_BYTE - repeat the value @x multiple times as an unsigned long value + * @x: value to repeat + * + * NOTE: @x is not checked for > 0xff; larger values produce odd results. + */ +#define REPEAT_BYTE(x) ((~0ul / 0xff) * (x)) + +#endif // _LINUX_WORDPART_H -- cgit v1.2.3 From 557f8c582a9ba8abe6aa0fd734b6f342af106b26 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 18 Jan 2024 15:06:05 -0800 Subject: ubsan: Reintroduce signed overflow sanitizer In order to mitigate unexpected signed wrap-around[1], bring back the signed integer overflow sanitizer. It was removed in commit 6aaa31aeb9cf ("ubsan: remove overflow checks") because it was effectively a no-op when combined with -fno-strict-overflow (which correctly changes signed overflow from being "undefined" to being explicitly "wrap around"). Compilers are adjusting their sanitizers to trap wrap-around and to detecting common code patterns that should not be instrumented (e.g. "var + offset < var"). Prepare for this and explicitly rename the option from "OVERFLOW" to "WRAP" to more accurately describe the behavior. To annotate intentional wrap-around arithmetic, the helpers wrapping_add/sub/mul_wrap() can be used for individual statements. At the function level, the __signed_wrap attribute can be used to mark an entire function as expecting its signed arithmetic to wrap around. For a single object file the Makefile can use "UBSAN_SIGNED_WRAP_target.o := n" to mark it as wrapping, and for an entire directory, "UBSAN_SIGNED_WRAP := n" can be used. Additionally keep these disabled under CONFIG_COMPILE_TEST for now. Link: https://github.com/KSPP/linux/issues/26 [1] Cc: Miguel Ojeda Cc: Nathan Chancellor Cc: Peter Zijlstra Cc: Hao Luo Reviewed-by: Marco Elver Reviewed-by: Justin Stitt Signed-off-by: Kees Cook --- include/linux/compiler_types.h | 9 +++++- lib/Kconfig.ubsan | 15 +++++++++- lib/test_ubsan.c | 37 +++++++++++++++++++++++ lib/ubsan.c | 68 ++++++++++++++++++++++++++++++++++++++++++ lib/ubsan.h | 4 +++ scripts/Makefile.lib | 3 ++ scripts/Makefile.ubsan | 3 ++ 7 files changed, 137 insertions(+), 2 deletions(-) (limited to 'include/linux') diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index 6f1ca49306d2..ee9d272008a5 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -282,11 +282,18 @@ struct ftrace_likely_data { #define __no_sanitize_or_inline __always_inline #endif +/* Do not trap wrapping arithmetic within an annotated function. */ +#ifdef CONFIG_UBSAN_SIGNED_WRAP +# define __signed_wrap __attribute__((no_sanitize("signed-integer-overflow"))) +#else +# define __signed_wrap +#endif + /* Section for code which can't be instrumented at all */ #define __noinstr_section(section) \ noinline notrace __attribute((__section__(section))) \ __no_kcsan __no_sanitize_address __no_profile __no_sanitize_coverage \ - __no_sanitize_memory + __no_sanitize_memory __signed_wrap #define noinstr __noinstr_section(".noinstr.text") diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan index 56d7653f4941..48a67058f84e 100644 --- a/lib/Kconfig.ubsan +++ b/lib/Kconfig.ubsan @@ -87,7 +87,6 @@ config UBSAN_LOCAL_BOUNDS config UBSAN_SHIFT bool "Perform checking for bit-shift overflows" - default UBSAN depends on $(cc-option,-fsanitize=shift) help This option enables -fsanitize=shift which checks for bit-shift @@ -116,6 +115,20 @@ config UBSAN_UNREACHABLE This option enables -fsanitize=unreachable which checks for control flow reaching an expected-to-be-unreachable position. +config UBSAN_SIGNED_WRAP + bool "Perform checking for signed arithmetic wrap-around" + default UBSAN + depends on !COMPILE_TEST + depends on $(cc-option,-fsanitize=signed-integer-overflow) + help + This option enables -fsanitize=signed-integer-overflow which checks + for wrap-around of any arithmetic operations with signed integers. + This currently performs nearly no instrumentation due to the + kernel's use of -fno-strict-overflow which converts all would-be + arithmetic undefined behavior into wrap-around arithmetic. Future + sanitizer versions will allow for wrap-around checking (rather than + exclusively undefined behavior). + config UBSAN_BOOL bool "Perform checking for non-boolean values used as boolean" default UBSAN diff --git a/lib/test_ubsan.c b/lib/test_ubsan.c index f4ee2484d4b5..276c12140ee2 100644 --- a/lib/test_ubsan.c +++ b/lib/test_ubsan.c @@ -11,6 +11,39 @@ typedef void(*test_ubsan_fp)(void); #config, IS_ENABLED(config) ? "y" : "n"); \ } while (0) +static void test_ubsan_add_overflow(void) +{ + volatile int val = INT_MAX; + + UBSAN_TEST(CONFIG_UBSAN_SIGNED_WRAP); + val += 2; +} + +static void test_ubsan_sub_overflow(void) +{ + volatile int val = INT_MIN; + volatile int val2 = 2; + + UBSAN_TEST(CONFIG_UBSAN_SIGNED_WRAP); + val -= val2; +} + +static void test_ubsan_mul_overflow(void) +{ + volatile int val = INT_MAX / 2; + + UBSAN_TEST(CONFIG_UBSAN_SIGNED_WRAP); + val *= 3; +} + +static void test_ubsan_negate_overflow(void) +{ + volatile int val = INT_MIN; + + UBSAN_TEST(CONFIG_UBSAN_SIGNED_WRAP); + val = -val; +} + static void test_ubsan_divrem_overflow(void) { volatile int val = 16; @@ -90,6 +123,10 @@ static void test_ubsan_misaligned_access(void) } static const test_ubsan_fp test_ubsan_array[] = { + test_ubsan_add_overflow, + test_ubsan_sub_overflow, + test_ubsan_mul_overflow, + test_ubsan_negate_overflow, test_ubsan_shift_out_of_bounds, test_ubsan_out_of_bounds, test_ubsan_load_invalid_value, diff --git a/lib/ubsan.c b/lib/ubsan.c index df4f8d1354bb..5fc107f61934 100644 --- a/lib/ubsan.c +++ b/lib/ubsan.c @@ -222,6 +222,74 @@ static void ubsan_epilogue(void) check_panic_on_warn("UBSAN"); } +static void handle_overflow(struct overflow_data *data, void *lhs, + void *rhs, char op) +{ + + struct type_descriptor *type = data->type; + char lhs_val_str[VALUE_LENGTH]; + char rhs_val_str[VALUE_LENGTH]; + + if (suppress_report(&data->location)) + return; + + ubsan_prologue(&data->location, type_is_signed(type) ? + "signed-integer-overflow" : + "unsigned-integer-overflow"); + + val_to_string(lhs_val_str, sizeof(lhs_val_str), type, lhs); + val_to_string(rhs_val_str, sizeof(rhs_val_str), type, rhs); + pr_err("%s %c %s cannot be represented in type %s\n", + lhs_val_str, + op, + rhs_val_str, + type->type_name); + + ubsan_epilogue(); +} + +void __ubsan_handle_add_overflow(void *data, + void *lhs, void *rhs) +{ + + handle_overflow(data, lhs, rhs, '+'); +} +EXPORT_SYMBOL(__ubsan_handle_add_overflow); + +void __ubsan_handle_sub_overflow(void *data, + void *lhs, void *rhs) +{ + handle_overflow(data, lhs, rhs, '-'); +} +EXPORT_SYMBOL(__ubsan_handle_sub_overflow); + +void __ubsan_handle_mul_overflow(void *data, + void *lhs, void *rhs) +{ + handle_overflow(data, lhs, rhs, '*'); +} +EXPORT_SYMBOL(__ubsan_handle_mul_overflow); + +void __ubsan_handle_negate_overflow(void *_data, void *old_val) +{ + struct overflow_data *data = _data; + char old_val_str[VALUE_LENGTH]; + + if (suppress_report(&data->location)) + return; + + ubsan_prologue(&data->location, "negation-overflow"); + + val_to_string(old_val_str, sizeof(old_val_str), data->type, old_val); + + pr_err("negation of %s cannot be represented in type %s:\n", + old_val_str, data->type->type_name); + + ubsan_epilogue(); +} +EXPORT_SYMBOL(__ubsan_handle_negate_overflow); + + void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) { struct overflow_data *data = _data; diff --git a/lib/ubsan.h b/lib/ubsan.h index 5d99ab81913b..0abbbac8700d 100644 --- a/lib/ubsan.h +++ b/lib/ubsan.h @@ -124,6 +124,10 @@ typedef s64 s_max; typedef u64 u_max; #endif +void __ubsan_handle_add_overflow(void *data, void *lhs, void *rhs); +void __ubsan_handle_sub_overflow(void *data, void *lhs, void *rhs); +void __ubsan_handle_mul_overflow(void *data, void *lhs, void *rhs); +void __ubsan_handle_negate_overflow(void *_data, void *old_val); void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs); void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr); void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr); diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 52efc520ae4f..b4a248c20654 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -177,6 +177,9 @@ ifeq ($(CONFIG_UBSAN),y) _c_flags += $(if $(patsubst n%,, \ $(UBSAN_SANITIZE_$(basetarget).o)$(UBSAN_SANITIZE)y), \ $(CFLAGS_UBSAN)) +_c_flags += $(if $(patsubst n%,, \ + $(UBSAN_SIGNED_WRAP_$(basetarget).o)$(UBSAN_SANITIZE_$(basetarget).o)$(UBSAN_SIGNED_WRAP)$(UBSAN_SANITIZE)y), \ + $(CFLAGS_UBSAN_SIGNED_WRAP)) endif ifeq ($(CONFIG_KCOV),y) diff --git a/scripts/Makefile.ubsan b/scripts/Makefile.ubsan index 7cf42231042b..b2d3b273b802 100644 --- a/scripts/Makefile.ubsan +++ b/scripts/Makefile.ubsan @@ -13,3 +13,6 @@ ubsan-cflags-$(CONFIG_UBSAN_ENUM) += -fsanitize=enum ubsan-cflags-$(CONFIG_UBSAN_TRAP) += $(call cc-option,-fsanitize-trap=undefined,-fsanitize-undefined-trap-on-error) export CFLAGS_UBSAN := $(ubsan-cflags-y) + +ubsan-signed-wrap-cflags-$(CONFIG_UBSAN_SIGNED_WRAP) += -fsanitize=signed-integer-overflow +export CFLAGS_UBSAN_SIGNED_WRAP := $(ubsan-signed-wrap-cflags-y) -- cgit v1.2.3 From f478898e0aa74a759fcf629a3ee8b040467b8533 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 2 Feb 2024 03:18:14 -0800 Subject: string: Redefine strscpy_pad() as a macro In preparation for making strscpy_pad()'s 3rd argument optional, redefine it as a macro. This also has the benefit of allowing greater FORITFY introspection, as it couldn't see into the strscpy() nor the memset() within strscpy_pad(). Cc: Andy Shevchenko Cc: Andrew Morton Cc: Reviewed-by: Justin Stitt Signed-off-by: Kees Cook --- include/linux/string.h | 33 +++++++++++++++++++++++++++++++-- lib/string_helpers.c | 34 ---------------------------------- 2 files changed, 31 insertions(+), 36 deletions(-) (limited to 'include/linux') diff --git a/include/linux/string.h b/include/linux/string.h index ab148d8dbfc1..78b28004c5ba 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -70,8 +70,37 @@ extern char * strncpy(char *,const char *, __kernel_size_t); ssize_t strscpy(char *, const char *, size_t); #endif -/* Wraps calls to strscpy()/memset(), no arch specific code required */ -ssize_t strscpy_pad(char *dest, const char *src, size_t count); +/** + * strscpy_pad() - Copy a C-string into a sized buffer + * @dest: Where to copy the string to + * @src: Where to copy the string from + * @count: Size of destination buffer + * + * Copy the string, or as much of it as fits, into the dest buffer. The + * behavior is undefined if the string buffers overlap. The destination + * buffer is always %NUL terminated, unless it's zero-sized. + * + * If the source string is shorter than the destination buffer, the + * remaining bytes in the buffer will be filled with %NUL bytes. + * + * For full explanation of why you may want to consider using the + * 'strscpy' functions please see the function docstring for strscpy(). + * + * Returns: + * * The number of characters copied (not including the trailing %NULs) + * * -E2BIG if count is 0 or @src was truncated. + */ +#define strscpy_pad(dest, src, count) ({ \ + char *__dst = (dest); \ + const char *__src = (src); \ + const size_t __count = (count); \ + ssize_t __wrote; \ + \ + __wrote = strscpy(__dst, __src, __count); \ + if (__wrote >= 0 && __wrote < __count) \ + memset(__dst + __wrote + 1, 0, __count - __wrote - 1); \ + __wrote; \ +}) #ifndef __HAVE_ARCH_STRCAT extern char * strcat(char *, const char *); diff --git a/lib/string_helpers.c b/lib/string_helpers.c index 7713f73e66b0..606c3099013f 100644 --- a/lib/string_helpers.c +++ b/lib/string_helpers.c @@ -825,40 +825,6 @@ char **devm_kasprintf_strarray(struct device *dev, const char *prefix, size_t n) } EXPORT_SYMBOL_GPL(devm_kasprintf_strarray); -/** - * strscpy_pad() - Copy a C-string into a sized buffer - * @dest: Where to copy the string to - * @src: Where to copy the string from - * @count: Size of destination buffer - * - * Copy the string, or as much of it as fits, into the dest buffer. The - * behavior is undefined if the string buffers overlap. The destination - * buffer is always %NUL terminated, unless it's zero-sized. - * - * If the source string is shorter than the destination buffer, zeros - * the tail of the destination buffer. - * - * For full explanation of why you may want to consider using the - * 'strscpy' functions please see the function docstring for strscpy(). - * - * Returns: - * * The number of characters copied (not including the trailing %NUL) - * * -E2BIG if count is 0 or @src was truncated. - */ -ssize_t strscpy_pad(char *dest, const char *src, size_t count) -{ - ssize_t written; - - written = strscpy(dest, src, count); - if (written < 0 || written == count - 1) - return written; - - memset(dest + written + 1, 0, count - written - 1); - - return written; -} -EXPORT_SYMBOL(strscpy_pad); - /** * skip_spaces - Removes leading whitespace from @str. * @str: The string to be stripped. -- cgit v1.2.3 From e6584c3964f2ff76a9fb5a701e4a59997b35e547 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 20 Sep 2023 12:38:14 -0700 Subject: string: Allow 2-argument strscpy() Using sizeof(dst) for the "size" argument in strscpy() is the overwhelmingly common case. Instead of requiring this everywhere, allow a 2-argument version to be used that will use the sizeof() internally. There are other functions in the kernel with optional arguments[1], so this isn't unprecedented, and improves readability. Update and relocate the kern-doc for strscpy() too, and drop __HAVE_ARCH_STRSCPY as it is unused. Adjust ARCH=um build to notice the changed export name, as it doesn't do full header includes for the string helpers. This could additionally let us save a few hundred lines of code: 1177 files changed, 2455 insertions(+), 3026 deletions(-) with a treewide cleanup using Coccinelle: @needless_arg@ expression DST, SRC; @@ strscpy(DST, SRC -, sizeof(DST) ) Link: https://elixir.bootlin.com/linux/v6.7/source/include/linux/pci.h#L1517 [1] Reviewed-by: Justin Stitt Cc: Andy Shevchenko Cc: linux-hardening@vger.kernel.org Signed-off-by: Kees Cook --- arch/um/include/shared/user.h | 3 ++- include/linux/fortify-string.h | 22 ++-------------------- include/linux/string.h | 38 +++++++++++++++++++++++++++++++++++--- lib/string.c | 6 ++---- 4 files changed, 41 insertions(+), 28 deletions(-) (limited to 'include/linux') diff --git a/arch/um/include/shared/user.h b/arch/um/include/shared/user.h index 981e11d8e025..9568cc04cbb7 100644 --- a/arch/um/include/shared/user.h +++ b/arch/um/include/shared/user.h @@ -51,7 +51,8 @@ static inline int printk(const char *fmt, ...) extern int in_aton(char *str); extern size_t strlcat(char *, const char *, size_t); -extern size_t strscpy(char *, const char *, size_t); +extern size_t sized_strscpy(char *, const char *, size_t); +#define strscpy(dst, src, size) sized_strscpy(dst, src, size) /* Copied from linux/compiler-gcc.h since we can't include it directly */ #define barrier() __asm__ __volatile__("": : :"memory") diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h index 89a6888f2f9e..06b3aaa63724 100644 --- a/include/linux/fortify-string.h +++ b/include/linux/fortify-string.h @@ -215,26 +215,8 @@ __kernel_size_t __fortify_strlen(const char * const POS p) } /* Defined after fortified strnlen() to reuse it. */ -extern ssize_t __real_strscpy(char *, const char *, size_t) __RENAME(strscpy); -/** - * strscpy - Copy a C-string into a sized buffer - * - * @p: Where to copy the string to - * @q: Where to copy the string from - * @size: Size of destination buffer - * - * Copy the source string @q, or as much of it as fits, into the destination - * @p buffer. The behavior is undefined if the string buffers overlap. The - * destination @p buffer is always NUL terminated, unless it's zero-sized. - * - * Preferred to strncpy() since it always returns a valid string, and - * doesn't unnecessarily force the tail of the destination buffer to be - * zero padded. If padding is desired please use strscpy_pad(). - * - * Returns the number of characters copied in @p (not including the - * trailing %NUL) or -E2BIG if @size is 0 or the copy of @q was truncated. - */ -__FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, size_t size) +extern ssize_t __real_strscpy(char *, const char *, size_t) __RENAME(sized_strscpy); +__FORTIFY_INLINE ssize_t sized_strscpy(char * const POS p, const char * const POS q, size_t size) { /* Use string size rather than possible enclosing struct size. */ const size_t p_size = __member_size(p); diff --git a/include/linux/string.h b/include/linux/string.h index 78b28004c5ba..0d66bf9407fd 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -2,6 +2,7 @@ #ifndef _LINUX_STRING_H_ #define _LINUX_STRING_H_ +#include #include #include /* for inline */ #include /* for size_t */ @@ -66,9 +67,40 @@ extern char * strcpy(char *,const char *); #ifndef __HAVE_ARCH_STRNCPY extern char * strncpy(char *,const char *, __kernel_size_t); #endif -#ifndef __HAVE_ARCH_STRSCPY -ssize_t strscpy(char *, const char *, size_t); -#endif +ssize_t sized_strscpy(char *, const char *, size_t); + +/* + * The 2 argument style can only be used when dst is an array with a + * known size. + */ +#define __strscpy0(dst, src, ...) \ + sized_strscpy(dst, src, sizeof(dst) + __must_be_array(dst)) +#define __strscpy1(dst, src, size) sized_strscpy(dst, src, size) + +/** + * strscpy - Copy a C-string into a sized buffer + * @dst: Where to copy the string to + * @src: Where to copy the string from + * @...: Size of destination buffer (optional) + * + * Copy the source string @src, or as much of it as fits, into the + * destination @dst buffer. The behavior is undefined if the string + * buffers overlap. The destination @dst buffer is always NUL terminated, + * unless it's zero-sized. + * + * The size argument @... is only required when @dst is not an array, or + * when the copy needs to be smaller than sizeof(@dst). + * + * Preferred to strncpy() since it always returns a valid string, and + * doesn't unnecessarily force the tail of the destination buffer to be + * zero padded. If padding is desired please use strscpy_pad(). + * + * Returns the number of characters copied in @dst (not including the + * trailing %NUL) or -E2BIG if @size is 0 or the copy from @src was + * truncated. + */ +#define strscpy(dst, src, ...) \ + CONCATENATE(__strscpy, COUNT_ARGS(__VA_ARGS__))(dst, src, __VA_ARGS__) /** * strscpy_pad() - Copy a C-string into a sized buffer diff --git a/lib/string.c b/lib/string.c index f791559102f6..966da44bfc86 100644 --- a/lib/string.c +++ b/lib/string.c @@ -104,8 +104,7 @@ char *strncpy(char *dest, const char *src, size_t count) EXPORT_SYMBOL(strncpy); #endif -#ifndef __HAVE_ARCH_STRSCPY -ssize_t strscpy(char *dest, const char *src, size_t count) +ssize_t sized_strscpy(char *dest, const char *src, size_t count) { const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS; size_t max = count; @@ -171,8 +170,7 @@ ssize_t strscpy(char *dest, const char *src, size_t count) return -E2BIG; } -EXPORT_SYMBOL(strscpy); -#endif +EXPORT_SYMBOL(sized_strscpy); /** * stpcpy - copy a string from src to dest returning a pointer to the new end -- cgit v1.2.3 From 8366d124ec937c3815212c00daf00b687eb27969 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 2 Feb 2024 03:40:23 -0800 Subject: string: Allow 2-argument strscpy_pad() Similar to strscpy(), update strscpy_pad()'s 3rd argument to be optional when the destination is a compile-time known size array. Cc: Andy Shevchenko Cc: Reviewed-by: Justin Stitt Signed-off-by: Kees Cook --- include/linux/string.h | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) (limited to 'include/linux') diff --git a/include/linux/string.h b/include/linux/string.h index 0d66bf9407fd..96e6b1af86b5 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -77,6 +77,10 @@ ssize_t sized_strscpy(char *, const char *, size_t); sized_strscpy(dst, src, sizeof(dst) + __must_be_array(dst)) #define __strscpy1(dst, src, size) sized_strscpy(dst, src, size) +#define __strscpy_pad0(dst, src, ...) \ + sized_strscpy_pad(dst, src, sizeof(dst) + __must_be_array(dst)) +#define __strscpy_pad1(dst, src, size) sized_strscpy_pad(dst, src, size) + /** * strscpy - Copy a C-string into a sized buffer * @dst: Where to copy the string to @@ -102,11 +106,23 @@ ssize_t sized_strscpy(char *, const char *, size_t); #define strscpy(dst, src, ...) \ CONCATENATE(__strscpy, COUNT_ARGS(__VA_ARGS__))(dst, src, __VA_ARGS__) +#define sized_strscpy_pad(dest, src, count) ({ \ + char *__dst = (dest); \ + const char *__src = (src); \ + const size_t __count = (count); \ + ssize_t __wrote; \ + \ + __wrote = sized_strscpy(__dst, __src, __count); \ + if (__wrote >= 0 && __wrote < __count) \ + memset(__dst + __wrote + 1, 0, __count - __wrote - 1); \ + __wrote; \ +}) + /** * strscpy_pad() - Copy a C-string into a sized buffer - * @dest: Where to copy the string to + * @dst: Where to copy the string to * @src: Where to copy the string from - * @count: Size of destination buffer + * @...: Size of destination buffer * * Copy the string, or as much of it as fits, into the dest buffer. The * behavior is undefined if the string buffers overlap. The destination @@ -122,17 +138,8 @@ ssize_t sized_strscpy(char *, const char *, size_t); * * The number of characters copied (not including the trailing %NULs) * * -E2BIG if count is 0 or @src was truncated. */ -#define strscpy_pad(dest, src, count) ({ \ - char *__dst = (dest); \ - const char *__src = (src); \ - const size_t __count = (count); \ - ssize_t __wrote; \ - \ - __wrote = strscpy(__dst, __src, __count); \ - if (__wrote >= 0 && __wrote < __count) \ - memset(__dst + __wrote + 1, 0, __count - __wrote - 1); \ - __wrote; \ -}) +#define strscpy_pad(dst, src, ...) \ + CONCATENATE(__strscpy_pad, COUNT_ARGS(__VA_ARGS__))(dst, src, __VA_ARGS__) #ifndef __HAVE_ARCH_STRCAT extern char * strcat(char *, const char *); -- cgit v1.2.3 From adeb04362d74188c1e22ccb824b15a0a7b3de2f4 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 14 Feb 2024 19:26:32 +0200 Subject: kernel.h: Move upper_*_bits() and lower_*_bits() to wordpart.h The wordpart.h header is collecting APIs related to the handling parts of the word (usually in byte granularity). The upper_*_bits() and lower_*_bits() are good candidates to be moved to there. This helps to clean up header dependency hell with regard to kernel.h as the latter gathers completely unrelated stuff together and slows down compilation (especially when it's included into other header). Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20240214172752.3605073-1-andriy.shevchenko@linux.intel.com Reviewed-by: Randy Dunlap Signed-off-by: Kees Cook --- include/linux/kernel.h | 30 ++---------------------------- include/linux/wordpart.h | 29 +++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 28 deletions(-) (limited to 'include/linux') diff --git a/include/linux/kernel.h b/include/linux/kernel.h index f4a1d582b79d..86dd8939c2cd 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -33,6 +33,8 @@ #include #include #include +#include + #include #include @@ -52,34 +54,6 @@ } \ ) -/** - * upper_32_bits - return bits 32-63 of a number - * @n: the number we're accessing - * - * A basic shift-right of a 64- or 32-bit quantity. Use this to suppress - * the "right shift count >= width of type" warning when that quantity is - * 32-bits. - */ -#define upper_32_bits(n) ((u32)(((n) >> 16) >> 16)) - -/** - * lower_32_bits - return bits 0-31 of a number - * @n: the number we're accessing - */ -#define lower_32_bits(n) ((u32)((n) & 0xffffffff)) - -/** - * upper_16_bits - return bits 16-31 of a number - * @n: the number we're accessing - */ -#define upper_16_bits(n) ((u16)((n) >> 16)) - -/** - * lower_16_bits - return bits 0-15 of a number - * @n: the number we're accessing - */ -#define lower_16_bits(n) ((u16)((n) & 0xffff)) - struct completion; struct user; diff --git a/include/linux/wordpart.h b/include/linux/wordpart.h index c9e6bd773ebd..f6f8f83b15b0 100644 --- a/include/linux/wordpart.h +++ b/include/linux/wordpart.h @@ -2,6 +2,35 @@ #ifndef _LINUX_WORDPART_H #define _LINUX_WORDPART_H + +/** + * upper_32_bits - return bits 32-63 of a number + * @n: the number we're accessing + * + * A basic shift-right of a 64- or 32-bit quantity. Use this to suppress + * the "right shift count >= width of type" warning when that quantity is + * 32-bits. + */ +#define upper_32_bits(n) ((u32)(((n) >> 16) >> 16)) + +/** + * lower_32_bits - return bits 0-31 of a number + * @n: the number we're accessing + */ +#define lower_32_bits(n) ((u32)((n) & 0xffffffff)) + +/** + * upper_16_bits - return bits 16-31 of a number + * @n: the number we're accessing + */ +#define upper_16_bits(n) ((u16)((n) >> 16)) + +/** + * lower_16_bits - return bits 0-15 of a number + * @n: the number we're accessing + */ +#define lower_16_bits(n) ((u16)((n) & 0xffff)) + /** * REPEAT_BYTE - repeat the value @x multiple times as an unsigned long value * @x: value to repeat -- cgit v1.2.3 From 3e19086fb5a9079611de426e8cb2f4503e28757e Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 29 Jan 2024 10:21:58 -0800 Subject: overflow: Adjust check_*_overflow() kern-doc to reflect results The check_*_overflow() helpers will return results with potentially wrapped-around values. These values have always been checked by the selftests, so avoid the confusing language in the kern-doc. The idea of "safe for use" was relative to the expectation of whether or not the caller wants a wrapped value -- the calculation itself will always follow arithmetic wrapping rules. Reviewed-by: Gustavo A. R. Silva Acked-by: Mark Rutland Signed-off-by: Kees Cook --- include/linux/overflow.h | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) (limited to 'include/linux') diff --git a/include/linux/overflow.h b/include/linux/overflow.h index 7b5cf4a5cd19..ad64d810c8aa 100644 --- a/include/linux/overflow.h +++ b/include/linux/overflow.h @@ -57,11 +57,10 @@ static inline bool __must_check __must_check_overflow(bool overflow) * @b: second addend * @d: pointer to store sum * - * Returns 0 on success. + * Returns true on wrap-around, false otherwise. * - * *@d holds the results of the attempted addition, but is not considered - * "safe for use" on a non-zero return value, which indicates that the - * sum has overflowed or been truncated. + * *@d holds the results of the attempted addition, regardless of whether + * wrap-around occurred. */ #define check_add_overflow(a, b, d) \ __must_check_overflow(__builtin_add_overflow(a, b, d)) @@ -72,11 +71,10 @@ static inline bool __must_check __must_check_overflow(bool overflow) * @b: subtrahend; value to subtract from @a * @d: pointer to store difference * - * Returns 0 on success. + * Returns true on wrap-around, false otherwise. * - * *@d holds the results of the attempted subtraction, but is not considered - * "safe for use" on a non-zero return value, which indicates that the - * difference has underflowed or been truncated. + * *@d holds the results of the attempted subtraction, regardless of whether + * wrap-around occurred. */ #define check_sub_overflow(a, b, d) \ __must_check_overflow(__builtin_sub_overflow(a, b, d)) @@ -87,11 +85,10 @@ static inline bool __must_check __must_check_overflow(bool overflow) * @b: second factor * @d: pointer to store product * - * Returns 0 on success. + * Returns true on wrap-around, false otherwise. * - * *@d holds the results of the attempted multiplication, but is not - * considered "safe for use" on a non-zero return value, which indicates - * that the product has overflowed or been truncated. + * *@d holds the results of the attempted multiplication, regardless of whether + * wrap-around occurred. */ #define check_mul_overflow(a, b, d) \ __must_check_overflow(__builtin_mul_overflow(a, b, d)) -- cgit v1.2.3 From d70de8054c58d7bd9a4654c9f4797d29fa46d545 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 18 Jan 2024 16:05:52 -0800 Subject: overflow: Introduce wrapping_add(), wrapping_sub(), and wrapping_mul() Provide helpers that will perform wrapping addition, subtraction, or multiplication without tripping the arithmetic wrap-around sanitizers. The first argument is the type under which the wrap-around should happen with. In other words, these two calls will get very different results: wrapping_mul(int, 50, 50) == 2500 wrapping_mul(u8, 50, 50) == 196 Add to the selftests to validate behavior and lack of side-effects. Reviewed-by: Gustavo A. R. Silva Reviewed-by: Marco Elver Acked-by: Mark Rutland Signed-off-by: Kees Cook --- include/linux/overflow.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ lib/overflow_kunit.c | 24 ++++++++++++++++++++---- 2 files changed, 68 insertions(+), 4 deletions(-) (limited to 'include/linux') diff --git a/include/linux/overflow.h b/include/linux/overflow.h index ad64d810c8aa..d3ff8e2bec29 100644 --- a/include/linux/overflow.h +++ b/include/linux/overflow.h @@ -65,6 +65,22 @@ static inline bool __must_check __must_check_overflow(bool overflow) #define check_add_overflow(a, b, d) \ __must_check_overflow(__builtin_add_overflow(a, b, d)) +/** + * wrapping_add() - Intentionally perform a wrapping addition + * @type: type for result of calculation + * @a: first addend + * @b: second addend + * + * Return the potentially wrapped-around addition without + * tripping any wrap-around sanitizers that may be enabled. + */ +#define wrapping_add(type, a, b) \ + ({ \ + type __val; \ + __builtin_add_overflow(a, b, &__val); \ + __val; \ + }) + /** * check_sub_overflow() - Calculate subtraction with overflow checking * @a: minuend; value to subtract from @@ -79,6 +95,22 @@ static inline bool __must_check __must_check_overflow(bool overflow) #define check_sub_overflow(a, b, d) \ __must_check_overflow(__builtin_sub_overflow(a, b, d)) +/** + * wrapping_sub() - Intentionally perform a wrapping subtraction + * @type: type for result of calculation + * @a: minuend; value to subtract from + * @b: subtrahend; value to subtract from @a + * + * Return the potentially wrapped-around subtraction without + * tripping any wrap-around sanitizers that may be enabled. + */ +#define wrapping_sub(type, a, b) \ + ({ \ + type __val; \ + __builtin_sub_overflow(a, b, &__val); \ + __val; \ + }) + /** * check_mul_overflow() - Calculate multiplication with overflow checking * @a: first factor @@ -93,6 +125,22 @@ static inline bool __must_check __must_check_overflow(bool overflow) #define check_mul_overflow(a, b, d) \ __must_check_overflow(__builtin_mul_overflow(a, b, d)) +/** + * wrapping_mul() - Intentionally perform a wrapping multiplication + * @type: type for result of calculation + * @a: first factor + * @b: second factor + * + * Return the potentially wrapped-around multiplication without + * tripping any wrap-around sanitizers that may be enabled. + */ +#define wrapping_mul(type, a, b) \ + ({ \ + type __val; \ + __builtin_mul_overflow(a, b, &__val); \ + __val; \ + }) + /** * check_shl_overflow() - Calculate a left-shifted value and check overflow * @a: Value to be shifted diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c index c527f6b75789..d3fdb906d3fe 100644 --- a/lib/overflow_kunit.c +++ b/lib/overflow_kunit.c @@ -258,20 +258,36 @@ DEFINE_TEST_ARRAY(s64) = { \ _of = check_ ## op ## _overflow(a, b, &_r); \ KUNIT_EXPECT_EQ_MSG(test, _of, of, \ - "expected "fmt" "sym" "fmt" to%s overflow (type %s)\n", \ + "expected check "fmt" "sym" "fmt" to%s overflow (type %s)\n", \ a, b, of ? "" : " not", #t); \ KUNIT_EXPECT_EQ_MSG(test, _r, r, \ - "expected "fmt" "sym" "fmt" == "fmt", got "fmt" (type %s)\n", \ + "expected check "fmt" "sym" "fmt" == "fmt", got "fmt" (type %s)\n", \ a, b, r, _r, #t); \ /* Check for internal macro side-effects. */ \ _of = check_ ## op ## _overflow(_a_orig++, _b_orig++, &_r); \ - KUNIT_EXPECT_EQ_MSG(test, _a_orig, _a_bump, "Unexpected " #op " macro side-effect!\n"); \ - KUNIT_EXPECT_EQ_MSG(test, _b_orig, _b_bump, "Unexpected " #op " macro side-effect!\n"); \ + KUNIT_EXPECT_EQ_MSG(test, _a_orig, _a_bump, \ + "Unexpected check " #op " macro side-effect!\n"); \ + KUNIT_EXPECT_EQ_MSG(test, _b_orig, _b_bump, \ + "Unexpected check " #op " macro side-effect!\n"); \ + \ + _r = wrapping_ ## op(t, a, b); \ + KUNIT_EXPECT_TRUE_MSG(test, _r == r, \ + "expected wrap "fmt" "sym" "fmt" == "fmt", got "fmt" (type %s)\n", \ + a, b, r, _r, #t); \ + /* Check for internal macro side-effects. */ \ + _a_orig = a; \ + _b_orig = b; \ + _r = wrapping_ ## op(t, _a_orig++, _b_orig++); \ + KUNIT_EXPECT_EQ_MSG(test, _a_orig, _a_bump, \ + "Unexpected wrap " #op " macro side-effect!\n"); \ + KUNIT_EXPECT_EQ_MSG(test, _b_orig, _b_bump, \ + "Unexpected wrap " #op " macro side-effect!\n"); \ } while (0) #define DEFINE_TEST_FUNC_TYPED(n, t, fmt) \ static void do_test_ ## n(struct kunit *test, const struct test_ ## n *p) \ { \ + /* check_{add,sub,mul}_overflow() and wrapping_{add,sub,mul} */ \ check_one_op(t, fmt, add, "+", p->a, p->b, p->sum, p->s_of); \ check_one_op(t, fmt, add, "+", p->b, p->a, p->sum, p->s_of); \ check_one_op(t, fmt, sub, "-", p->a, p->b, p->diff, p->d_of); \ -- cgit v1.2.3 From 08d45ee84bb2650e237e150caca87cc4ded9b3e2 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 26 Jan 2024 22:09:50 -0800 Subject: overflow: Introduce wrapping_assign_add() and wrapping_assign_sub() This allows replacements of the idioms "var += offset" and "var -= offset" with the wrapping_assign_add() and wrapping_assign_sub() helpers respectively. They will avoid wrap-around sanitizer instrumentation. Add to the selftests to validate behavior and lack of side-effects. Reviewed-by: Marco Elver Acked-by: Mark Rutland Signed-off-by: Kees Cook --- include/linux/overflow.h | 32 ++++++++++++++++++++++++++++++++ lib/overflow_kunit.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) (limited to 'include/linux') diff --git a/include/linux/overflow.h b/include/linux/overflow.h index d3ff8e2bec29..dede374832c9 100644 --- a/include/linux/overflow.h +++ b/include/linux/overflow.h @@ -81,6 +81,22 @@ static inline bool __must_check __must_check_overflow(bool overflow) __val; \ }) +/** + * wrapping_assign_add() - Intentionally perform a wrapping increment assignment + * @var: variable to be incremented + * @offset: amount to add + * + * Increments @var by @offset with wrap-around. Returns the resulting + * value of @var. Will not trip any wrap-around sanitizers. + * + * Returns the new value of @var. + */ +#define wrapping_assign_add(var, offset) \ + ({ \ + typeof(var) *__ptr = &(var); \ + *__ptr = wrapping_add(typeof(var), *__ptr, offset); \ + }) + /** * check_sub_overflow() - Calculate subtraction with overflow checking * @a: minuend; value to subtract from @@ -111,6 +127,22 @@ static inline bool __must_check __must_check_overflow(bool overflow) __val; \ }) +/** + * wrapping_assign_sub() - Intentionally perform a wrapping decrement assign + * @var: variable to be decremented + * @offset: amount to subtract + * + * Decrements @var by @offset with wrap-around. Returns the resulting + * value of @var. Will not trip any wrap-around sanitizers. + * + * Returns the new value of @var. + */ +#define wrapping_assign_sub(var, offset) \ + ({ \ + typeof(var) *__ptr = &(var); \ + *__ptr = wrapping_sub(typeof(var), *__ptr, offset); \ + }) + /** * check_mul_overflow() - Calculate multiplication with overflow checking * @a: first factor diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c index d3fdb906d3fe..65e8a72a83bf 100644 --- a/lib/overflow_kunit.c +++ b/lib/overflow_kunit.c @@ -284,6 +284,45 @@ DEFINE_TEST_ARRAY(s64) = { "Unexpected wrap " #op " macro side-effect!\n"); \ } while (0) +static int global_counter; +static void bump_counter(void) +{ + global_counter++; +} + +static int get_index(void) +{ + volatile int index = 0; + bump_counter(); + return index; +} + +#define check_self_op(fmt, op, sym, a, b) do { \ + typeof(a + 0) _a = a; \ + typeof(b + 0) _b = b; \ + typeof(a + 0) _a_sym = a; \ + typeof(a + 0) _a_orig[1] = { a }; \ + typeof(b + 0) _b_orig = b; \ + typeof(b + 0) _b_bump = b + 1; \ + typeof(a + 0) _r; \ + \ + _a_sym sym _b; \ + _r = wrapping_ ## op(_a, _b); \ + KUNIT_EXPECT_TRUE_MSG(test, _r == _a_sym, \ + "expected "fmt" "#op" "fmt" == "fmt", got "fmt"\n", \ + a, b, _a_sym, _r); \ + KUNIT_EXPECT_TRUE_MSG(test, _a == _a_sym, \ + "expected "fmt" "#op" "fmt" == "fmt", got "fmt"\n", \ + a, b, _a_sym, _a); \ + /* Check for internal macro side-effects. */ \ + global_counter = 0; \ + wrapping_ ## op(_a_orig[get_index()], _b_orig++); \ + KUNIT_EXPECT_EQ_MSG(test, global_counter, 1, \ + "Unexpected wrapping_" #op " macro side-effect on arg1!\n"); \ + KUNIT_EXPECT_EQ_MSG(test, _b_orig, _b_bump, \ + "Unexpected wrapping_" #op " macro side-effect on arg2!\n"); \ +} while (0) + #define DEFINE_TEST_FUNC_TYPED(n, t, fmt) \ static void do_test_ ## n(struct kunit *test, const struct test_ ## n *p) \ { \ @@ -293,6 +332,10 @@ static void do_test_ ## n(struct kunit *test, const struct test_ ## n *p) \ check_one_op(t, fmt, sub, "-", p->a, p->b, p->diff, p->d_of); \ check_one_op(t, fmt, mul, "*", p->a, p->b, p->prod, p->p_of); \ check_one_op(t, fmt, mul, "*", p->b, p->a, p->prod, p->p_of); \ + /* wrapping_assign_{add,sub}() */ \ + check_self_op(fmt, assign_add, +=, p->a, p->b); \ + check_self_op(fmt, assign_add, +=, p->b, p->a); \ + check_self_op(fmt, assign_sub, -=, p->a, p->b); \ } \ \ static void n ## _overflow_test(struct kunit *test) { \ -- cgit v1.2.3 From 9ca5facd0400f610f3f7f71aeb7fc0b949a48c67 Mon Sep 17 00:00:00 2001 From: Michal Wajdeczko Date: Wed, 14 Feb 2024 17:50:15 +0100 Subject: lib/string_choices: Add str_plural() helper Add str_plural() helper to replace existing open implementations used by many drivers and help improve future user facing messages. Signed-off-by: Michal Wajdeczko Link: https://lore.kernel.org/r/20240214165015.1656-1-michal.wajdeczko@intel.com Signed-off-by: Kees Cook --- include/linux/string_choices.h | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'include/linux') diff --git a/include/linux/string_choices.h b/include/linux/string_choices.h index 3c1091941eb8..d9ebe20229f8 100644 --- a/include/linux/string_choices.h +++ b/include/linux/string_choices.h @@ -42,4 +42,15 @@ static inline const char *str_yes_no(bool v) return v ? "yes" : "no"; } +/** + * str_plural - Return the simple pluralization based on English counts + * @num: Number used for deciding pluralization + * + * If @num is 1, returns empty string, otherwise returns "s". + */ +static inline const char *str_plural(size_t num) +{ + return num == 1 ? "" : "s"; +} + #endif -- cgit v1.2.3 From 99db710f768e988e70f1164537bf533a017be24d Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 20 Feb 2024 21:16:38 -0800 Subject: refcount: Annotated intentional signed integer wrap-around Mark the various refcount_t functions with __signed_wrap, as we depend on the wrapping behavior to detect the overflow and perform saturation. Silences warnings seen with the LKDTM REFCOUNT_* tests: UBSAN: signed-integer-overflow in ../include/linux/refcount.h:189:11 2147483647 + 1 cannot be represented in type 'int' Reviewed-by: Miguel Ojeda Link: https://lore.kernel.org/r/20240221051634.work.287-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/refcount.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'include/linux') diff --git a/include/linux/refcount.h b/include/linux/refcount.h index 85c6df0d1bef..59b3b752394d 100644 --- a/include/linux/refcount.h +++ b/include/linux/refcount.h @@ -136,7 +136,8 @@ static inline unsigned int refcount_read(const refcount_t *r) return atomic_read(&r->refs); } -static inline __must_check bool __refcount_add_not_zero(int i, refcount_t *r, int *oldp) +static inline __must_check __signed_wrap +bool __refcount_add_not_zero(int i, refcount_t *r, int *oldp) { int old = refcount_read(r); @@ -177,7 +178,8 @@ static inline __must_check bool refcount_add_not_zero(int i, refcount_t *r) return __refcount_add_not_zero(i, r, NULL); } -static inline void __refcount_add(int i, refcount_t *r, int *oldp) +static inline __signed_wrap +void __refcount_add(int i, refcount_t *r, int *oldp) { int old = atomic_fetch_add_relaxed(i, &r->refs); @@ -256,7 +258,8 @@ static inline void refcount_inc(refcount_t *r) __refcount_inc(r, NULL); } -static inline __must_check bool __refcount_sub_and_test(int i, refcount_t *r, int *oldp) +static inline __must_check __signed_wrap +bool __refcount_sub_and_test(int i, refcount_t *r, int *oldp) { int old = atomic_fetch_sub_release(i, &r->refs); -- cgit v1.2.3 From 475ddf1fce1ec4826c8dda40ec59f7f83a7aadb8 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 7 Apr 2023 12:27:13 -0700 Subject: fortify: Split reporting and avoid passing string pointer 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 a tiny bit of 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 Reviewed-by: Alexander Lobakin Signed-off-by: Kees Cook --- arch/arm/boot/compressed/misc.c | 2 +- arch/arm/boot/compressed/misc.h | 2 +- arch/x86/boot/compressed/misc.c | 2 +- include/linux/fortify-string.h | 81 ++++++++++++++++++++++++++++++----------- lib/string_helpers.c | 23 ++++++++++-- tools/objtool/noreturns.h | 2 +- 6 files changed, 84 insertions(+), 28 deletions(-) (limited to 'include/linux') diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c index 6b4baa6a9a50..d93e2e466f6a 100644 --- a/arch/arm/boot/compressed/misc.c +++ b/arch/arm/boot/compressed/misc.c @@ -154,7 +154,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/arm/boot/compressed/misc.h b/arch/arm/boot/compressed/misc.h index 6da00a26ac08..4d59c427253c 100644 --- a/arch/arm/boot/compressed/misc.h +++ b/arch/arm/boot/compressed/misc.h @@ -10,7 +10,7 @@ void __div0(void); void decompress_kernel(unsigned long output_start, unsigned long free_mem_ptr_p, unsigned long free_mem_ptr_end_p, int arch_id); -void fortify_panic(const char *name); +void __fortify_panic(const u8 reason); int atags_to_fdt(void *atag_list, void *fdt, int total_space); uint32_t fdt_check_mem_start(uint32_t mem_start, const void *fdt); int do_decompress(u8 *input, int len, u8 *output, void (*error)(char *x)); diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c index b99e08e6815b..c9971b9dbb09 100644 --- a/arch/x86/boot/compressed/misc.c +++ b/arch/x86/boot/compressed/misc.c @@ -496,7 +496,7 @@ asmlinkage __visible void *extract_kernel(void *rmode, unsigned char *output) 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 06b3aaa63724..4f6767dcd933 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 #include #include #include @@ -9,7 +10,44 @@ #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(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()?"); @@ -143,7 +181,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); } @@ -174,7 +212,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; } @@ -210,7 +248,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; } @@ -261,7 +299,7 @@ __FORTIFY_INLINE ssize_t sized_strscpy(char * const POS p, const char * const PO * 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: @@ -319,7 +357,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; @@ -328,7 +366,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'; @@ -357,7 +395,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; } @@ -393,7 +431,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; @@ -434,7 +472,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) ({ \ @@ -488,7 +526,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)) { /* @@ -532,9 +570,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. @@ -567,7 +606,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_LINE, \ @@ -634,7 +673,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); } @@ -651,7 +690,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); } @@ -663,7 +702,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); } @@ -675,7 +714,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); } @@ -688,7 +727,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); } @@ -725,7 +764,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 606c3099013f..9291dc74ae01 100644 --- a/lib/string_helpers.c +++ b/lib/string_helpers.c @@ -1008,10 +1008,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) +static const char * const fortify_func_name[] = { +#define MAKE_FORTIFY_FUNC_NAME(func) [MAKE_FORTIFY_FUNC(func)] = #func + EACH_FORTIFY_FUNC(MAKE_FORTIFY_FUNC_NAME) +#undef 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[umin(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/noreturns.h b/tools/objtool/noreturns.h index 1685d7ea6a9f..3a301696f005 100644 --- a/tools/objtool/noreturns.h +++ b/tools/objtool/noreturns.h @@ -6,6 +6,7 @@ * * Yes, this is unfortunate. A better solution is in the works. */ +NORETURN(__fortify_panic) NORETURN(__kunit_abort) NORETURN(__module_put_and_kthread_exit) NORETURN(__reiserfs_panic) @@ -22,7 +23,6 @@ NORETURN(do_exit) NORETURN(do_group_exit) NORETURN(do_task_dead) NORETURN(ex_handler_msr_mce) -NORETURN(fortify_panic) NORETURN(hlt_play_dead) NORETURN(hv_ghcb_terminate) NORETURN(kthread_complete_and_exit) -- cgit v1.2.3 From 4ce615e798a752d4431fcc52960478906dec2f0e Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 7 Apr 2023 12:27:14 -0700 Subject: fortify: Provide KUnit counters for failure testing 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 of 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 --- include/linux/fortify-string.h | 43 ++++++++++++++++++++++-------------------- lib/fortify_kunit.c | 41 ++++++++++++++++++++++++++++++++++++++++ lib/string_helpers.c | 2 ++ 3 files changed, 66 insertions(+), 20 deletions(-) (limited to 'include/linux') diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h index 4f6767dcd933..fbfb90479b8f 100644 --- a/include/linux/fortify-string.h +++ b/include/linux/fortify-string.h @@ -15,8 +15,10 @@ #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)) +#ifndef fortify_panic +# define fortify_panic(func, write, retfail) \ + __fortify_panic(FORTIFY_REASON(func, write)) +#endif #define FORTIFY_READ 0 #define FORTIFY_WRITE 1 @@ -181,7 +183,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); } @@ -212,7 +214,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; } @@ -248,7 +250,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; } @@ -299,7 +301,7 @@ __FORTIFY_INLINE ssize_t sized_strscpy(char * const POS p, const char * const PO * 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: @@ -357,7 +359,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; @@ -366,7 +368,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'; @@ -395,7 +397,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; } @@ -431,13 +433,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) { @@ -472,7 +474,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) ({ \ @@ -571,9 +574,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. @@ -673,7 +676,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); } @@ -690,7 +693,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); } @@ -702,7 +705,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); } @@ -714,7 +717,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); } @@ -727,7 +730,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); } @@ -764,7 +767,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 7a88b5dd3d27..4ba7d02fdd78 100644 --- a/lib/fortify_kunit.c +++ b/lib/fortify_kunit.c @@ -15,8 +15,17 @@ */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +/* Redefine fortify_panic() to track failures. */ +void fortify_add_kunit_error(int write); +#define fortify_panic(func, write, retfail) do { \ + __fortify_report(FORTIFY_REASON(func, write)); \ + fortify_add_kunit_error(write); \ + return (retfail); \ +} while (0) + #include #include +#include #include #include #include @@ -27,10 +36,34 @@ # define __compiletime_strlen __builtin_strlen #endif +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"; +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); @@ -318,6 +351,14 @@ 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 9291dc74ae01..5e53d42e32bb 100644 --- a/lib/string_helpers.c +++ b/lib/string_helpers.c @@ -18,6 +18,8 @@ #include #include #include +#include +#include /** * string_get_size - get the size in the specified units -- cgit v1.2.3 From 3d965b33e40d973b450cb0212913f039476c16f4 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 7 Apr 2023 12:27:16 -0700 Subject: fortify: Improve buffer overflow reporting 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 of buffer size 1 Link: https://lore.kernel.org/r/20230407192717.636137-10-keescook@chromium.org Signed-off-by: Kees Cook --- arch/arm/boot/compressed/misc.c | 2 +- arch/arm/boot/compressed/misc.h | 2 +- arch/x86/boot/compressed/misc.c | 2 +- include/linux/fortify-string.h | 56 ++++++++++++++++++++++------------------- lib/fortify_kunit.c | 4 +-- lib/string_helpers.c | 9 ++++--- 6 files changed, 40 insertions(+), 35 deletions(-) (limited to 'include/linux') diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c index d93e2e466f6a..6c41b270560e 100644 --- a/arch/arm/boot/compressed/misc.c +++ b/arch/arm/boot/compressed/misc.c @@ -154,7 +154,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/arm/boot/compressed/misc.h b/arch/arm/boot/compressed/misc.h index 4d59c427253c..8c73940b5fe4 100644 --- a/arch/arm/boot/compressed/misc.h +++ b/arch/arm/boot/compressed/misc.h @@ -10,7 +10,7 @@ void __div0(void); void decompress_kernel(unsigned long output_start, unsigned long free_mem_ptr_p, unsigned long free_mem_ptr_end_p, int arch_id); -void __fortify_panic(const u8 reason); +void __fortify_panic(const u8 reason, size_t avail, size_t size); int atags_to_fdt(void *atag_list, void *fdt, int total_space); uint32_t fdt_check_mem_start(uint32_t mem_start, const void *fdt); int do_decompress(u8 *input, int len, u8 *output, void (*error)(char *x)); diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c index c9971b9dbb09..1844da203da9 100644 --- a/arch/x86/boot/compressed/misc.c +++ b/arch/x86/boot/compressed/misc.c @@ -496,7 +496,7 @@ asmlinkage __visible void *extract_kernel(void *rmode, unsigned char *output) 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 fbfb90479b8f..6aeebe0a6777 100644 --- a/include/linux/fortify-string.h +++ b/include/linux/fortify-string.h @@ -16,8 +16,8 @@ FIELD_PREP(GENMASK(7, 1), func)) #ifndef fortify_panic -# 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 @@ -48,8 +48,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()?"); @@ -183,7 +183,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); } @@ -214,7 +214,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; } @@ -250,7 +250,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; } @@ -300,8 +300,8 @@ __FORTIFY_INLINE ssize_t sized_strscpy(char * const POS p, const char * const PO * 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: @@ -359,7 +359,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; @@ -368,7 +368,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'; @@ -395,9 +395,10 @@ __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); + const size_t wanted = strlcat(p, q, p_size); - if (strlcat(p, q, p_size) >= p_size) - fortify_panic(FORTIFY_FUNC_strcat, FORTIFY_WRITE, p); + if (p_size <= wanted) + fortify_panic(FORTIFY_FUNC_strcat, FORTIFY_WRITE, p_size, wanted + 1, p); return p; } @@ -426,14 +427,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; @@ -474,7 +476,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; } @@ -574,9 +576,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. @@ -676,7 +678,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); } @@ -692,8 +694,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); } @@ -705,7 +709,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); } @@ -717,7 +721,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); } @@ -730,7 +734,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); } @@ -767,7 +771,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 f0accebeca02..493ec02dd5b3 100644 --- a/lib/fortify_kunit.c +++ b/lib/fortify_kunit.c @@ -17,8 +17,8 @@ /* Redefine fortify_panic() to track failures. */ void fortify_add_kunit_error(int write); -#define fortify_panic(func, write, retfail) do { \ - __fortify_report(FORTIFY_REASON(func, write)); \ +#define fortify_panic(func, write, avail, size, retfail) do { \ + __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 5e53d42e32bb..6bbafd6a10d9 100644 --- a/lib/string_helpers.c +++ b/lib/string_helpers.c @@ -1016,20 +1016,21 @@ static const char * const fortify_func_name[] = { #undef 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[umin(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 of buffer 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); -- cgit v1.2.3 From 57914905f3ff2212a949e7191d52d9994c2c6215 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Tue, 3 Oct 2023 16:01:42 +0300 Subject: kernel.h: Move lib/cmdline.c prototypes to string.h The lib/cmdline.c is basically a set of some small string parsers which are wide used in the kernel. Their prototypes belong to the string.h rather then kernel.h. Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20231003130142.2936503-1-andriy.shevchenko@linux.intel.com Signed-off-by: Kees Cook --- include/linux/kernel.h | 6 ------ include/linux/string.h | 8 ++++++++ 2 files changed, 8 insertions(+), 6 deletions(-) (limited to 'include/linux') diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 86dd8939c2cd..d718fbec72dd 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -165,12 +165,6 @@ static inline void might_fault(void) { } void do_exit(long error_code) __noreturn; -extern int get_option(char **str, int *pint); -extern char *get_options(const char *str, int nints, int *ints); -extern unsigned long long memparse(const char *ptr, char **retptr); -extern bool parse_option_str(const char *str, const char *option); -extern char *next_arg(char *args, char **param, char **val); - extern int core_kernel_text(unsigned long addr); extern int __kernel_text_address(unsigned long addr); extern int kernel_text_address(unsigned long addr); diff --git a/include/linux/string.h b/include/linux/string.h index 96e6b1af86b5..adf3b3eb0ab7 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -286,9 +286,17 @@ extern void *kmemdup(const void *src, size_t len, gfp_t gfp) __realloc_size(2); extern void *kvmemdup(const void *src, size_t len, gfp_t gfp) __realloc_size(2); extern char *kmemdup_nul(const char *s, size_t len, gfp_t gfp); +/* lib/argv_split.c */ extern char **argv_split(gfp_t gfp, const char *str, int *argcp); extern void argv_free(char **argv); +/* lib/cmdline.c */ +extern int get_option(char **str, int *pint); +extern char *get_options(const char *str, int nints, int *ints); +extern unsigned long long memparse(const char *ptr, char **retptr); +extern bool parse_option_str(const char *str, const char *option); +extern char *next_arg(char *args, char **param, char **val); + extern bool sysfs_streq(const char *s1, const char *s2); int match_string(const char * const *array, size_t n, const char *string); int __sysfs_match_string(const char * const *array, size_t n, const char *s); -- cgit v1.2.3 From c5e6d3d85efa7451590edd94725b4b280e2fd8a3 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 28 Feb 2024 22:41:31 +0200 Subject: overflow: Use POD in check_shl_overflow() The check_shl_overflow() uses u64 type that is defined in types.h. Instead of including that header, just switch to use POD type directly. Signed-off-by: Andy Shevchenko Acked-by: Kees Cook Link: https://lore.kernel.org/r/20240228204919.3680786-2-andriy.shevchenko@linux.intel.com Signed-off-by: Kees Cook --- include/linux/overflow.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/linux') diff --git a/include/linux/overflow.h b/include/linux/overflow.h index dede374832c9..bc390f026128 100644 --- a/include/linux/overflow.h +++ b/include/linux/overflow.h @@ -197,7 +197,7 @@ static inline bool __must_check __must_check_overflow(bool overflow) typeof(a) _a = a; \ typeof(s) _s = s; \ typeof(d) _d = d; \ - u64 _a_full = _a; \ + unsigned long long _a_full = _a; \ unsigned int _to_shift = \ is_non_negative(_s) && _s < 8 * sizeof(*d) ? _s : 0; \ *_d = (_a_full << _to_shift); \ -- cgit v1.2.3 From f0b7f8ade9d2532a7d7da40eb297570d48dd2147 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 29 Feb 2024 22:52:30 +0200 Subject: lib/string_helpers: Add flags param to string_get_size() The new flags parameter allows controlling - Whether or not the units suffix is separated by a space, for compatibility with sort -h - Whether or not to append a B suffix - we're not always printing bytes. Co-developed-by: Kent Overstreet Signed-off-by: Kent Overstreet Signed-off-by: Andy Shevchenko Reviewed-by: Kent Overstreet Link: https://lore.kernel.org/r/20240229205345.93902-1-andriy.shevchenko@linux.intel.com Signed-off-by: Kees Cook --- include/linux/string_helpers.h | 10 +++++-- lib/string_helpers.c | 29 ++++++++++--------- lib/test-string_helpers.c | 63 ++++++++++++++++++++++++++++++++++++------ 3 files changed, 77 insertions(+), 25 deletions(-) (limited to 'include/linux') diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h index 58fb1f90eda5..e93fbb5b0c01 100644 --- a/include/linux/string_helpers.h +++ b/include/linux/string_helpers.h @@ -17,14 +17,18 @@ static inline bool string_is_terminated(const char *s, int len) return memchr(s, '\0', len) ? true : false; } -/* Descriptions of the types of units to - * print in */ +/* Descriptions of the types of units to print in */ enum string_size_units { STRING_UNITS_10, /* use powers of 10^3 (standard SI) */ STRING_UNITS_2, /* use binary powers of 2^10 */ + STRING_UNITS_MASK = BIT(0), + + /* Modifiers */ + STRING_UNITS_NO_SPACE = BIT(30), + STRING_UNITS_NO_BYTES = BIT(31), }; -int string_get_size(u64 size, u64 blk_size, enum string_size_units units, +int string_get_size(u64 size, u64 blk_size, const enum string_size_units units, char *buf, int len); int parse_int_array_user(const char __user *from, size_t count, int **array); diff --git a/lib/string_helpers.c b/lib/string_helpers.c index 6bbafd6a10d9..69ba49b853c7 100644 --- a/lib/string_helpers.c +++ b/lib/string_helpers.c @@ -25,7 +25,7 @@ * string_get_size - get the size in the specified units * @size: The size to be converted in blocks * @blk_size: Size of the block (use 1 for size in bytes) - * @units: units to use (powers of 1000 or 1024) + * @units: Units to use (powers of 1000 or 1024), whether to include space separator * @buf: buffer to format to * @len: length of buffer * @@ -39,11 +39,12 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units, char *buf, int len) { + enum string_size_units units_base = units & STRING_UNITS_MASK; static const char *const units_10[] = { - "B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB" + "", "k", "M", "G", "T", "P", "E", "Z", "Y", }; static const char *const units_2[] = { - "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB" + "", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei", "Zi", "Yi", }; static const char *const *const units_str[] = { [STRING_UNITS_10] = units_10, @@ -68,7 +69,7 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units, /* This is Napier's algorithm. Reduce the original block size to * - * coefficient * divisor[units]^i + * coefficient * divisor[units_base]^i * * we do the reduction so both coefficients are just under 32 bits so * that multiplying them together won't overflow 64 bits and we keep @@ -78,12 +79,12 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units, * precision is in the coefficients. */ while (blk_size >> 32) { - do_div(blk_size, divisor[units]); + do_div(blk_size, divisor[units_base]); i++; } while (size >> 32) { - do_div(size, divisor[units]); + do_div(size, divisor[units_base]); i++; } @@ -92,8 +93,8 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units, size *= blk_size; /* and logarithmically reduce it until it's just under the divisor */ - while (size >= divisor[units]) { - remainder = do_div(size, divisor[units]); + while (size >= divisor[units_base]) { + remainder = do_div(size, divisor[units_base]); i++; } @@ -103,10 +104,10 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units, for (j = 0; sf_cap*10 < 1000; j++) sf_cap *= 10; - if (units == STRING_UNITS_2) { + if (units_base == STRING_UNITS_2) { /* express the remainder as a decimal. It's currently the * numerator of a fraction whose denominator is - * divisor[units], which is 1 << 10 for STRING_UNITS_2 */ + * divisor[units_base], which is 1 << 10 for STRING_UNITS_2 */ remainder *= 1000; remainder >>= 10; } @@ -128,10 +129,12 @@ int string_get_size(u64 size, u64 blk_size, const enum string_size_units units, if (i >= ARRAY_SIZE(units_2)) unit = "UNK"; else - unit = units_str[units][i]; + unit = units_str[units_base][i]; - return snprintf(buf, len, "%u%s %s", (u32)size, - tmp, unit); + return snprintf(buf, len, "%u%s%s%s%s", (u32)size, tmp, + (units & STRING_UNITS_NO_SPACE) ? "" : " ", + unit, + (units & STRING_UNITS_NO_BYTES) ? "" : "B"); } EXPORT_SYMBOL(string_get_size); diff --git a/lib/test-string_helpers.c b/lib/test-string_helpers.c index 9a68849a5d55..dce67698297b 100644 --- a/lib/test-string_helpers.c +++ b/lib/test-string_helpers.c @@ -3,6 +3,7 @@ */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include #include #include #include @@ -500,21 +501,65 @@ static __init void test_string_get_size_check(const char *units, pr_warn("expected: '%s', got '%s'\n", exp, res); } -static __init void __test_string_get_size(const u64 size, const u64 blk_size, - const char *exp_result10, - const char *exp_result2) +static __init void __strchrcut(char *dst, const char *src, const char *cut) +{ + const char *from = src; + size_t len; + + do { + len = strcspn(from, cut); + memcpy(dst, from, len); + dst += len; + from += len; + } while (*from++); + *dst = '\0'; +} + +static __init void __test_string_get_size_one(const u64 size, const u64 blk_size, + const char *exp_result10, + const char *exp_result2, + enum string_size_units units, + const char *cut) { char buf10[string_get_size_maxbuf]; char buf2[string_get_size_maxbuf]; + char exp10[string_get_size_maxbuf]; + char exp2[string_get_size_maxbuf]; + char prefix10[64]; + char prefix2[64]; + + sprintf(prefix10, "STRING_UNITS_10 [%s]", cut); + sprintf(prefix2, "STRING_UNITS_2 [%s]", cut); + + __strchrcut(exp10, exp_result10, cut); + __strchrcut(exp2, exp_result2, cut); - string_get_size(size, blk_size, STRING_UNITS_10, buf10, sizeof(buf10)); - string_get_size(size, blk_size, STRING_UNITS_2, buf2, sizeof(buf2)); + string_get_size(size, blk_size, STRING_UNITS_10 | units, buf10, sizeof(buf10)); + string_get_size(size, blk_size, STRING_UNITS_2 | units, buf2, sizeof(buf2)); - test_string_get_size_check("STRING_UNITS_10", exp_result10, buf10, - size, blk_size); + test_string_get_size_check(prefix10, exp10, buf10, size, blk_size); + test_string_get_size_check(prefix2, exp2, buf2, size, blk_size); +} + +static __init void __test_string_get_size(const u64 size, const u64 blk_size, + const char *exp_result10, + const char *exp_result2) +{ + struct { + enum string_size_units units; + const char *cut; + } get_size_test_cases[] = { + { 0, "" }, + { STRING_UNITS_NO_SPACE, " " }, + { STRING_UNITS_NO_SPACE | STRING_UNITS_NO_BYTES, " B" }, + { STRING_UNITS_NO_BYTES, "B" }, + }; + int i; - test_string_get_size_check("STRING_UNITS_2", exp_result2, buf2, - size, blk_size); + for (i = 0; i < ARRAY_SIZE(get_size_test_cases); i++) + __test_string_get_size_one(size, blk_size, exp_result10, exp_result2, + get_size_test_cases[i].units, + get_size_test_cases[i].cut); } static __init void test_string_get_size(void) -- cgit v1.2.3 From bd1ebf2467f9c5d157bec7b025e83f8ffdae1318 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 29 Feb 2024 22:22:26 -0800 Subject: overflow: Allow non-type arg to type_max() and type_min() A common use of type_max() is to find the max for the type of a variable. Using the pattern type_max(typeof(var)) is needlessly verbose. Instead, since typeof(type) == type we can just explicitly call typeof() on the argument to type_max() and type_min(). Add wrappers for readability. We can do some replacements right away: $ git grep '\btype_\(min\|max\)(typeof' | wc -l 11 Link: https://lore.kernel.org/r/20240301062221.work.840-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/overflow.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'include/linux') diff --git a/include/linux/overflow.h b/include/linux/overflow.h index bc390f026128..aa691f2119b0 100644 --- a/include/linux/overflow.h +++ b/include/linux/overflow.h @@ -31,8 +31,10 @@ * credit to Christian Biere. */ #define __type_half_max(type) ((type)1 << (8*sizeof(type) - 1 - is_signed_type(type))) -#define type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T))) -#define type_min(T) ((T)((T)-type_max(T)-(T)1)) +#define __type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T))) +#define type_max(t) __type_max(typeof(t)) +#define __type_min(T) ((T)((T)-type_max(T)-(T)1)) +#define type_min(t) __type_min(typeof(t)) /* * Avoids triggering -Wtype-limits compilation warning, @@ -207,10 +209,10 @@ static inline bool __must_check __must_check_overflow(bool overflow) #define __overflows_type_constexpr(x, T) ( \ is_unsigned_type(typeof(x)) ? \ - (x) > type_max(typeof(T)) : \ + (x) > type_max(T) : \ is_unsigned_type(typeof(T)) ? \ - (x) < 0 || (x) > type_max(typeof(T)) : \ - (x) < type_min(typeof(T)) || (x) > type_max(typeof(T))) + (x) < 0 || (x) > type_max(T) : \ + (x) < type_min(T) || (x) > type_max(T)) #define __overflows_type(x, T) ({ \ typeof(T) v = 0; \ -- cgit v1.2.3 From c3b9a398fb0dae67f91e7ae4bb492e04ac2c80c0 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 29 Feb 2024 20:44:37 -0800 Subject: compiler.h: Explain how __is_constexpr() works The __is_constexpr() macro is dark magic. Shed some light on it with a comment to explain how and why it works. Acked-by: Gustavo A. R. Silva Reviewed-by: Jani Nikula Link: https://lore.kernel.org/r/20240301044428.work.411-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/compiler.h | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) (limited to 'include/linux') diff --git a/include/linux/compiler.h b/include/linux/compiler.h index bb1339c7057b..b688ad992127 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -231,6 +231,45 @@ static inline void *offset_to_ptr(const int *off) * This returns a constant expression while determining if an argument is * a constant expression, most importantly without evaluating the argument. * Glory to Martin Uecker + * + * Details: + * - sizeof() return an integer constant expression, and does not evaluate + * the value of its operand; it only examines the type of its operand. + * - The results of comparing two integer constant expressions is also + * an integer constant expression. + * - The first literal "8" isn't important. It could be any literal value. + * - The second literal "8" is to avoid warnings about unaligned pointers; + * this could otherwise just be "1". + * - (long)(x) is used to avoid warnings about 64-bit types on 32-bit + * architectures. + * - The C Standard defines "null pointer constant", "(void *)0", as + * distinct from other void pointers. + * - If (x) is an integer constant expression, then the "* 0l" resolves + * it into an integer constant expression of value 0. Since it is cast to + * "void *", this makes the second operand a null pointer constant. + * - If (x) is not an integer constant expression, then the second operand + * resolves to a void pointer (but not a null pointer constant: the value + * is not an integer constant 0). + * - The conditional operator's third operand, "(int *)8", is an object + * pointer (to type "int"). + * - The behavior (including the return type) of the conditional operator + * ("operand1 ? operand2 : operand3") depends on the kind of expressions + * given for the second and third operands. This is the central mechanism + * of the macro: + * - When one operand is a null pointer constant (i.e. when x is an integer + * constant expression) and the other is an object pointer (i.e. our + * third operand), the conditional operator returns the type of the + * object pointer operand (i.e. "int *). Here, within the sizeof(), we + * would then get: + * sizeof(*((int *)(...)) == sizeof(int) == 4 + * - When one operand is a void pointer (i.e. when x is not an integer + * constant expression) and the other is an object pointer (i.e. our + * third operand), the conditional operator returns a "void *" type. + * Here, within the sizeof(), we would then get: + * sizeof(*((void *)(...)) == sizeof(void) == 1 + * - The equality comparison to "sizeof(int)" therefore depends on (x): + * sizeof(int) == sizeof(int) (x) was a constant expression + * sizeof(int) != sizeof(void) (x) was not a constant expression */ #define __is_constexpr(x) \ (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8))) -- cgit v1.2.3