From 9ba8ec8ee67a00eb5631364e4b716f35559724d4 Mon Sep 17 00:00:00 2001 From: "H. Peter Anvin" Date: Fri, 2 Feb 2024 03:51:43 +0000 Subject: x86/boot: Add error_putdec() helper Add a helper to print decimal numbers to early console. Suggested-by: Borislav Petkov (AMD) Signed-off-by: H. Peter Anvin (Intel) Signed-off-by: Jun'ichi Nomura Signed-off-by: Borislav Petkov (AMD) Link: https://lore.kernel.org/lkml/20240123112624.GBZa-iYP1l9SSYtr-V@fat_crate.local/ Link: https://lore.kernel.org/r/20240202035052.17963-1-junichi.nomura@nec.com --- arch/x86/boot/compressed/misc.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) (limited to 'arch/x86/boot/compressed/misc.c') diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c index b99e08e6815b..bf2aac4f195e 100644 --- a/arch/x86/boot/compressed/misc.c +++ b/arch/x86/boot/compressed/misc.c @@ -164,21 +164,34 @@ void __putstr(const char *s) outb(0xff & (pos >> 1), vidport+1); } -void __puthex(unsigned long value) +static noinline void __putnum(unsigned long value, unsigned int base, + int mindig) { - char alpha[2] = "0"; - int bits; + char buf[8*sizeof(value)+1]; + char *p; - for (bits = sizeof(value) * 8 - 4; bits >= 0; bits -= 4) { - unsigned long digit = (value >> bits) & 0xf; + p = buf + sizeof(buf); + *--p = '\0'; - if (digit < 0xA) - alpha[0] = '0' + digit; - else - alpha[0] = 'a' + (digit - 0xA); + while (mindig-- > 0 || value) { + unsigned char digit = value % base; + digit += (digit >= 10) ? ('a'-10) : '0'; + *--p = digit; - __putstr(alpha); + value /= base; } + + __putstr(p); +} + +void __puthex(unsigned long value) +{ + __putnum(value, 16, sizeof(value)*2); +} + +void __putdec(unsigned long value) +{ + __putnum(value, 10, 1); } #ifdef CONFIG_X86_NEED_RELOCS -- cgit v1.2.3 From ac456ca0af4fe9630cf84e7efd20b7f7bf596aab Mon Sep 17 00:00:00 2001 From: "NOMURA JUNICHI(野村 淳一)" Date: Fri, 2 Feb 2024 03:51:58 +0000 Subject: x86/boot: Add a message about ignored early NMIs Commit 78a509fba9c9 ("x86/boot: Ignore NMIs during very early boot") added an empty handler in early boot stage to avoid boot failure due to spurious NMIs. Add a diagnostic message to show that early NMIs have occurred. [ bp: Touchups. ] [ Committer note: tested by stopping the guest really early and injecting NMIs through qemu's monitor. Result: early console in setup code Spurious early NMIs ignored: 13 ... ] Suggested-by: Borislav Petkov Suggested-by: H. Peter Anvin Signed-off-by: Jun'ichi Nomura Signed-off-by: Borislav Petkov (AMD) Acked-by: Kirill A. Shutemov Link: https://lore.kernel.org/lkml/20231130103339.GCZWhlA196uRklTMNF@fat_crate.local --- arch/x86/boot/compressed/ident_map_64.c | 2 +- arch/x86/boot/compressed/misc.c | 7 +++++++ arch/x86/boot/compressed/misc.h | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) (limited to 'arch/x86/boot/compressed/misc.c') diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c index d040080d7edb..4a029baa5147 100644 --- a/arch/x86/boot/compressed/ident_map_64.c +++ b/arch/x86/boot/compressed/ident_map_64.c @@ -389,5 +389,5 @@ void do_boot_page_fault(struct pt_regs *regs, unsigned long error_code) void do_boot_nmi_trap(struct pt_regs *regs, unsigned long error_code) { - /* Empty handler to ignore NMI during early boot */ + spurious_nmi_count++; } diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c index bf2aac4f195e..bd6857a9f15a 100644 --- a/arch/x86/boot/compressed/misc.c +++ b/arch/x86/boot/compressed/misc.c @@ -52,6 +52,7 @@ struct port_io_ops pio_ops; memptr free_mem_ptr; memptr free_mem_end_ptr; +int spurious_nmi_count; static char *vidmem; static int vidport; @@ -506,6 +507,12 @@ asmlinkage __visible void *extract_kernel(void *rmode, unsigned char *output) /* Disable exception handling before booting the kernel */ cleanup_exception_handling(); + if (spurious_nmi_count) { + error_putstr("Spurious early NMIs ignored: "); + error_putdec(spurious_nmi_count); + error_putstr("\n"); + } + return output + entry_offset; } diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h index 6502bc69d1b8..b353a7be380c 100644 --- a/arch/x86/boot/compressed/misc.h +++ b/arch/x86/boot/compressed/misc.h @@ -59,6 +59,7 @@ extern char _head[], _end[]; /* misc.c */ extern memptr free_mem_ptr; extern memptr free_mem_end_ptr; +extern int spurious_nmi_count; void *malloc(int size); void free(void *where); void __putstr(const char *s); -- 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 'arch/x86/boot/compressed/misc.c') 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 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 'arch/x86/boot/compressed/misc.c') 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 cd0d9d92c8bb46e77de62efd7df13069ddd61e7d Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Tue, 27 Feb 2024 16:19:14 +0100 Subject: x86/boot: Move mem_encrypt= parsing to the decompressor The early SME/SEV code parses the command line very early, in order to decide whether or not memory encryption should be enabled, which needs to occur even before the initial page tables are created. This is problematic for a number of reasons: - this early code runs from the 1:1 mapping provided by the decompressor or firmware, which uses a different translation than the one assumed by the linker, and so the code needs to be built in a special way; - parsing external input while the entire kernel image is still mapped writable is a bad idea in general, and really does not belong in security minded code; - the current code ignores the built-in command line entirely (although this appears to be the case for the entire decompressor) Given that the decompressor/EFI stub is an intrinsic part of the x86 bootable kernel image, move the command line parsing there and out of the core kernel. This removes the need to build lib/cmdline.o in a special way, or to use RIP-relative LEA instructions in inline asm blocks. This involves a new xloadflag in the setup header to indicate that mem_encrypt=on appeared on the kernel command line. Signed-off-by: Ard Biesheuvel Signed-off-by: Borislav Petkov (AMD) Tested-by: Tom Lendacky Link: https://lore.kernel.org/r/20240227151907.387873-17-ardb+git@google.com --- arch/x86/boot/compressed/misc.c | 15 +++++++++++++++ arch/x86/include/uapi/asm/bootparam.h | 1 + arch/x86/lib/Makefile | 13 ------------- arch/x86/mm/mem_encrypt_identity.c | 32 +++----------------------------- drivers/firmware/efi/libstub/x86-stub.c | 3 +++ 5 files changed, 22 insertions(+), 42 deletions(-) (limited to 'arch/x86/boot/compressed/misc.c') diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c index bd6857a9f15a..408507e305be 100644 --- a/arch/x86/boot/compressed/misc.c +++ b/arch/x86/boot/compressed/misc.c @@ -371,6 +371,19 @@ unsigned long decompress_kernel(unsigned char *outbuf, unsigned long virt_addr, return entry; } +/* + * Set the memory encryption xloadflag based on the mem_encrypt= command line + * parameter, if provided. + */ +static void parse_mem_encrypt(struct setup_header *hdr) +{ + int on = cmdline_find_option_bool("mem_encrypt=on"); + int off = cmdline_find_option_bool("mem_encrypt=off"); + + if (on > off) + hdr->xloadflags |= XLF_MEM_ENCRYPTION; +} + /* * The compressed kernel image (ZO), has been moved so that its position * is against the end of the buffer used to hold the uncompressed kernel @@ -401,6 +414,8 @@ asmlinkage __visible void *extract_kernel(void *rmode, unsigned char *output) /* Clear flags intended for solely in-kernel use. */ boot_params_ptr->hdr.loadflags &= ~KASLR_FLAG; + parse_mem_encrypt(&boot_params_ptr->hdr); + sanitize_boot_params(boot_params_ptr); if (boot_params_ptr->screen_info.orig_video_mode == 7) { diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h index 01d19fc22346..eeea058cf602 100644 --- a/arch/x86/include/uapi/asm/bootparam.h +++ b/arch/x86/include/uapi/asm/bootparam.h @@ -38,6 +38,7 @@ #define XLF_EFI_KEXEC (1<<4) #define XLF_5LEVEL (1<<5) #define XLF_5LEVEL_ENABLED (1<<6) +#define XLF_MEM_ENCRYPTION (1<<7) #ifndef __ASSEMBLY__ diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile index ea3a28e7b613..f0dae4fb6d07 100644 --- a/arch/x86/lib/Makefile +++ b/arch/x86/lib/Makefile @@ -14,19 +14,6 @@ ifdef CONFIG_KCSAN CFLAGS_REMOVE_delay.o = $(CC_FLAGS_FTRACE) endif -# Early boot use of cmdline; don't instrument it -ifdef CONFIG_AMD_MEM_ENCRYPT -KCOV_INSTRUMENT_cmdline.o := n -KASAN_SANITIZE_cmdline.o := n -KCSAN_SANITIZE_cmdline.o := n - -ifdef CONFIG_FUNCTION_TRACER -CFLAGS_REMOVE_cmdline.o = -pg -endif - -CFLAGS_cmdline.o := -fno-stack-protector -fno-jump-tables -endif - inat_tables_script = $(srctree)/arch/x86/tools/gen-insn-attr-x86.awk inat_tables_maps = $(srctree)/arch/x86/lib/x86-opcode-map.txt quiet_cmd_inat_tables = GEN $@ diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c index 0166ab1780cc..d210c7fc8fa2 100644 --- a/arch/x86/mm/mem_encrypt_identity.c +++ b/arch/x86/mm/mem_encrypt_identity.c @@ -43,7 +43,6 @@ #include #include -#include #include #include @@ -95,9 +94,6 @@ struct sme_populate_pgd_data { */ static char sme_workarea[2 * PMD_SIZE] __section(".init.scratch"); -static char sme_cmdline_arg[] __initdata = "mem_encrypt"; -static char sme_cmdline_on[] __initdata = "on"; - static void __init sme_clear_pgd(struct sme_populate_pgd_data *ppd) { unsigned long pgd_start, pgd_end, pgd_size; @@ -504,11 +500,9 @@ void __init sme_encrypt_kernel(struct boot_params *bp) void __init sme_enable(struct boot_params *bp) { - const char *cmdline_ptr, *cmdline_arg, *cmdline_on; unsigned int eax, ebx, ecx, edx; unsigned long feature_mask; unsigned long me_mask; - char buffer[16]; bool snp; u64 msr; @@ -551,6 +545,9 @@ void __init sme_enable(struct boot_params *bp) /* Check if memory encryption is enabled */ if (feature_mask == AMD_SME_BIT) { + if (!(bp->hdr.xloadflags & XLF_MEM_ENCRYPTION)) + return; + /* * No SME if Hypervisor bit is set. This check is here to * prevent a guest from trying to enable SME. For running as a @@ -570,31 +567,8 @@ void __init sme_enable(struct boot_params *bp) msr = __rdmsr(MSR_AMD64_SYSCFG); if (!(msr & MSR_AMD64_SYSCFG_MEM_ENCRYPT)) return; - } else { - /* SEV state cannot be controlled by a command line option */ - goto out; } - /* - * Fixups have not been applied to phys_base yet and we're running - * identity mapped, so we must obtain the address to the SME command - * line argument data using rip-relative addressing. - */ - asm ("lea sme_cmdline_arg(%%rip), %0" - : "=r" (cmdline_arg) - : "p" (sme_cmdline_arg)); - asm ("lea sme_cmdline_on(%%rip), %0" - : "=r" (cmdline_on) - : "p" (sme_cmdline_on)); - - cmdline_ptr = (const char *)((u64)bp->hdr.cmd_line_ptr | - ((u64)bp->ext_cmd_line_ptr << 32)); - - if (cmdline_find_option(cmdline_ptr, cmdline_arg, buffer, sizeof(buffer)) < 0 || - strncmp(buffer, cmdline_on, sizeof(buffer))) - return; - -out: RIP_REL_REF(sme_me_mask) = me_mask; physical_mask &= ~me_mask; cc_vendor = CC_VENDOR_AMD; diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c index 99429bc4b0c7..0336ed175e67 100644 --- a/drivers/firmware/efi/libstub/x86-stub.c +++ b/drivers/firmware/efi/libstub/x86-stub.c @@ -884,6 +884,9 @@ void __noreturn efi_stub_entry(efi_handle_t handle, } } + if (efi_mem_encrypt > 0) + hdr->xloadflags |= XLF_MEM_ENCRYPTION; + status = efi_decompress_kernel(&kernel_entry); if (status != EFI_SUCCESS) { efi_err("Failed to decompress kernel\n"); -- cgit v1.2.3 From 9c55461040a9264b7e44444c53d26480b438eda6 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Thu, 25 Jan 2024 14:32:07 +0100 Subject: x86/efistub: Remap kernel text read-only before dropping NX attribute Currently, the EFI stub invokes the EFI memory attributes protocol to strip any NX restrictions from the entire loaded kernel, resulting in all code and data being mapped read-write-execute. The point of the EFI memory attributes protocol is to remove the need for all memory allocations to be mapped with both write and execute permissions by default, and make it the OS loader's responsibility to transition data mappings to code mappings where appropriate. Even though the UEFI specification does not appear to leave room for denying memory attribute changes based on security policy, let's be cautious and avoid relying on the ability to create read-write-execute mappings. This is trivially achievable, given that the amount of kernel code executing via the firmware's 1:1 mapping is rather small and limited to the .head.text region. So let's drop the NX restrictions only on that subregion, but not before remapping it as read-only first. Signed-off-by: Ard Biesheuvel --- arch/x86/boot/compressed/Makefile | 2 +- arch/x86/boot/compressed/misc.c | 1 + arch/x86/include/asm/boot.h | 1 + drivers/firmware/efi/libstub/x86-stub.c | 11 ++++++++++- 4 files changed, 13 insertions(+), 2 deletions(-) (limited to 'arch/x86/boot/compressed/misc.c') diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index f19c038409aa..e9522c6893be 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -84,7 +84,7 @@ LDFLAGS_vmlinux += -T hostprogs := mkpiggy HOST_EXTRACFLAGS += -I$(srctree)/tools/include -sed-voffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW] \(_text\|__bss_start\|_end\)$$/\#define VO_\2 _AC(0x\1,UL)/p' +sed-voffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW] \(_text\|__start_rodata\|__bss_start\|_end\)$$/\#define VO_\2 _AC(0x\1,UL)/p' quiet_cmd_voffset = VOFFSET $@ cmd_voffset = $(NM) $< | sed -n $(sed-voffset) > $@ diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c index b99e08e6815b..044e9add713d 100644 --- a/arch/x86/boot/compressed/misc.c +++ b/arch/x86/boot/compressed/misc.c @@ -330,6 +330,7 @@ static size_t parse_elf(void *output) return ehdr.e_entry - LOAD_PHYSICAL_ADDR; } +const unsigned long kernel_text_size = VO___start_rodata - VO__text; const unsigned long kernel_total_size = VO__end - VO__text; static u8 boot_heap[BOOT_HEAP_SIZE] __aligned(4); diff --git a/arch/x86/include/asm/boot.h b/arch/x86/include/asm/boot.h index a38cc0afc90a..a3e0be0470a4 100644 --- a/arch/x86/include/asm/boot.h +++ b/arch/x86/include/asm/boot.h @@ -81,6 +81,7 @@ #ifndef __ASSEMBLY__ extern unsigned int output_len; +extern const unsigned long kernel_text_size; extern const unsigned long kernel_total_size; unsigned long decompress_kernel(unsigned char *outbuf, unsigned long virt_addr, diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c index d09aa13c7ff0..35413c8dfc25 100644 --- a/drivers/firmware/efi/libstub/x86-stub.c +++ b/drivers/firmware/efi/libstub/x86-stub.c @@ -236,6 +236,15 @@ efi_status_t efi_adjust_memory_range_protection(unsigned long start, rounded_end = roundup(start + size, EFI_PAGE_SIZE); if (memattr != NULL) { + status = efi_call_proto(memattr, set_memory_attributes, + rounded_start, + rounded_end - rounded_start, + EFI_MEMORY_RO); + if (status != EFI_SUCCESS) { + efi_warn("Failed to set EFI_MEMORY_RO attribute\n"); + return status; + } + status = efi_call_proto(memattr, clear_memory_attributes, rounded_start, rounded_end - rounded_start, @@ -812,7 +821,7 @@ static efi_status_t efi_decompress_kernel(unsigned long *kernel_entry) *kernel_entry = addr + entry; - return efi_adjust_memory_range_protection(addr, kernel_total_size); + return efi_adjust_memory_range_protection(addr, kernel_text_size); } static void __noreturn enter_kernel(unsigned long kernel_addr, -- cgit v1.2.3