LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] x86, boot: skip relocs when load address unchanged
@ 2015-01-16  0:51 Kees Cook
  2015-01-16 11:16 ` Thomas D.
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Kees Cook @ 2015-01-16  0:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, Vivek Goyal,
	Jan Beulich, Junjie Mao, Andi Kleen, Baoquan He, Thomas D.

On 64-bit, relocation is not required unless the load address gets
changed. Without this, relocations do unexpected things when the kernel
is above 4G.

Reported-by: Baoquan He <bhe@redhat.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
---
This is a reimplementation of Baoquan's "kaslr: check if kernel location is
changed", which performs the check without needing to change the function
declaration. This should have exactly the same effect, but I dropped Vivek's
Ack and Thomas's Test, since it's technically a different patch.
---
 arch/x86/boot/compressed/misc.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index dcc1c536cc21..a950864a64da 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -373,6 +373,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
 				  unsigned long output_len,
 				  unsigned long run_size)
 {
+	unsigned char *output_orig = output;
+
 	real_mode = rmode;
 
 	sanitize_boot_params(real_mode);
@@ -421,7 +423,12 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
 	debug_putstr("\nDecompressing Linux... ");
 	decompress(input_data, input_len, NULL, NULL, output, NULL, error);
 	parse_elf(output);
-	handle_relocations(output, output_len);
+	/*
+	 * 32-bit always performs relocations. 64-bit relocations are only
+	 * needed if kASLR has chosen a different load address.
+	 */
+	if (!IS_ENABLED(CONFIG_X86_64) || output != output_orig)
+		handle_relocations(output, output_len);
 	debug_putstr("done.\nBooting the kernel.\n");
 	return output;
 }
-- 
1.9.1


-- 
Kees Cook
Chrome OS Security

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

* Re: [PATCH] x86, boot: skip relocs when load address unchanged
  2015-01-16  0:51 [PATCH] x86, boot: skip relocs when load address unchanged Kees Cook
@ 2015-01-16 11:16 ` Thomas D.
  2015-01-20 11:42 ` [tip:x86/urgent] x86, boot: Skip " tip-bot for Kees Cook
  2015-01-20 13:04 ` [PATCH] x86, boot: skip " Baoquan He
  2 siblings, 0 replies; 7+ messages in thread
From: Thomas D. @ 2015-01-16 11:16 UTC (permalink / raw)
  To: Kees Cook, linux-kernel
  Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, Vivek Goyal,
	Jan Beulich, Junjie Mao, Andi Kleen, Baoquan He

Hi,

Kees Cook wrote:
> This is a reimplementation of Baoquan's "kaslr: check if kernel location is
> changed", which performs the check without needing to change the function
> declaration. This should have exactly the same effect, but I dropped Vivek's
> Ack and Thomas's Test, since it's technically a different patch.

Your patch works for me, too.

Tested with linux-3.19-rc4 on a 64-bit system.

Thanks!


-Thomas



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

* [tip:x86/urgent] x86, boot: Skip relocs when load address unchanged
  2015-01-16  0:51 [PATCH] x86, boot: skip relocs when load address unchanged Kees Cook
  2015-01-16 11:16 ` Thomas D.
@ 2015-01-20 11:42 ` tip-bot for Kees Cook
  2015-01-20 13:04 ` [PATCH] x86, boot: skip " Baoquan He
  2 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Kees Cook @ 2015-01-20 11:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: whissi, bhe, vgoyal, keescook, hpa, JBeulich, linux-kernel,
	eternal.n08, ak, mingo, tglx

Commit-ID:  f285f4a21c3253887caceed493089ece17579d59
Gitweb:     http://git.kernel.org/tip/f285f4a21c3253887caceed493089ece17579d59
Author:     Kees Cook <keescook@chromium.org>
AuthorDate: Thu, 15 Jan 2015 16:51:46 -0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 20 Jan 2015 12:37:23 +0100

x86, boot: Skip relocs when load address unchanged

On 64-bit, relocation is not required unless the load address gets
changed. Without this, relocations do unexpected things when the kernel
is above 4G.

Reported-by: Baoquan He <bhe@redhat.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Tested-by: Thomas D. <whissi@whissi.de>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Junjie Mao <eternal.n08@gmail.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/20150116005146.GA4212@www.outflux.net
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/boot/compressed/misc.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index dcc1c53..a950864 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -373,6 +373,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
 				  unsigned long output_len,
 				  unsigned long run_size)
 {
+	unsigned char *output_orig = output;
+
 	real_mode = rmode;
 
 	sanitize_boot_params(real_mode);
@@ -421,7 +423,12 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
 	debug_putstr("\nDecompressing Linux... ");
 	decompress(input_data, input_len, NULL, NULL, output, NULL, error);
 	parse_elf(output);
-	handle_relocations(output, output_len);
+	/*
+	 * 32-bit always performs relocations. 64-bit relocations are only
+	 * needed if kASLR has chosen a different load address.
+	 */
+	if (!IS_ENABLED(CONFIG_X86_64) || output != output_orig)
+		handle_relocations(output, output_len);
 	debug_putstr("done.\nBooting the kernel.\n");
 	return output;
 }

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

* Re: [PATCH] x86, boot: skip relocs when load address unchanged
  2015-01-16  0:51 [PATCH] x86, boot: skip relocs when load address unchanged Kees Cook
  2015-01-16 11:16 ` Thomas D.
  2015-01-20 11:42 ` [tip:x86/urgent] x86, boot: Skip " tip-bot for Kees Cook
@ 2015-01-20 13:04 ` Baoquan He
  2015-02-26  6:29   ` MegaBrutal
       [not found]   ` <CAE8gLh=9S-FMcekB43nM6RaAxEHsPkU0PK_=wNNbMT9YzkZiXw@mail.gmail.com>
  2 siblings, 2 replies; 7+ messages in thread
From: Baoquan He @ 2015-01-20 13:04 UTC (permalink / raw)
  To: Kees Cook
  Cc: linux-kernel, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86,
	Vivek Goyal, Jan Beulich, Junjie Mao, Andi Kleen, Thomas D.

On 01/15/15 at 04:51pm, Kees Cook wrote:
> On 64-bit, relocation is not required unless the load address gets
> changed. Without this, relocations do unexpected things when the kernel
> is above 4G.

This patch works for me. And good to see it's being merged. About the
patch log, I would say relocations do unexpected things when the kernel
is above 1G since randomization is done from 16M to 1G, namely
CONFIG_RANDOMIZE_BASE_MAX_OFFSET. So above 1G kernel text mapping will
step into kernel module mapping region.

BTW, I am working on separate randomization of kernel physical and virtual
address , will post it. But it won't conflict with this because I don't
think it can be accepted in a short time. Before that this patch truly
fix the kexec/kdump bug when kaslr is compiled in.

Thanks
Baoquan

> 
> Reported-by: Baoquan He <bhe@redhat.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: stable@vger.kernel.org
> ---
> This is a reimplementation of Baoquan's "kaslr: check if kernel location is
> changed", which performs the check without needing to change the function
> declaration. This should have exactly the same effect, but I dropped Vivek's
> Ack and Thomas's Test, since it's technically a different patch.
> ---
>  arch/x86/boot/compressed/misc.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> index dcc1c536cc21..a950864a64da 100644
> --- a/arch/x86/boot/compressed/misc.c
> +++ b/arch/x86/boot/compressed/misc.c
> @@ -373,6 +373,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
>  				  unsigned long output_len,
>  				  unsigned long run_size)
>  {
> +	unsigned char *output_orig = output;
> +
>  	real_mode = rmode;
>  
>  	sanitize_boot_params(real_mode);
> @@ -421,7 +423,12 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
>  	debug_putstr("\nDecompressing Linux... ");
>  	decompress(input_data, input_len, NULL, NULL, output, NULL, error);
>  	parse_elf(output);
> -	handle_relocations(output, output_len);
> +	/*
> +	 * 32-bit always performs relocations. 64-bit relocations are only
> +	 * needed if kASLR has chosen a different load address.
> +	 */
> +	if (!IS_ENABLED(CONFIG_X86_64) || output != output_orig)
> +		handle_relocations(output, output_len);
>  	debug_putstr("done.\nBooting the kernel.\n");
>  	return output;
>  }
> -- 
> 1.9.1
> 
> 
> -- 
> Kees Cook
> Chrome OS Security

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

* Re: [PATCH] x86, boot: skip relocs when load address unchanged
  2015-01-20 13:04 ` [PATCH] x86, boot: skip " Baoquan He
@ 2015-02-26  6:29   ` MegaBrutal
  2015-02-26  6:45     ` Baoquan He
       [not found]   ` <CAE8gLh=9S-FMcekB43nM6RaAxEHsPkU0PK_=wNNbMT9YzkZiXw@mail.gmail.com>
  1 sibling, 1 reply; 7+ messages in thread
From: MegaBrutal @ 2015-02-26  6:29 UTC (permalink / raw)
  To: Baoquan He
  Cc: Kees Cook, Linux kernel, H. Peter Anvin, Thomas Gleixner,
	Ingo Molnar, x86, Vivek Goyal, Jan Beulich, Junjie Mao,
	Andi Kleen, Thomas D.

Thanks for this patch, and good to see it in mainline!

This actually fixes the problem I reported here:
https://lkml.org/lkml/2014/12/1/15

I wish it to be backported into the Ubuntu Utopic kernel asap.

> This patch works for me. And good to see it's being merged. About the
> patch log, I would say relocations do unexpected things when the kernel
> is above 1G since randomization is done from 16M to 1G, namely
> CONFIG_RANDOMIZE_BASE_MAX_OFFSET. So above 1G kernel text mapping will
> step into kernel module mapping region.

I'm just speculating, but is it the reason why I only get problems
when I boot with kexec? Maybe it's only then when the kernel gets
above 1G. Otherwise, the kernels boot properly when they are booted
from GRUB.


2015-01-20 14:04 GMT+01:00 Baoquan He <bhe@redhat.com>:
>
> On 01/15/15 at 04:51pm, Kees Cook wrote:
> > On 64-bit, relocation is not required unless the load address gets
> > changed. Without this, relocations do unexpected things when the kernel
> > is above 4G.
>
> This patch works for me. And good to see it's being merged. About the
> patch log, I would say relocations do unexpected things when the kernel
> is above 1G since randomization is done from 16M to 1G, namely
> CONFIG_RANDOMIZE_BASE_MAX_OFFSET. So above 1G kernel text mapping will
> step into kernel module mapping region.
>
> BTW, I am working on separate randomization of kernel physical and virtual
> address , will post it. But it won't conflict with this because I don't
> think it can be accepted in a short time. Before that this patch truly
> fix the kexec/kdump bug when kaslr is compiled in.
>
> Thanks
> Baoquan
>
> >
> > Reported-by: Baoquan He <bhe@redhat.com>
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > Cc: stable@vger.kernel.org
> > ---
> > This is a reimplementation of Baoquan's "kaslr: check if kernel location is
> > changed", which performs the check without needing to change the function
> > declaration. This should have exactly the same effect, but I dropped Vivek's
> > Ack and Thomas's Test, since it's technically a different patch.
> > ---
> >  arch/x86/boot/compressed/misc.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> > index dcc1c536cc21..a950864a64da 100644
> > --- a/arch/x86/boot/compressed/misc.c
> > +++ b/arch/x86/boot/compressed/misc.c
> > @@ -373,6 +373,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
> >                                 unsigned long output_len,
> >                                 unsigned long run_size)
> >  {
> > +     unsigned char *output_orig = output;
> > +
> >       real_mode = rmode;
> >
> >       sanitize_boot_params(real_mode);
> > @@ -421,7 +423,12 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
> >       debug_putstr("\nDecompressing Linux... ");
> >       decompress(input_data, input_len, NULL, NULL, output, NULL, error);
> >       parse_elf(output);
> > -     handle_relocations(output, output_len);
> > +     /*
> > +      * 32-bit always performs relocations. 64-bit relocations are only
> > +      * needed if kASLR has chosen a different load address.
> > +      */
> > +     if (!IS_ENABLED(CONFIG_X86_64) || output != output_orig)
> > +             handle_relocations(output, output_len);
> >       debug_putstr("done.\nBooting the kernel.\n");
> >       return output;
> >  }
> > --
> > 1.9.1
> >
> >
> > --
> > Kees Cook
> > Chrome OS Security
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

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

* Re: [PATCH] x86, boot: skip relocs when load address unchanged
  2015-02-26  6:29   ` MegaBrutal
@ 2015-02-26  6:45     ` Baoquan He
  0 siblings, 0 replies; 7+ messages in thread
From: Baoquan He @ 2015-02-26  6:45 UTC (permalink / raw)
  To: MegaBrutal
  Cc: Kees Cook, Linux kernel, H. Peter Anvin, Thomas Gleixner,
	Ingo Molnar, x86, Vivek Goyal, Jan Beulich, Junjie Mao,
	Andi Kleen, Thomas D.

On 02/26/15 at 07:29am, MegaBrutal wrote:
> Thanks for this patch, and good to see it in mainline!
> 
> This actually fixes the problem I reported here:
> https://lkml.org/lkml/2014/12/1/15
> 
> I wish it to be backported into the Ubuntu Utopic kernel asap.
> 
> > This patch works for me. And good to see it's being merged. About the
> > patch log, I would say relocations do unexpected things when the kernel
> > is above 1G since randomization is done from 16M to 1G, namely
> > CONFIG_RANDOMIZE_BASE_MAX_OFFSET. So above 1G kernel text mapping will
> > step into kernel module mapping region.
> 
> I'm just speculating, but is it the reason why I only get problems
> when I boot with kexec? Maybe it's only then when the kernel gets
> above 1G. Otherwise, the kernels boot properly when they are booted
> from GRUB.

Yeah, kexec loads kernel at the top of physical memory. And above 1G
kernel mapping will step into module mapping area and collapse system.
Grub doesn't incur this problem.

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

* Re: [PATCH] x86, boot: skip relocs when load address unchanged
       [not found]   ` <CAE8gLh=9S-FMcekB43nM6RaAxEHsPkU0PK_=wNNbMT9YzkZiXw@mail.gmail.com>
@ 2015-02-26 11:50     ` Ingo Molnar
  0 siblings, 0 replies; 7+ messages in thread
From: Ingo Molnar @ 2015-02-26 11:50 UTC (permalink / raw)
  To: MegaBrutal
  Cc: Baoquan He, Kees Cook, Linux kernel, H. Peter Anvin,
	Thomas Gleixner, Ingo Molnar, x86, Vivek Goyal, Jan Beulich,
	Junjie Mao, Andi Kleen, Thomas D.


* MegaBrutal <megabrutal@gmail.com> wrote:

> Thanks for this patch, and good to see it in mainline!
> 
> This actually fixes the problem I reported here:
> https://lkml.org/lkml/2014/12/1/15
> 
> I wish it to be backported into the Ubuntu Utopic kernel asap.

We marked the commit Cc: stable, so it ought to be picked 
up by the stable kernel and then it should show up in 
Ubuntu if they refresh their kernel package to the latest 
stable kernel.

Thanks,

	Ingo

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

end of thread, other threads:[~2015-02-26 11:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-16  0:51 [PATCH] x86, boot: skip relocs when load address unchanged Kees Cook
2015-01-16 11:16 ` Thomas D.
2015-01-20 11:42 ` [tip:x86/urgent] x86, boot: Skip " tip-bot for Kees Cook
2015-01-20 13:04 ` [PATCH] x86, boot: skip " Baoquan He
2015-02-26  6:29   ` MegaBrutal
2015-02-26  6:45     ` Baoquan He
     [not found]   ` <CAE8gLh=9S-FMcekB43nM6RaAxEHsPkU0PK_=wNNbMT9YzkZiXw@mail.gmail.com>
2015-02-26 11:50     ` Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).