Skip to contentSkip to conversation

x86/boot/KASLR: Skip relocation handling in no kaslr case

PATCH In review Baoquan He 1 participant 2 messages Jun 24, 2017 x86

lore
Share & tools
lore

Baoquan He

author
#0 lore raw

PATCH 1/1 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")

"H. Peter Anvin" <[email protected]>
Thomas Gleixner <[email protected]>
Ingo Molnar <[email protected]>
[email protected]
Kees Cook <[email protected]
Baoquan He <[email protected]>
Dave Jiang <[email protected]>
Yinghai Lu <[email protected]>
Arnd Bergmann <[email protected]>
Thomas Garnier <[email protected]>

---
 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(-)
arch/x86/boot/compressed/kaslr.c +0 -3
index fe318b4..91f27ab 100644
@@ -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;
arch/x86/boot/compressed/misc.c +2 -2
index b3c5a5f0..c945acd 100644
@@ -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
 
arch/x86/boot/compressed/misc.h +0 -2
index 1c8355e..766a521 100644
@@ -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
 
Signature · 1 line
2.5.5
author
#1 lore raw

Sorry, forgot adding Cc: before maintainers contact, NACK this patch and will repost.

Quoted 94 lines · Baoquan He

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")

"H. Peter Anvin" <[email protected]>
Thomas Gleixner <[email protected]>
Ingo Molnar <[email protected]>
[email protected]
Kees Cook <[email protected]
Baoquan He <[email protected]>
Dave Jiang <[email protected]>
Yinghai Lu <[email protected]>
Arnd Bergmann <[email protected]>
Thomas Garnier <[email protected]>

---
 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(-)
arch/x86/boot/compressed/kaslr.c +0 -3
index fe318b4..91f27ab 100644
@@ -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;
arch/x86/boot/compressed/misc.c +2 -2
index b3c5a5f0..c945acd 100644
@@ -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
 
arch/x86/boot/compressed/misc.h +0 -2
index 1c8355e..766a521 100644
@@ -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
 
Signature · 1 line
2.5.5