The following commit has been merged into the x86/core branch of tip: Commit-ID: 33f98a9798f55fd77c36ce79d8cfa5329e55a789 Gitweb: https://git.kernel.org/tip/33f98a9798f55fd77c36ce79d8cfa5329e55a789 Author: Kees Cook <keescook@xxxxxxxxxxxx> AuthorDate: Wed, 13 Oct 2021 10:57:41 -07:00 Committer: Peter Zijlstra <peterz@xxxxxxxxxxxxx> CommitterDate: Wed, 27 Oct 2021 11:07:59 +02:00 x86/boot/compressed: Avoid duplicate malloc() implementations The early malloc() and free() implementation in include/linux/decompress/mm.h (which is also included by the static decompressors) is static. This is fine when the only thing interested in using malloc() is the decompression code, but the x86 early boot environment may use malloc() in a couple places, leading to a potential collision when the static copies of the available memory region ("malloc_ptr") gets reset to the global "free_mem_ptr" value. As it happened, the existing usage pattern was accidentally safe because each user did 1 malloc() and 1 free() before returning and were not nested: extract_kernel() (misc.c) choose_random_location() (kaslr.c) mem_avoid_init() handle_mem_options() malloc() ... free() ... parse_elf() (misc.c) malloc() ... free() Once the future FGKASLR series is added, however, it will insert additional malloc() calls local to fgkaslr.c in the middle of parse_elf()'s malloc()/free() pair: parse_elf() (misc.c) malloc() if (...) { layout_randomized_image(output, &ehdr, phdrs); malloc() <- boom ... else layout_image(output, &ehdr, phdrs); free() To avoid collisions, there must be a single implementation of malloc(). Adjust include/linux/decompress/mm.h so that visibility can be controlled, provide prototypes in misc.h, and implement the functions in misc.c. This also results in a small size savings: $ size vmlinux.before vmlinux.after text data bss dec hex filename 8842314 468 178320 9021102 89a6ae vmlinux.before 8842240 468 178320 9021028 89a664 vmlinux.after Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx> Link: https://lore.kernel.org/r/20211013175742.1197608-4-keescook@xxxxxxxxxxxx --- arch/x86/boot/compressed/kaslr.c | 4 ---- arch/x86/boot/compressed/misc.c | 3 +++ arch/x86/boot/compressed/misc.h | 2 ++ include/linux/decompress/mm.h | 12 ++++++++++-- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 67c3208..411b268 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -32,10 +32,6 @@ #include <generated/utsrelease.h> #include <asm/efi.h> -/* Macros used by the included decompressor code below. */ -#define STATIC -#include <linux/decompress/mm.h> - #define _SETUP #include <asm/setup.h> /* For COMMAND_LINE_SIZE */ #undef _SETUP diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c index 743f13e..a4339cb 100644 --- a/arch/x86/boot/compressed/misc.c +++ b/arch/x86/boot/compressed/misc.c @@ -28,6 +28,9 @@ /* Macros used by the included decompressor code below. */ #define STATIC static +/* Define an externally visible malloc()/free(). */ +#define MALLOC_VISIBLE +#include <linux/decompress/mm.h> /* * Provide definitions of memzero and memmove as some of the decompressors will diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h index 3113925..975ef4a 100644 --- a/arch/x86/boot/compressed/misc.h +++ b/arch/x86/boot/compressed/misc.h @@ -44,6 +44,8 @@ extern char _head[], _end[]; /* misc.c */ extern memptr free_mem_ptr; extern memptr free_mem_end_ptr; +void *malloc(int size); +void free(void *where); extern struct boot_params *boot_params; void __putstr(const char *s); void __puthex(unsigned long value); diff --git a/include/linux/decompress/mm.h b/include/linux/decompress/mm.h index 868e9ea..9192986 100644 --- a/include/linux/decompress/mm.h +++ b/include/linux/decompress/mm.h @@ -25,13 +25,21 @@ #define STATIC_RW_DATA static #endif +/* + * When an architecture needs to share the malloc()/free() implementation + * between compilation units, it needs to have non-local visibility. + */ +#ifndef MALLOC_VISIBLE +#define MALLOC_VISIBLE static +#endif + /* A trivial malloc implementation, adapted from * malloc by Hannu Savolainen 1993 and Matthias Urlichs 1994 */ STATIC_RW_DATA unsigned long malloc_ptr; STATIC_RW_DATA int malloc_count; -static void *malloc(int size) +MALLOC_VISIBLE void *malloc(int size) { void *p; @@ -52,7 +60,7 @@ static void *malloc(int size) return p; } -static void free(void *where) +MALLOC_VISIBLE void free(void *where) { malloc_count--; if (!malloc_count)
![]() |