* [Patch v2 1/2] kaslr: check if kernel location is changed
@ 2014-09-12 6:20 Baoquan He
2014-09-12 6:20 ` [Patch v2 2/2] export the kernel image size KERNEL_IMAGE_SIZE Baoquan He
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Baoquan He @ 2014-09-12 6:20 UTC (permalink / raw)
To: linux-kernel
Cc: kexec, ebiederm, hpa, tglx, mingo, vgoyal, keescook, ak,
kumagai-atsushi, Baoquan He
Function handle_relocations() is used to do the relocations handling
for i686 and kaslr of x86_64. For 32 bit the relocation handling is
mandotary to perform. For x86_64 only when kaslr is enabled and a
random kernel location is chosen successfully the relocation handling
shound be done. However previous implementation only compared the
kernel loading address and LOAD_PHYSICAL_ADDR where kernel were
compiled to run at. This would casue system to hang when kernel
loading address is not equal to LOAD_PHYSICAL_ADDR.
So in this patch check if kernel location is changed after
choose_kernel_location() when x86_64. If and only if in x86_64
and kernel location is changed, we say a kaslr random kernel
location is chosen, then the relocation handling need be done.
Signed-off-by: Baoquan He <bhe@redhat.com>
---
arch/x86/boot/compressed/misc.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 57ab74d..3bb2a17 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -230,8 +230,9 @@ static void error(char *x)
asm("hlt");
}
-#if CONFIG_X86_NEED_RELOCS
-static void handle_relocations(void *output, unsigned long output_len)
+#ifdef CONFIG_X86_NEED_RELOCS
+static void handle_relocations(void *output_orig, void *output,
+ unsigned long output_len)
{
int *reloc;
unsigned long delta, map, ptr;
@@ -239,6 +240,20 @@ static void handle_relocations(void *output, unsigned long output_len)
unsigned long max_addr = min_addr + output_len;
/*
+ * 32bit always requires relocations to be performed. For x86_64,
+ * relocations need to be performed only if kaslr has chosen a
+ * different load address then kernel was originally loaded at.
+ *
+ * If we are here, either kaslr is not configured in or kaslr is disabled
+ * or kaslr has chosen not to change the load location of kernel. Don't
+ * perform any relocations.
+ */
+#if CONFIG_X86_64
+ if (output_orig == output)
+ return;
+#endif
+
+ /*
* Calculate the delta between where vmlinux was linked to load
* and where it was actually loaded.
*/
@@ -299,7 +314,8 @@ static void handle_relocations(void *output, unsigned long output_len)
#endif
}
#else
-static inline void handle_relocations(void *output, unsigned long output_len)
+static inline void handle_relocations(void *output_orig, void *output,
+ unsigned long output_len)
{ }
#endif
@@ -360,6 +376,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
unsigned char *output,
unsigned long output_len)
{
+ unsigned char *output_orig = output;
+
real_mode = rmode;
sanitize_boot_params(real_mode);
@@ -402,7 +420,7 @@ 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);
+ handle_relocations(output_orig, output, output_len);
debug_putstr("done.\nBooting the kernel.\n");
return output;
}
--
1.8.5.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Patch v2 2/2] export the kernel image size KERNEL_IMAGE_SIZE
2014-09-12 6:20 [Patch v2 1/2] kaslr: check if kernel location is changed Baoquan He
@ 2014-09-12 6:20 ` Baoquan He
2014-09-12 12:04 ` [Patch v2 1/2] kaslr: check if kernel location is changed Vivek Goyal
2014-09-12 15:22 ` [Patch v3 " Baoquan He
2 siblings, 0 replies; 9+ messages in thread
From: Baoquan He @ 2014-09-12 6:20 UTC (permalink / raw)
To: linux-kernel
Cc: kexec, ebiederm, hpa, tglx, mingo, vgoyal, keescook, ak,
kumagai-atsushi, Baoquan He
Now kaslr makes kernel image size changable, not the fixed size 512M.
So KERNEL_IMAGE_SIZE need be exported to VMCOREINFO, otherwise makedumfile
will crash.
Signed-off-by: Baoquan He <bhe@redhat.com>
Acked-by: Kees Cook <keescook@chromium.org>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
---
kernel/kexec.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 2bee072..bd680d3 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -2003,6 +2003,9 @@ static int __init crash_save_vmcoreinfo_init(void)
#endif
VMCOREINFO_NUMBER(PG_head_mask);
VMCOREINFO_NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE);
+#ifdef CONFIG_X86
+ VMCOREINFO_NUMBER(KERNEL_IMAGE_SIZE);
+#endif
#ifdef CONFIG_HUGETLBFS
VMCOREINFO_SYMBOL(free_huge_page);
#endif
--
1.8.5.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Patch v2 1/2] kaslr: check if kernel location is changed
2014-09-12 6:20 [Patch v2 1/2] kaslr: check if kernel location is changed Baoquan He
2014-09-12 6:20 ` [Patch v2 2/2] export the kernel image size KERNEL_IMAGE_SIZE Baoquan He
@ 2014-09-12 12:04 ` Vivek Goyal
2014-09-12 13:41 ` Baoquan He
2014-09-12 15:22 ` [Patch v3 " Baoquan He
2 siblings, 1 reply; 9+ messages in thread
From: Vivek Goyal @ 2014-09-12 12:04 UTC (permalink / raw)
To: Baoquan He
Cc: linux-kernel, kexec, ebiederm, hpa, tglx, mingo, keescook, ak,
kumagai-atsushi
On Fri, Sep 12, 2014 at 02:20:31PM +0800, Baoquan He wrote:
> Function handle_relocations() is used to do the relocations handling
> for i686 and kaslr of x86_64. For 32 bit the relocation handling is
> mandotary to perform. For x86_64 only when kaslr is enabled and a
> random kernel location is chosen successfully the relocation handling
> shound be done. However previous implementation only compared the
> kernel loading address and LOAD_PHYSICAL_ADDR where kernel were
> compiled to run at.
> This would casue system to hang when kernel
> loading address is not equal to LOAD_PHYSICAL_ADDR.
I don't think above is correct. It will not always fails. It will fail
only in few conditions like when delta between load address and compiled
address is bigger than what 32bit signed relocations can handle.
Also there will be limitations that delta can't be too big otherwise
kernel text virtual addresses will overflow in module address space.
Thanks
Vivek
>
> So in this patch check if kernel location is changed after
> choose_kernel_location() when x86_64. If and only if in x86_64
> and kernel location is changed, we say a kaslr random kernel
> location is chosen, then the relocation handling need be done.
>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
> arch/x86/boot/compressed/misc.c | 26 ++++++++++++++++++++++----
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> index 57ab74d..3bb2a17 100644
> --- a/arch/x86/boot/compressed/misc.c
> +++ b/arch/x86/boot/compressed/misc.c
> @@ -230,8 +230,9 @@ static void error(char *x)
> asm("hlt");
> }
>
> -#if CONFIG_X86_NEED_RELOCS
> -static void handle_relocations(void *output, unsigned long output_len)
> +#ifdef CONFIG_X86_NEED_RELOCS
> +static void handle_relocations(void *output_orig, void *output,
> + unsigned long output_len)
> {
> int *reloc;
> unsigned long delta, map, ptr;
> @@ -239,6 +240,20 @@ static void handle_relocations(void *output, unsigned long output_len)
> unsigned long max_addr = min_addr + output_len;
>
> /*
> + * 32bit always requires relocations to be performed. For x86_64,
> + * relocations need to be performed only if kaslr has chosen a
> + * different load address then kernel was originally loaded at.
> + *
> + * If we are here, either kaslr is not configured in or kaslr is disabled
> + * or kaslr has chosen not to change the load location of kernel. Don't
> + * perform any relocations.
> + */
> +#if CONFIG_X86_64
> + if (output_orig == output)
> + return;
> +#endif
> +
> + /*
> * Calculate the delta between where vmlinux was linked to load
> * and where it was actually loaded.
> */
> @@ -299,7 +314,8 @@ static void handle_relocations(void *output, unsigned long output_len)
> #endif
> }
> #else
> -static inline void handle_relocations(void *output, unsigned long output_len)
> +static inline void handle_relocations(void *output_orig, void *output,
> + unsigned long output_len)
> { }
> #endif
>
> @@ -360,6 +376,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
> unsigned char *output,
> unsigned long output_len)
> {
> + unsigned char *output_orig = output;
> +
> real_mode = rmode;
>
> sanitize_boot_params(real_mode);
> @@ -402,7 +420,7 @@ 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);
> + handle_relocations(output_orig, output, output_len);
> debug_putstr("done.\nBooting the kernel.\n");
> return output;
> }
> --
> 1.8.5.3
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Patch v2 1/2] kaslr: check if kernel location is changed
2014-09-12 12:04 ` [Patch v2 1/2] kaslr: check if kernel location is changed Vivek Goyal
@ 2014-09-12 13:41 ` Baoquan He
0 siblings, 0 replies; 9+ messages in thread
From: Baoquan He @ 2014-09-12 13:41 UTC (permalink / raw)
To: Vivek Goyal
Cc: linux-kernel, kexec, ebiederm, hpa, tglx, mingo, keescook, ak,
kumagai-atsushi
On 09/12/14 at 08:04am, Vivek Goyal wrote:
> On Fri, Sep 12, 2014 at 02:20:31PM +0800, Baoquan He wrote:
> > Function handle_relocations() is used to do the relocations handling
> > for i686 and kaslr of x86_64. For 32 bit the relocation handling is
> > mandotary to perform. For x86_64 only when kaslr is enabled and a
> > random kernel location is chosen successfully the relocation handling
> > shound be done. However previous implementation only compared the
> > kernel loading address and LOAD_PHYSICAL_ADDR where kernel were
> > compiled to run at.
>
> > This would casue system to hang when kernel
> > loading address is not equal to LOAD_PHYSICAL_ADDR.
>
> I don't think above is correct. It will not always fails. It will fail
> only in few conditions like when delta between load address and compiled
> address is bigger than what 32bit signed relocations can handle.
>
> Also there will be limitations that delta can't be too big otherwise
> kernel text virtual addresses will overflow in module address space.
Yes, will repost with changed description.
>
> Thanks
> Vivek
> >
> > So in this patch check if kernel location is changed after
> > choose_kernel_location() when x86_64. If and only if in x86_64
> > and kernel location is changed, we say a kaslr random kernel
> > location is chosen, then the relocation handling need be done.
> >
> > Signed-off-by: Baoquan He <bhe@redhat.com>
> > ---
> > arch/x86/boot/compressed/misc.c | 26 ++++++++++++++++++++++----
> > 1 file changed, 22 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> > index 57ab74d..3bb2a17 100644
> > --- a/arch/x86/boot/compressed/misc.c
> > +++ b/arch/x86/boot/compressed/misc.c
> > @@ -230,8 +230,9 @@ static void error(char *x)
> > asm("hlt");
> > }
> >
> > -#if CONFIG_X86_NEED_RELOCS
> > -static void handle_relocations(void *output, unsigned long output_len)
> > +#ifdef CONFIG_X86_NEED_RELOCS
> > +static void handle_relocations(void *output_orig, void *output,
> > + unsigned long output_len)
> > {
> > int *reloc;
> > unsigned long delta, map, ptr;
> > @@ -239,6 +240,20 @@ static void handle_relocations(void *output, unsigned long output_len)
> > unsigned long max_addr = min_addr + output_len;
> >
> > /*
> > + * 32bit always requires relocations to be performed. For x86_64,
> > + * relocations need to be performed only if kaslr has chosen a
> > + * different load address then kernel was originally loaded at.
> > + *
> > + * If we are here, either kaslr is not configured in or kaslr is disabled
> > + * or kaslr has chosen not to change the load location of kernel. Don't
> > + * perform any relocations.
> > + */
> > +#if CONFIG_X86_64
> > + if (output_orig == output)
> > + return;
> > +#endif
> > +
> > + /*
> > * Calculate the delta between where vmlinux was linked to load
> > * and where it was actually loaded.
> > */
> > @@ -299,7 +314,8 @@ static void handle_relocations(void *output, unsigned long output_len)
> > #endif
> > }
> > #else
> > -static inline void handle_relocations(void *output, unsigned long output_len)
> > +static inline void handle_relocations(void *output_orig, void *output,
> > + unsigned long output_len)
> > { }
> > #endif
> >
> > @@ -360,6 +376,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
> > unsigned char *output,
> > unsigned long output_len)
> > {
> > + unsigned char *output_orig = output;
> > +
> > real_mode = rmode;
> >
> > sanitize_boot_params(real_mode);
> > @@ -402,7 +420,7 @@ 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);
> > + handle_relocations(output_orig, output, output_len);
> > debug_putstr("done.\nBooting the kernel.\n");
> > return output;
> > }
> > --
> > 1.8.5.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Patch v3 1/2] kaslr: check if kernel location is changed
2014-09-12 6:20 [Patch v2 1/2] kaslr: check if kernel location is changed Baoquan He
2014-09-12 6:20 ` [Patch v2 2/2] export the kernel image size KERNEL_IMAGE_SIZE Baoquan He
2014-09-12 12:04 ` [Patch v2 1/2] kaslr: check if kernel location is changed Vivek Goyal
@ 2014-09-12 15:22 ` Baoquan He
2014-09-12 15:33 ` Vivek Goyal
2 siblings, 1 reply; 9+ messages in thread
From: Baoquan He @ 2014-09-12 15:22 UTC (permalink / raw)
To: linux-kernel
Cc: kexec, ebiederm, hpa, tglx, mingo, vgoyal, keescook, ak,
kumagai-atsushi
Function handle_relocations() is used to do the relocations handling
for i686 and kaslr of x86_64. For 32 bit the relocation handling is
mandotary to perform. For x86_64 only when kaslr is enabled and a
random kernel location is chosen successfully the relocation handling
shound be done. However previous implementation only compared the
kernel loading address and LOAD_PHYSICAL_ADDR where kernel were
compiled to run at. This would casue system to be exceptional in
few conditions like when delta between load address and compiled
address is bigger than what 32bit signed relocations can handle.
Also there will be limitations that delta can't be too big otherwise
kernel text virtual addresses will overflow in module address space.
So in this patch check if kernel location is changed after
choose_kernel_location() when x86_64. If and only if in x86_64
and kernel location is changed, we say a kaslr random kernel
location is chosen, then the relocation handling is needed.
Signed-off-by: Baoquan He <bhe@redhat.com>
---
arch/x86/boot/compressed/misc.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 57ab74d..3bb2a17 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -230,8 +230,9 @@ static void error(char *x)
asm("hlt");
}
-#if CONFIG_X86_NEED_RELOCS
-static void handle_relocations(void *output, unsigned long output_len)
+#ifdef CONFIG_X86_NEED_RELOCS
+static void handle_relocations(void *output_orig, void *output,
+ unsigned long output_len)
{
int *reloc;
unsigned long delta, map, ptr;
@@ -239,6 +240,20 @@ static void handle_relocations(void *output, unsigned long output_len)
unsigned long max_addr = min_addr + output_len;
/*
+ * 32bit always requires relocations to be performed. For x86_64,
+ * relocations need to be performed only if kaslr has chosen a
+ * different load address then kernel was originally loaded at.
+ *
+ * If we are here, either kaslr is not configured in or kaslr is disabled
+ * or kaslr has chosen not to change the load location of kernel. Don't
+ * perform any relocations.
+ */
+#if CONFIG_X86_64
+ if (output_orig == output)
+ return;
+#endif
+
+ /*
* Calculate the delta between where vmlinux was linked to load
* and where it was actually loaded.
*/
@@ -299,7 +314,8 @@ static void handle_relocations(void *output, unsigned long output_len)
#endif
}
#else
-static inline void handle_relocations(void *output, unsigned long output_len)
+static inline void handle_relocations(void *output_orig, void *output,
+ unsigned long output_len)
{ }
#endif
@@ -360,6 +376,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
unsigned char *output,
unsigned long output_len)
{
+ unsigned char *output_orig = output;
+
real_mode = rmode;
sanitize_boot_params(real_mode);
@@ -402,7 +420,7 @@ 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);
+ handle_relocations(output_orig, output, output_len);
debug_putstr("done.\nBooting the kernel.\n");
return output;
}
--
1.8.5.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Patch v3 1/2] kaslr: check if kernel location is changed
2014-09-12 15:22 ` [Patch v3 " Baoquan He
@ 2014-09-12 15:33 ` Vivek Goyal
2014-09-12 15:56 ` Thomas D.
2014-09-12 17:41 ` Kees Cook
0 siblings, 2 replies; 9+ messages in thread
From: Vivek Goyal @ 2014-09-12 15:33 UTC (permalink / raw)
To: Baoquan He, Thomas D.
Cc: linux-kernel, kexec, ebiederm, hpa, tglx, mingo, keescook, ak,
kumagai-atsushi
On Fri, Sep 12, 2014 at 11:22:44PM +0800, Baoquan He wrote:
> Function handle_relocations() is used to do the relocations handling
> for i686 and kaslr of x86_64. For 32 bit the relocation handling is
> mandotary to perform. For x86_64 only when kaslr is enabled and a
> random kernel location is chosen successfully the relocation handling
> shound be done. However previous implementation only compared the
> kernel loading address and LOAD_PHYSICAL_ADDR where kernel were
> compiled to run at. This would casue system to be exceptional in
> few conditions like when delta between load address and compiled
> address is bigger than what 32bit signed relocations can handle.
> Also there will be limitations that delta can't be too big otherwise
> kernel text virtual addresses will overflow in module address space.
>
> So in this patch check if kernel location is changed after
> choose_kernel_location() when x86_64. If and only if in x86_64
> and kernel location is changed, we say a kaslr random kernel
> location is chosen, then the relocation handling is needed.
>
> Signed-off-by: Baoquan He <bhe@redhat.com>
I think this patch should make kexec and kdump working with kaslr
enabled (CONFIG_RANDOMIZE_BASE=y).
In case of kdump, we will need to pass "nokaslr" to make sure kernel
does not move from kexec chosen address.
In case of kexec, I think it should be ok to not pass "nokaslr". This
case is no different than any other bootloader.
Hence.
Acked-by: Vivek Goyal <vgoyal@redhat.com>
Thomas D.,
You had reported kexec issues with CONFIG_RANDOMIZE_BASE=y. Does this
patch resolve the issue for you?
Thanks
Vivek
> ---
> arch/x86/boot/compressed/misc.c | 26 ++++++++++++++++++++++----
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> index 57ab74d..3bb2a17 100644
> --- a/arch/x86/boot/compressed/misc.c
> +++ b/arch/x86/boot/compressed/misc.c
> @@ -230,8 +230,9 @@ static void error(char *x)
> asm("hlt");
> }
>
> -#if CONFIG_X86_NEED_RELOCS
> -static void handle_relocations(void *output, unsigned long output_len)
> +#ifdef CONFIG_X86_NEED_RELOCS
> +static void handle_relocations(void *output_orig, void *output,
> + unsigned long output_len)
> {
> int *reloc;
> unsigned long delta, map, ptr;
> @@ -239,6 +240,20 @@ static void handle_relocations(void *output, unsigned long output_len)
> unsigned long max_addr = min_addr + output_len;
>
> /*
> + * 32bit always requires relocations to be performed. For x86_64,
> + * relocations need to be performed only if kaslr has chosen a
> + * different load address then kernel was originally loaded at.
> + *
> + * If we are here, either kaslr is not configured in or kaslr is disabled
> + * or kaslr has chosen not to change the load location of kernel. Don't
> + * perform any relocations.
> + */
> +#if CONFIG_X86_64
> + if (output_orig == output)
> + return;
> +#endif
> +
> + /*
> * Calculate the delta between where vmlinux was linked to load
> * and where it was actually loaded.
> */
> @@ -299,7 +314,8 @@ static void handle_relocations(void *output, unsigned long output_len)
> #endif
> }
> #else
> -static inline void handle_relocations(void *output, unsigned long output_len)
> +static inline void handle_relocations(void *output_orig, void *output,
> + unsigned long output_len)
> { }
> #endif
>
> @@ -360,6 +376,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
> unsigned char *output,
> unsigned long output_len)
> {
> + unsigned char *output_orig = output;
> +
> real_mode = rmode;
>
> sanitize_boot_params(real_mode);
> @@ -402,7 +420,7 @@ 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);
> + handle_relocations(output_orig, output, output_len);
> debug_putstr("done.\nBooting the kernel.\n");
> return output;
> }
> --
> 1.8.5.3
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Patch v3 1/2] kaslr: check if kernel location is changed
2014-09-12 15:33 ` Vivek Goyal
@ 2014-09-12 15:56 ` Thomas D.
2014-09-12 15:59 ` Vivek Goyal
2014-09-12 17:41 ` Kees Cook
1 sibling, 1 reply; 9+ messages in thread
From: Thomas D. @ 2014-09-12 15:56 UTC (permalink / raw)
To: Vivek Goyal, Baoquan He
Cc: linux-kernel@vger.kernel.org, kexec@lists.infradead.org,
ebiederm@xmission.com, hpa@zytor.com, tglx@linutronix.de,
mingo@redhat.com, keescook@chromium.org, ak@linux.intel.com,
kumagai-atsushi@mxc.nes.nec.co.jp
Hi,
Vivek Goyal wrote:
> You had reported kexec issues with CONFIG_RANDOMIZE_BASE=y. Does this
> patch resolve the issue for you?
Yup! Tested against kernel-3.16.2.
-Thomas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch v3 1/2] kaslr: check if kernel location is changed
2014-09-12 15:56 ` Thomas D.
@ 2014-09-12 15:59 ` Vivek Goyal
0 siblings, 0 replies; 9+ messages in thread
From: Vivek Goyal @ 2014-09-12 15:59 UTC (permalink / raw)
To: Thomas D.
Cc: Baoquan He, linux-kernel@vger.kernel.org,
kexec@lists.infradead.org, ebiederm@xmission.com, hpa@zytor.com,
tglx@linutronix.de, mingo@redhat.com, keescook@chromium.org,
ak@linux.intel.com, kumagai-atsushi@mxc.nes.nec.co.jp
On Fri, Sep 12, 2014 at 05:56:12PM +0200, Thomas D. wrote:
> Hi,
>
> Vivek Goyal wrote:
> > You had reported kexec issues with CONFIG_RANDOMIZE_BASE=y. Does this
> > patch resolve the issue for you?
>
> Yup! Tested against kernel-3.16.2.
Thanks. Given this patch is small and should not break anything else, I
think it might make sense to send it to stable too.
Thanks
Vivek
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch v3 1/2] kaslr: check if kernel location is changed
2014-09-12 15:33 ` Vivek Goyal
2014-09-12 15:56 ` Thomas D.
@ 2014-09-12 17:41 ` Kees Cook
1 sibling, 0 replies; 9+ messages in thread
From: Kees Cook @ 2014-09-12 17:41 UTC (permalink / raw)
To: Vivek Goyal
Cc: Baoquan He, Thomas D., LKML, Kexec Mailing List,
Eric W. Biederman, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
Andi Kleen, kumagai-atsushi
On Fri, Sep 12, 2014 at 8:33 AM, Vivek Goyal <vgoyal@redhat.com> wrote:
> On Fri, Sep 12, 2014 at 11:22:44PM +0800, Baoquan He wrote:
>> Function handle_relocations() is used to do the relocations handling
>> for i686 and kaslr of x86_64. For 32 bit the relocation handling is
>> mandotary to perform. For x86_64 only when kaslr is enabled and a
>> random kernel location is chosen successfully the relocation handling
>> shound be done. However previous implementation only compared the
>> kernel loading address and LOAD_PHYSICAL_ADDR where kernel were
>> compiled to run at. This would casue system to be exceptional in
>> few conditions like when delta between load address and compiled
>> address is bigger than what 32bit signed relocations can handle.
>> Also there will be limitations that delta can't be too big otherwise
>> kernel text virtual addresses will overflow in module address space.
>>
>> So in this patch check if kernel location is changed after
>> choose_kernel_location() when x86_64. If and only if in x86_64
>> and kernel location is changed, we say a kaslr random kernel
>> location is chosen, then the relocation handling is needed.
>>
>> Signed-off-by: Baoquan He <bhe@redhat.com>
>
> I think this patch should make kexec and kdump working with kaslr
> enabled (CONFIG_RANDOMIZE_BASE=y).
>
> In case of kdump, we will need to pass "nokaslr" to make sure kernel
> does not move from kexec chosen address.
>
> In case of kexec, I think it should be ok to not pass "nokaslr". This
> case is no different than any other bootloader.
>
> Hence.
> Acked-by: Vivek Goyal <vgoyal@redhat.com>
Yup, I think this looks fine to me too. Thanks for getting this fixed!
And as Vivek mentioned later, I think this should probably go into
-stable as well.
Acked-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
-Kees
>
> Thomas D.,
>
> You had reported kexec issues with CONFIG_RANDOMIZE_BASE=y. Does this
> patch resolve the issue for you?
>
> Thanks
> Vivek
>
>> ---
>> arch/x86/boot/compressed/misc.c | 26 ++++++++++++++++++++++----
>> 1 file changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
>> index 57ab74d..3bb2a17 100644
>> --- a/arch/x86/boot/compressed/misc.c
>> +++ b/arch/x86/boot/compressed/misc.c
>> @@ -230,8 +230,9 @@ static void error(char *x)
>> asm("hlt");
>> }
>>
>> -#if CONFIG_X86_NEED_RELOCS
>> -static void handle_relocations(void *output, unsigned long output_len)
>> +#ifdef CONFIG_X86_NEED_RELOCS
>> +static void handle_relocations(void *output_orig, void *output,
>> + unsigned long output_len)
>> {
>> int *reloc;
>> unsigned long delta, map, ptr;
>> @@ -239,6 +240,20 @@ static void handle_relocations(void *output, unsigned long output_len)
>> unsigned long max_addr = min_addr + output_len;
>>
>> /*
>> + * 32bit always requires relocations to be performed. For x86_64,
>> + * relocations need to be performed only if kaslr has chosen a
>> + * different load address then kernel was originally loaded at.
>> + *
>> + * If we are here, either kaslr is not configured in or kaslr is disabled
>> + * or kaslr has chosen not to change the load location of kernel. Don't
>> + * perform any relocations.
>> + */
>> +#if CONFIG_X86_64
>> + if (output_orig == output)
>> + return;
>> +#endif
>> +
>> + /*
>> * Calculate the delta between where vmlinux was linked to load
>> * and where it was actually loaded.
>> */
>> @@ -299,7 +314,8 @@ static void handle_relocations(void *output, unsigned long output_len)
>> #endif
>> }
>> #else
>> -static inline void handle_relocations(void *output, unsigned long output_len)
>> +static inline void handle_relocations(void *output_orig, void *output,
>> + unsigned long output_len)
>> { }
>> #endif
>>
>> @@ -360,6 +376,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
>> unsigned char *output,
>> unsigned long output_len)
>> {
>> + unsigned char *output_orig = output;
>> +
>> real_mode = rmode;
>>
>> sanitize_boot_params(real_mode);
>> @@ -402,7 +420,7 @@ 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);
>> + handle_relocations(output_orig, output, output_len);
>> debug_putstr("done.\nBooting the kernel.\n");
>> return output;
>> }
>> --
>> 1.8.5.3
>>
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-09-12 17:41 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-12 6:20 [Patch v2 1/2] kaslr: check if kernel location is changed Baoquan He
2014-09-12 6:20 ` [Patch v2 2/2] export the kernel image size KERNEL_IMAGE_SIZE Baoquan He
2014-09-12 12:04 ` [Patch v2 1/2] kaslr: check if kernel location is changed Vivek Goyal
2014-09-12 13:41 ` Baoquan He
2014-09-12 15:22 ` [Patch v3 " Baoquan He
2014-09-12 15:33 ` Vivek Goyal
2014-09-12 15:56 ` Thomas D.
2014-09-12 15:59 ` Vivek Goyal
2014-09-12 17:41 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox