All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] x86/boot/KASLR: Code bug fix about kernel virtual address randomization
@ 2017-06-27 12:39 Baoquan He
  2017-06-27 12:39 ` [PATCH v2 1/2] x86/boot/KASLR: Add checking for the offset of " Baoquan He
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Baoquan He @ 2017-06-27 12:39 UTC (permalink / raw)
  To: mingo, linux-kernel; +Cc: tglx, hpa, Baoquan He

People complained that crashkernel high doesn't work when kaslr code
compiled in but add 'nokaslr' to diable it. Kexec has the same
phenomenon.

The root cause is a code bug which assigned the original loading address
of kernel to the local variable 'virt_addr' which represents the offset
of kernel virtual address randmoization. As we know, kernel can be loaded
to anywhere under 64T physically, this wrong assignment could cause kernel
relocation handling of x86 64 error if no kaslr is taken.

The v1 post can be found here:
  x86/boot/KASLR: Skip relocation handling in no kaslr case
  https://patchwork.kernel.org/patch/9807789/

In v2, Ingo suggested that we should add a judgement to check if 'virt_addr'
is randomized to make kernel beyond the kernel mapping area. This checking
can let us know the error but not reset to firmware quietly as it does now.

Baoquan He (2):
  x86/boot/KASLR: Add checking for the offset of kernel virtual address
    randomization
  x86/boot/KASLR: Fix the wrong assignment to 'virt_addr'

 arch/x86/boot/compressed/kaslr.c | 3 ---
 arch/x86/boot/compressed/misc.c  | 6 ++++--
 arch/x86/boot/compressed/misc.h  | 2 --
 3 files changed, 4 insertions(+), 7 deletions(-)

-- 
2.5.5

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] x86/boot/KASLR: Add checking for the offset of kernel virtual address randomization
  2017-06-27 12:39 [PATCH v2 0/2] x86/boot/KASLR: Code bug fix about kernel virtual address randomization Baoquan He
@ 2017-06-27 12:39 ` Baoquan He
  2017-06-30 13:07   ` [tip:x86/urgent] " tip-bot for Baoquan He
  2017-06-27 12:39 ` [PATCH v2 2/2] x86/boot/KASLR: Fix the wrong assignment to 'virt_addr' Baoquan He
  2017-06-30  6:14 ` [PATCH v2 0/2] x86/boot/KASLR: Code bug fix about kernel virtual address randomization Dave Young
  2 siblings, 1 reply; 6+ messages in thread
From: Baoquan He @ 2017-06-27 12:39 UTC (permalink / raw)
  To: mingo, linux-kernel; +Cc: tglx, hpa, Baoquan He

For kernel text KASLR, the virtual address is confined to area of 1G,
[0xffffffff80000000, 0xffffffffc0000000). For the implemenataion of
virtual address randomization, we only randomize to get an offset
between 16M and 1G, then add this offset to the starting address,
0xffffffff80000000. Here 16M is the offset which is decided at linking
stage. So the amount of the local variable 'virt_addr' which respresents
the offset plus the kernel output size can not exceed KERNEL_IMAGE_SIZE.

Add a judgement to check the offset. If out of bounds, print error
message and hang there.

Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Baoquan He <bhe@redhat.com>
---
 arch/x86/boot/compressed/misc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index b3c5a5f030ce..6008fa9b74d9 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -390,6 +390,8 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
 #ifdef CONFIG_X86_64
 	if (heap > 0x3fffffffffffUL)
 		error("Destination address too large");
+	if (virt_addr + max(output_len, kernel_total_size) > KERNEL_IMAGE_SIZE)
+		error("Destination virtual address is beyond the kernel mapping area");
 #else
 	if (heap > ((-__PAGE_OFFSET-(128<<20)-1) & 0x7fffffff))
 		error("Destination address too large");
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] x86/boot/KASLR: Fix the wrong assignment to 'virt_addr'
  2017-06-27 12:39 [PATCH v2 0/2] x86/boot/KASLR: Code bug fix about kernel virtual address randomization Baoquan He
  2017-06-27 12:39 ` [PATCH v2 1/2] x86/boot/KASLR: Add checking for the offset of " Baoquan He
@ 2017-06-27 12:39 ` Baoquan He
  2017-06-30 13:08   ` [tip:x86/urgent] x86/boot/KASLR: Fix kexec crash due to 'virt_addr' calculation bug tip-bot for Baoquan He
  2017-06-30  6:14 ` [PATCH v2 0/2] x86/boot/KASLR: Code bug fix about kernel virtual address randomization Dave Young
  2 siblings, 1 reply; 6+ messages in thread
From: Baoquan He @ 2017-06-27 12:39 UTC (permalink / raw)
  To: mingo, linux-kernel; +Cc: tglx, hpa, Baoquan He

Kernel text KASLR is separated into physical address and virtual
address randomization. And for virtual address randomization, we
only randomiza to get an offset between 16M and KERNEL_IMAGE_SIZE.
So the initial value of 'virt_addr' should be LOAD_PHYSICAL_ADDR,
but not the original kernel loading address 'output'. It's a code
bug.

The bug will cause kernel failure if kernel is loaded at a different
position than the address, 16M, which is decided at compiled time.
Kexec/kdump is such pratical case.

To fix it, just assign LOAD_PHYSICAL_ADDR to virt_addr as initial
value.

Fixes: 8391c73 ("x86/KASLR: Randomize virtual address separately")
Tested-by: Dave Young <dyoung@redhat.com>
Signed-off-by: Baoquan He <bhe@redhat.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 fe318b44f7b8..91f27ab970ef 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 6008fa9b74d9..00241c815524 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;
@@ -399,7 +399,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 1c8355eadbd1..766a5211f827 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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 0/2] x86/boot/KASLR: Code bug fix about kernel virtual address randomization
  2017-06-27 12:39 [PATCH v2 0/2] x86/boot/KASLR: Code bug fix about kernel virtual address randomization Baoquan He
  2017-06-27 12:39 ` [PATCH v2 1/2] x86/boot/KASLR: Add checking for the offset of " Baoquan He
  2017-06-27 12:39 ` [PATCH v2 2/2] x86/boot/KASLR: Fix the wrong assignment to 'virt_addr' Baoquan He
@ 2017-06-30  6:14 ` Dave Young
  2 siblings, 0 replies; 6+ messages in thread
From: Dave Young @ 2017-06-30  6:14 UTC (permalink / raw)
  To: Baoquan He; +Cc: mingo, linux-kernel, tglx, hpa

On 06/27/17 at 08:39pm, Baoquan He wrote:
> People complained that crashkernel high doesn't work when kaslr code
> compiled in but add 'nokaslr' to diable it. Kexec has the same
> phenomenon.

This is a regression, with 4.12* kernel kexec reboot fails always on
my desktop pc now without kaslr being enabled.

> 
> The root cause is a code bug which assigned the original loading address
> of kernel to the local variable 'virt_addr' which represents the offset
> of kernel virtual address randmoization. As we know, kernel can be loaded
> to anywhere under 64T physically, this wrong assignment could cause kernel
> relocation handling of x86 64 error if no kaslr is taken.
> 
> The v1 post can be found here:
>   x86/boot/KASLR: Skip relocation handling in no kaslr case
>   https://patchwork.kernel.org/patch/9807789/
> 
> In v2, Ingo suggested that we should add a judgement to check if 'virt_addr'
> is randomized to make kernel beyond the kernel mapping area. This checking
> can let us know the error but not reset to firmware quietly as it does now.
> 
> Baoquan He (2):
>   x86/boot/KASLR: Add checking for the offset of kernel virtual address
>     randomization
>   x86/boot/KASLR: Fix the wrong assignment to 'virt_addr'
> 
>  arch/x86/boot/compressed/kaslr.c | 3 ---
>  arch/x86/boot/compressed/misc.c  | 6 ++++--
>  arch/x86/boot/compressed/misc.h  | 2 --
>  3 files changed, 4 insertions(+), 7 deletions(-)
> 
> -- 
> 2.5.5
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip:x86/urgent] x86/boot/KASLR: Add checking for the offset of kernel virtual address randomization
  2017-06-27 12:39 ` [PATCH v2 1/2] x86/boot/KASLR: Add checking for the offset of " Baoquan He
@ 2017-06-30 13:07   ` tip-bot for Baoquan He
  0 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Baoquan He @ 2017-06-30 13:07 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: mingo, tglx, linux-kernel, bhe, torvalds, peterz, hpa

Commit-ID:  b892cb873ced2af57dc5a018557d128c53ed6ae0
Gitweb:     http://git.kernel.org/tip/b892cb873ced2af57dc5a018557d128c53ed6ae0
Author:     Baoquan He <bhe@redhat.com>
AuthorDate: Tue, 27 Jun 2017 20:39:05 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 30 Jun 2017 08:53:14 +0200

x86/boot/KASLR: Add checking for the offset of kernel virtual address randomization

For kernel text KASLR, the virtual address is confined to area of 1G,
[0xffffffff80000000, 0xffffffffc0000000). For the implemenataion of
virtual address randomization, we only randomize to get an offset
between 16M and 1G, then add this offset to the starting address,
0xffffffff80000000. Here 16M is the offset which is decided at linking
stage. So the amount of the local variable 'virt_addr' which respresents
the offset plus the kernel output size can not exceed KERNEL_IMAGE_SIZE.

Add a debug check for the offset. If out of bounds, print error
message and hang there.

Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Baoquan He <bhe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1498567146-11990-2-git-send-email-bhe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/boot/compressed/misc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index b3c5a5f0..6008fa9 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -390,6 +390,8 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
 #ifdef CONFIG_X86_64
 	if (heap > 0x3fffffffffffUL)
 		error("Destination address too large");
+	if (virt_addr + max(output_len, kernel_total_size) > KERNEL_IMAGE_SIZE)
+		error("Destination virtual address is beyond the kernel mapping area");
 #else
 	if (heap > ((-__PAGE_OFFSET-(128<<20)-1) & 0x7fffffff))
 		error("Destination address too large");

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [tip:x86/urgent] x86/boot/KASLR: Fix kexec crash due to 'virt_addr' calculation bug
  2017-06-27 12:39 ` [PATCH v2 2/2] x86/boot/KASLR: Fix the wrong assignment to 'virt_addr' Baoquan He
@ 2017-06-30 13:08   ` tip-bot for Baoquan He
  0 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Baoquan He @ 2017-06-30 13:08 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, linux-kernel, mingo, peterz, hpa, bhe, torvalds, dyoung

Commit-ID:  8eabf42ae5237e6b699aeac687b5b629e3537c8d
Gitweb:     http://git.kernel.org/tip/8eabf42ae5237e6b699aeac687b5b629e3537c8d
Author:     Baoquan He <bhe@redhat.com>
AuthorDate: Tue, 27 Jun 2017 20:39:06 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 30 Jun 2017 08:53:14 +0200

x86/boot/KASLR: Fix kexec crash due to 'virt_addr' calculation bug

Kernel text KASLR is separated into physical address and virtual
address randomization. And for virtual address randomization, we
only randomiza to get an offset between 16M and KERNEL_IMAGE_SIZE.
So the initial value of 'virt_addr' should be LOAD_PHYSICAL_ADDR,
but not the original kernel loading address 'output'.

The bug will cause kernel boot failure if kernel is loaded at a different
position than the address, 16M, which is decided at compiled time.
Kexec/kdump is such practical case.

To fix it, just assign LOAD_PHYSICAL_ADDR to virt_addr as initial
value.

Tested-by: Dave Young <dyoung@redhat.com>
Signed-off-by: Baoquan He <bhe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: 8391c73 ("x86/KASLR: Randomize virtual address separately")
Link: http://lkml.kernel.org/r/1498567146-11990-3-git-send-email-bhe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 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 54c24f0..56a7e92 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -564,9 +564,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 6008fa9..00241c8 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;
@@ -399,7 +399,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
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-06-30 13:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-27 12:39 [PATCH v2 0/2] x86/boot/KASLR: Code bug fix about kernel virtual address randomization Baoquan He
2017-06-27 12:39 ` [PATCH v2 1/2] x86/boot/KASLR: Add checking for the offset of " Baoquan He
2017-06-30 13:07   ` [tip:x86/urgent] " tip-bot for Baoquan He
2017-06-27 12:39 ` [PATCH v2 2/2] x86/boot/KASLR: Fix the wrong assignment to 'virt_addr' Baoquan He
2017-06-30 13:08   ` [tip:x86/urgent] x86/boot/KASLR: Fix kexec crash due to 'virt_addr' calculation bug tip-bot for Baoquan He
2017-06-30  6:14 ` [PATCH v2 0/2] x86/boot/KASLR: Code bug fix about kernel virtual address randomization Dave Young

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.