| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20210831144114.154-11-alexandr.lobakin@intel.com>
Date: Tue, 31 Aug 2021 16:41:02 +0200
From: Alexander Lobakin <alexandr.lobakin@...el.com>
To: linux-hardening@...r.kernel.org
Cc: "Kristen C Accardi" <kristen.c.accardi@...el.com>,
Kristen Carlson Accardi <kristen@...ux.intel.com>,
Kees Cook <keescook@...omium.org>,
Masahiro Yamada <masahiroy@...nel.org>,
"H. Peter Anvin" <hpa@...or.com>, Jessica Yu <jeyu@...nel.org>,
Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <ndesaulniers@...gle.com>,
Marios Pomonis <pomonis@...gle.com>,
Sami Tolvanen <samitolvanen@...gle.com>,
Tony Luck <tony.luck@...el.com>,
Ard Biesheuvel <ardb@...nel.org>,
Jesse Brandeburg <jesse.brandeburg@...el.com>,
Lukasz Czapnik <lukasz.czapnik@...el.com>,
"Marta A Plantykow" <marta.a.plantykow@...el.com>,
Michal Kubiak <michal.kubiak@...el.com>,
Michal Swiatkowski <michal.swiatkowski@...el.com>,
Alexander Lobakin <alexandr.lobakin@...el.com>,
linux-kbuild@...r.kernel.org, linux-arch@...r.kernel.org,
linux-kernel@...r.kernel.org, clang-built-linux@...glegroups.com
Subject: [PATCH v6 kspp-next 10/22] x86/boot/compressed: Avoid duplicate malloc() implementations
From: Kees Cook <keescook@...omium.org>
The preboot 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 preboot environment uses 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 happens, the existing usage pattern happened to be 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()
Adding FGKASLR, however, 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@...omium.org>
Signed-off-by: Kristen Carlson Accardi <kristen@...ux.intel.com>
Signed-off-by: Alexander Lobakin <alexandr.lobakin@...el.com>
---
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 e36690778497..7d94f95c84dd 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 743f13ea25c1..a4339cb2d247 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 31139256859f..1a2e422dc357 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;
+extern void *malloc(int size);
+extern 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 868e9eacd69e..9192986b1a73 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)
--
2.31.1
Powered by blists - more mailing lists