linux-kernel - [PATCH] x86/boot/KASLR: Skip relocation handling in no kaslr case

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 24 Jun 2017 22:16:33 +0800
From:   Baoquan He <bhe@...hat.com>
To:     linux-kernel@...r.kernel.org
Cc:     Baoquan He <bhe@...hat.com>
Subject: [PATCH] x86/boot/KASLR: Skip relocation handling in no kaslr case

Kdump kernel will reset to firmware after crash is trigered when
crashkernel=xxM,high is added to kernel command line. Kexec has the
same phenomenon. This only happened on system with kaslr code
compiled in and kernel option 'nokaslr'is added. Both of them works
well when kaslr is enabled.

When crashkernel high is set or kexec case, kexec/kdump kernel will be
put above 4G. Since we assign the original loading address of kernel to
virt_addr as initial value, the virt_addr will be larger than 1G if kaslr
is disabled, it exceeds the kernel mapping size which is only 1G. Then
it will cause relocation handling error in handle_relocations().

In fact this is a known issue and fixed in commit:

... f285f4a ("x86, boot: Skip relocs when load address unchanged")

But above fix was lost carelessly in later commit:

... 8391c73 ("x86/KASLR: Randomize virtual address separately")

To fix it, just assign LOAD_PHYSICAL_ADDR to virt_addr as initial value.
If no kaslr is taken, it will skip the relocation handling and jump out.

Fixes: 8391c73 ("x86/KASLR: Randomize virtual address separately")
Tested-by: Dave Young <dyoung@...hat.com>
Signed-off-by: Baoquan He <bhe@...hat.com>
"H. Peter Anvin" <hpa@...or.com>
Thomas Gleixner <tglx@...utronix.de>
Ingo Molnar <mingo@...hat.com>
x86@...nel.org
Kees Cook <keescook@...omium.org
Baoquan He <bhe@...hat.com>
Dave Jiang <dave.jiang@...el.com>
Yinghai Lu <yinghai@...nel.org>
Arnd Bergmann <arnd@...db.de>
Thomas Garnier <thgarnie@...gle.com>
---
 arch/x86/boot/compressed/kaslr.c | 3 ---
 arch/x86/boot/compressed/misc.c  | 4 ++--
 arch/x86/boot/compressed/misc.h  | 2 --
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index fe318b4..91f27ab 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -625,9 +625,6 @@ void choose_random_location(unsigned long input,
 {
 	unsigned long random_addr, min_addr;
 
-	/* By default, keep output position unchanged. */
-	*virt_addr = *output;
-
 	if (cmdline_find_option_bool("nokaslr")) {
 		warn("KASLR disabled: 'nokaslr' on cmdline.");
 		return;
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index b3c5a5f0..c945acd 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -338,7 +338,7 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
 				  unsigned long output_len)
 {
 	const unsigned long kernel_total_size = VO__end - VO__text;
-	unsigned long virt_addr = (unsigned long)output;
+	unsigned long virt_addr = LOAD_PHYSICAL_ADDR;
 
 	/* Retain x86 boot parameters pointer passed from startup_32/64. */
 	boot_params = rmode;
@@ -397,7 +397,7 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
 #ifndef CONFIG_RELOCATABLE
 	if ((unsigned long)output != LOAD_PHYSICAL_ADDR)
 		error("Destination address does not match LOAD_PHYSICAL_ADDR");
-	if ((unsigned long)output != virt_addr)
+	if (virt_addr != LOAD_PHYSICAL_ADDR)
 		error("Destination virtual address changed when not relocatable");
 #endif
 
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 1c8355e..766a521 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -81,8 +81,6 @@ static inline void choose_random_location(unsigned long input,
 					  unsigned long output_size,
 					  unsigned long *virt_addr)
 {
-	/* No change from existing output location. */
-	*virt_addr = *output;
 }
 #endif
 
-- 
2.5.5