* [resend Patch v3 1/2] kaslr: check if kernel location is changed @ 2014-09-30 7:08 Baoquan He 2014-09-30 7:08 ` [resend Patch v3 2/2] export the kernel image size KERNEL_IMAGE_SIZE Baoquan He ` (2 more replies) 0 siblings, 3 replies; 25+ messages in thread From: Baoquan He @ 2014-09-30 7:08 UTC (permalink / raw) To: hpa Cc: linux-kernel, tglx, mingo, x86, vgoyal, keescook, ak, ebiederm, kexec, whissi, kumagai-atsushi, Baoquan He, stable 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> Acked-by: Vivek Goyal <vgoyal@redhat.com> Acked-by: Kees Cook <keescook@chromium.org> Tested-by: Thomas D. <whissi@whissi.de> Cc: stable@vger.kernel.org --- 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] 25+ messages in thread
* [resend Patch v3 2/2] export the kernel image size KERNEL_IMAGE_SIZE 2014-09-30 7:08 [resend Patch v3 1/2] kaslr: check if kernel location is changed Baoquan He @ 2014-09-30 7:08 ` Baoquan He 2015-02-02 7:32 ` Baoquan He 2014-09-30 21:21 ` [resend Patch v3 1/2] kaslr: check if kernel location is changed H. Peter Anvin 2015-01-09 2:09 ` Baoquan He 2 siblings, 1 reply; 25+ messages in thread From: Baoquan He @ 2014-09-30 7:08 UTC (permalink / raw) To: hpa Cc: linux-kernel, tglx, mingo, x86, vgoyal, keescook, ak, ebiederm, kexec, whissi, 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] 25+ messages in thread
* Re: [resend Patch v3 2/2] export the kernel image size KERNEL_IMAGE_SIZE 2014-09-30 7:08 ` [resend Patch v3 2/2] export the kernel image size KERNEL_IMAGE_SIZE Baoquan He @ 2015-02-02 7:32 ` Baoquan He 2015-02-09 20:18 ` Kees Cook 0 siblings, 1 reply; 25+ messages in thread From: Baoquan He @ 2015-02-02 7:32 UTC (permalink / raw) To: Thomas Gleixner Cc: linux-kernel, mingo, x86, vgoyal, keescook, ak, ebiederm, kexec, whissi, kumagai-atsushi, hpa Hi Thomas, Could you please also merge this patch? Since you have merged this patch "x86, boot: Skip relocs when load address unchanged [commit f285f4a21]", and this issue was raised because it broke kexec/kdump, then I posted these 2 patches. Without this patch makedumpfile will be broken when enable kaslr and do the kdump. Thanks Baouqan On 09/30/2014 03:08 PM, Baoquan He wrote: > 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 > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 2/2] export the kernel image size KERNEL_IMAGE_SIZE 2015-02-02 7:32 ` Baoquan He @ 2015-02-09 20:18 ` Kees Cook 0 siblings, 0 replies; 25+ messages in thread From: Kees Cook @ 2015-02-09 20:18 UTC (permalink / raw) To: Baoquan He Cc: Thomas Gleixner, LKML, Ingo Molnar, x86, Vivek Goyal, Andi Kleen, Eric W. Biederman, Kexec Mailing List, Thomas Deutschmann, kumagai-atsushi, H. Peter Anvin Ping? Can someone pull this into x86 tip? Thanks! -Kees On Sun, Feb 1, 2015 at 11:32 PM, Baoquan He <bhe@redhat.com> wrote: > > Hi Thomas, > > Could you please also merge this patch? Since you have merged this patch > "x86, boot: Skip relocs when load address unchanged [commit f285f4a21]", > and this issue was raised because it broke kexec/kdump, then I posted > these 2 patches. Without this patch makedumpfile will be broken when > enable kaslr and do the kdump. > > Thanks > Baouqan > > On 09/30/2014 03:08 PM, Baoquan He wrote: >> 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 >> > -- Kees Cook Chrome OS Security ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-09-30 7:08 [resend Patch v3 1/2] kaslr: check if kernel location is changed Baoquan He 2014-09-30 7:08 ` [resend Patch v3 2/2] export the kernel image size KERNEL_IMAGE_SIZE Baoquan He @ 2014-09-30 21:21 ` H. Peter Anvin 2014-10-01 13:52 ` Vivek Goyal 2014-10-08 14:40 ` Baoquan He 2015-01-09 2:09 ` Baoquan He 2 siblings, 2 replies; 25+ messages in thread From: H. Peter Anvin @ 2014-09-30 21:21 UTC (permalink / raw) To: Baoquan He Cc: linux-kernel, tglx, mingo, x86, vgoyal, keescook, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On 09/30/2014 12:08 AM, 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> > Acked-by: Vivek Goyal <vgoyal@redhat.com> > Acked-by: Kees Cook <keescook@chromium.org> > Tested-by: Thomas D. <whissi@whissi.de> > Cc: stable@vger.kernel.org Could you clarify under what conditions we may end up with 32-bit signed overflow, and yet have a functional kernel? -hpa ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-09-30 21:21 ` [resend Patch v3 1/2] kaslr: check if kernel location is changed H. Peter Anvin @ 2014-10-01 13:52 ` Vivek Goyal 2014-10-08 15:09 ` H. Peter Anvin 2014-10-08 14:40 ` Baoquan He 1 sibling, 1 reply; 25+ messages in thread From: Vivek Goyal @ 2014-10-01 13:52 UTC (permalink / raw) To: H. Peter Anvin Cc: Baoquan He, linux-kernel, tglx, mingo, x86, keescook, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On Tue, Sep 30, 2014 at 02:21:05PM -0700, H. Peter Anvin wrote: > On 09/30/2014 12:08 AM, 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> > > Acked-by: Vivek Goyal <vgoyal@redhat.com> > > Acked-by: Kees Cook <keescook@chromium.org> > > Tested-by: Thomas D. <whissi@whissi.de> > > Cc: stable@vger.kernel.org > > Could you clarify under what conditions we may end up with 32-bit signed > overflow, and yet have a functional kernel? Hi Peter, I think there is some confusion. I will try to clarify. If we have 32bit signed overflow, we will not have a functional kernel. And that's the message we get when we try to kexec with CONFIG_RANDOMIZE_BASE=y. ********************************************************************** [ 340.709078] kexec: Starting new kernel early console in decompress_kernel KASLR disabled by default... Decompressing Linux... Parsing ELF... Performing relocations... 32-bit relocation outside of kernel! -- System halted ***************************************************************** We realized that kexec tries to load kernel at higher physical addresses and that can lead to problmes. Currently for x86_64, handle_relocations() will perform relocations if kernel is not loaded at LOAD_PHYSICAL_ADDR. I think this does not work for all the cases and kerenl can not be loaded anywhere in the physical address space. And that's why we run into issues with kexec. My understanding is that we introduce handle_relcoations() i386 style because of RANDOMIZE_BASE. If that's the case, one possible solution is that perform relocations only if ranodmize base logic has chosen a different load location for kernel than where boot loader loaded it. Otherwise don't do anything. In case of kexec/kdump, we will pass "nokaslr" to second kernel forcing it to do nothing and let the kernel run where it was loaded by bootloader. And in that case handle_relocations() should not do any relocations and that should allow kernel to be loaded anywhere in physical memory on x86_64. Thanks Vivek ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-01 13:52 ` Vivek Goyal @ 2014-10-08 15:09 ` H. Peter Anvin 2014-10-08 19:27 ` Vivek Goyal 0 siblings, 1 reply; 25+ messages in thread From: H. Peter Anvin @ 2014-10-08 15:09 UTC (permalink / raw) To: Vivek Goyal Cc: Baoquan He, linux-kernel, tglx, mingo, x86, keescook, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On 10/01/2014 06:52 AM, Vivek Goyal wrote: > > Hi Peter, > > I think there is some confusion. I will try to clarify. > > If we have 32bit signed overflow, we will not have a functional kernel. > And that's the message we get when we try to kexec with > CONFIG_RANDOMIZE_BASE=y. > And how does that happen? > ********************************************************************** > [ 340.709078] kexec: Starting new kernel > early console in decompress_kernel > KASLR disabled by default... > > Decompressing Linux... Parsing ELF... > > Performing relocations... > 32-bit relocation outside of kernel! > > > -- System halted > ***************************************************************** > > We realized that kexec tries to load kernel at higher physical addresses > and that can lead to problmes. > > Currently for x86_64, handle_relocations() will perform relocations if > kernel is not loaded at LOAD_PHYSICAL_ADDR. I think this does not work for > all the cases and kerenl can not be loaded anywhere in the physical address > space. And that's why we run into issues with kexec. > > My understanding is that we introduce handle_relcoations() i386 style > because of RANDOMIZE_BASE. If that's the case, one possible solution > is that perform relocations only if ranodmize base logic has chosen > a different load location for kernel than where boot loader loaded > it. Otherwise don't do anything. > > In case of kexec/kdump, we will pass "nokaslr" to second kernel forcing > it to do nothing and let the kernel run where it was loaded by bootloader. > And in that case handle_relocations() should not do any relocations and > that should allow kernel to be loaded anywhere in physical memory on > x86_64. > Sorry... this makes no sense. For x86-64, there is no direct connection between the physical and virtual address spaces that the kernel runs in... -hpa ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-08 15:09 ` H. Peter Anvin @ 2014-10-08 19:27 ` Vivek Goyal 2014-10-11 3:14 ` Baoquan He 0 siblings, 1 reply; 25+ messages in thread From: Vivek Goyal @ 2014-10-08 19:27 UTC (permalink / raw) To: H. Peter Anvin Cc: Baoquan He, linux-kernel, tglx, mingo, x86, keescook, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On Wed, Oct 08, 2014 at 08:09:59AM -0700, H. Peter Anvin wrote: > On 10/01/2014 06:52 AM, Vivek Goyal wrote: > > > > Hi Peter, > > > > I think there is some confusion. I will try to clarify. > > > > If we have 32bit signed overflow, we will not have a functional kernel. > > And that's the message we get when we try to kexec with > > CONFIG_RANDOMIZE_BASE=y. > > > > And how does that happen? I compile a kernel for physical address 16MB (CONFIG_PHYSICAL_START=0x1000000). And kexec loads this kernel at physical address above 16GB (0x000000042e000000). When we boot into second kernel it tries to perform relocations and fails as follows. I have printed bunch of variables in handle_relocations(), so referring to code will help. min_addr=0x000000042e000000 (Physical address where decompressed kernel is loaded). delta=0x000000042d000000 (Difference between load and compile addr). map=0x00000004ad000000 (map = delta - __START_KERNEL_map) Now we start processing 32bit relocations and read first reloc. extended=0x81e819c2 (extended = *reloc) We add map to it and new value of extended is. extended=0x2ee819c2 (extended += map) Now we convert this to actual 64bit address which relocation needs to be applied and ptr value is. ptr = 0x000000002ee819c2 (ptr = (unsigned long)extended;) And this address is outside the range of where kernel is actually loaded. (0x000000042e000000). So we ended up with a wrong address to patch hence following check fails. if (ptr < min_addr || ptr > max_addr) error("32-bit relocation outside of kernel!\n"); > > > ********************************************************************** > > [ 340.709078] kexec: Starting new kernel > > early console in decompress_kernel > > KASLR disabled by default... > > > > Decompressing Linux... Parsing ELF... > > > > Performing relocations... > > 32-bit relocation outside of kernel! > > > > > > -- System halted > > ***************************************************************** > > > > We realized that kexec tries to load kernel at higher physical addresses > > and that can lead to problmes. > > > > Currently for x86_64, handle_relocations() will perform relocations if > > kernel is not loaded at LOAD_PHYSICAL_ADDR. I think this does not work for > > all the cases and kerenl can not be loaded anywhere in the physical address > > space. And that's why we run into issues with kexec. > > > > My understanding is that we introduce handle_relcoations() i386 style > > because of RANDOMIZE_BASE. If that's the case, one possible solution > > is that perform relocations only if ranodmize base logic has chosen > > a different load location for kernel than where boot loader loaded > > it. Otherwise don't do anything. > > > > In case of kexec/kdump, we will pass "nokaslr" to second kernel forcing > > it to do nothing and let the kernel run where it was loaded by bootloader. > > And in that case handle_relocations() should not do any relocations and > > that should allow kernel to be loaded anywhere in physical memory on > > x86_64. > > > > Sorry... this makes no sense. > > For x86-64, there is no direct connection between the physical and > virtual address spaces that the kernel runs in... I am sorry I did not understand this one. I thought that initial relocatable kernel implementaion did not have any direct connection between virtual and physical address. One could load kernel anywhere and kernel virtual address will not change and we will just adjust page tables to map virtual address to right physical address. Now handle_relocation() stuff seems to introduce a close coupling between physical and virtual address. So if kernel shifts by 16MB in physical address space, then it will shift by equal amount in virtual address space. So there seems to be a direct connection between virtual and physical address space in this case. What am I missing? Thanks Vivek ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-08 19:27 ` Vivek Goyal @ 2014-10-11 3:14 ` Baoquan He 2014-10-11 10:34 ` H. Peter Anvin 0 siblings, 1 reply; 25+ messages in thread From: Baoquan He @ 2014-10-11 3:14 UTC (permalink / raw) To: Vivek Goyal Cc: H. Peter Anvin, linux-kernel, tglx, mingo, x86, keescook, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On 10/08/14 at 03:27pm, Vivek Goyal wrote: > On Wed, Oct 08, 2014 at 08:09:59AM -0700, H. Peter Anvin wrote: > > Sorry... this makes no sense. > > > > For x86-64, there is no direct connection between the physical and > > virtual address spaces that the kernel runs in... > > I am sorry I did not understand this one. I thought that initial > relocatable kernel implementaion did not have any direct connection > between virtual and physical address. One could load kernel anywhere > and kernel virtual address will not change and we will just adjust > page tables to map virtual address to right physical address. > > Now handle_relocation() stuff seems to introduce a close coupling > between physical and virtual address. So if kernel shifts by 16MB > in physical address space, then it will shift by equal amount > in virtual address space. So there seems to be a direct connection > between virtual and physical address space in this case. Yeah, it's exactly as Vivek said. Before kaslr was introduced, x86_64 kernel can be put anywhere, and always _text is 0xffffffff81000000. Meanwhile phys_base contains the offset between the compiled addr (namely 0x1000000) and kernel loaded addr. After kaslr implementation was added, as long as kernel loaded addr is different 0x1000000, it will call handle_relocations(). The offset now is added onto each symbols including _text and phys_base becomes 0. It's clearly showing that by checking /proc/kallsyms and value of phys_base. Thanks Baoquan ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-11 3:14 ` Baoquan He @ 2014-10-11 10:34 ` H. Peter Anvin 2014-10-11 12:38 ` Baoquan He 2014-10-13 12:52 ` Vivek Goyal 0 siblings, 2 replies; 25+ messages in thread From: H. Peter Anvin @ 2014-10-11 10:34 UTC (permalink / raw) To: Baoquan He, Vivek Goyal, Kees Cook Cc: linux-kernel, tglx, mingo, x86, keescook, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On 10/10/2014 08:14 PM, Baoquan He wrote: > On 10/08/14 at 03:27pm, Vivek Goyal wrote: >> On Wed, Oct 08, 2014 at 08:09:59AM -0700, H. Peter Anvin wrote: > >>> Sorry... this makes no sense. >>> >>> For x86-64, there is no direct connection between the physical and >>> virtual address spaces that the kernel runs in... >> >> I am sorry I did not understand this one. I thought that initial >> relocatable kernel implementaion did not have any direct connection >> between virtual and physical address. One could load kernel anywhere >> and kernel virtual address will not change and we will just adjust >> page tables to map virtual address to right physical address. >> >> Now handle_relocation() stuff seems to introduce a close coupling >> between physical and virtual address. So if kernel shifts by 16MB >> in physical address space, then it will shift by equal amount >> in virtual address space. So there seems to be a direct connection >> between virtual and physical address space in this case. > > Yeah, it's exactly as Vivek said. > > Before kaslr was introduced, x86_64 kernel can be put anywhere, and > always _text is 0xffffffff81000000. Meanwhile phys_base contains the > offset between the compiled addr (namely 0x1000000) and kernel loaded > addr. After kaslr implementation was added, as long as kernel loaded > addr is different 0x1000000, it will call handle_relocations(). The > offset now is added onto each symbols including _text and phys_base > becomes 0. > > It's clearly showing that by checking /proc/kallsyms and value of > phys_base. > This really shouldn't have happened this way on x86-64. It has to happen this way on i386, but I worry that this may be a serious misdesign in kaslr on x86-64. I'm also wondering if there is any other fallout of this? -hpa ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-11 10:34 ` H. Peter Anvin @ 2014-10-11 12:38 ` Baoquan He 2014-10-11 12:44 ` Baoquan He 2014-10-13 12:52 ` Vivek Goyal 1 sibling, 1 reply; 25+ messages in thread From: Baoquan He @ 2014-10-11 12:38 UTC (permalink / raw) To: H. Peter Anvin Cc: Vivek Goyal, Kees Cook, linux-kernel, tglx, mingo, x86, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On 10/11/14 at 03:34am, H. Peter Anvin wrote: > On 10/10/2014 08:14 PM, Baoquan He wrote: > >On 10/08/14 at 03:27pm, Vivek Goyal wrote: > >>On Wed, Oct 08, 2014 at 08:09:59AM -0700, H. Peter Anvin wrote: > > > >>>Sorry... this makes no sense. > >>> > >>>For x86-64, there is no direct connection between the physical and > >>>virtual address spaces that the kernel runs in... > >> > >>I am sorry I did not understand this one. I thought that initial > >>relocatable kernel implementaion did not have any direct connection > >>between virtual and physical address. One could load kernel anywhere > >>and kernel virtual address will not change and we will just adjust > >>page tables to map virtual address to right physical address. > >> > >>Now handle_relocation() stuff seems to introduce a close coupling > >>between physical and virtual address. So if kernel shifts by 16MB > >>in physical address space, then it will shift by equal amount > >>in virtual address space. So there seems to be a direct connection > >>between virtual and physical address space in this case. > > > >Yeah, it's exactly as Vivek said. > > > >Before kaslr was introduced, x86_64 kernel can be put anywhere, and > >always _text is 0xffffffff81000000. Meanwhile phys_base contains the > >offset between the compiled addr (namely 0x1000000) and kernel loaded > >addr. After kaslr implementation was added, as long as kernel loaded > >addr is different 0x1000000, it will call handle_relocations(). The > >offset now is added onto each symbols including _text and phys_base > >becomes 0. > > > >It's clearly showing that by checking /proc/kallsyms and value of > >phys_base. > > > > This really shouldn't have happened this way on x86-64. It has to > happen this way on i386, but I worry that this may be a serious > misdesign in kaslr on x86-64. I'm also wondering if there is any > other fallout of this? Yes, this shouldn't happen this way on x86_64. With this patch, those are fixed as expected. If kernel location is not chosen randomly, we should not do the relocations handling. If and only if kaslr is enabled and it relocated the kernel randomly as expected, we do the relocations handling. I think this patch really makes sense and it's simple and won't impact i386 and other implementations. Thanks Baoquan ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-11 12:38 ` Baoquan He @ 2014-10-11 12:44 ` Baoquan He 0 siblings, 0 replies; 25+ messages in thread From: Baoquan He @ 2014-10-11 12:44 UTC (permalink / raw) To: H. Peter Anvin Cc: Vivek Goyal, Kees Cook, linux-kernel, tglx, mingo, x86, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On 10/11/14 at 08:38pm, Baoquan He wrote: > On 10/11/14 at 03:34am, H. Peter Anvin wrote: > > On 10/10/2014 08:14 PM, Baoquan He wrote: > > >On 10/08/14 at 03:27pm, Vivek Goyal wrote: > > >>On Wed, Oct 08, 2014 at 08:09:59AM -0700, H. Peter Anvin wrote: > > > > > >>>Sorry... this makes no sense. > > >>> > > >>>For x86-64, there is no direct connection between the physical and > > >>>virtual address spaces that the kernel runs in... > > >> > > >>I am sorry I did not understand this one. I thought that initial > > >>relocatable kernel implementaion did not have any direct connection > > >>between virtual and physical address. One could load kernel anywhere > > >>and kernel virtual address will not change and we will just adjust > > >>page tables to map virtual address to right physical address. > > >> > > >>Now handle_relocation() stuff seems to introduce a close coupling > > >>between physical and virtual address. So if kernel shifts by 16MB > > >>in physical address space, then it will shift by equal amount > > >>in virtual address space. So there seems to be a direct connection > > >>between virtual and physical address space in this case. > > > > > >Yeah, it's exactly as Vivek said. > > > > > >Before kaslr was introduced, x86_64 kernel can be put anywhere, and > > >always _text is 0xffffffff81000000. Meanwhile phys_base contains the > > >offset between the compiled addr (namely 0x1000000) and kernel loaded > > >addr. After kaslr implementation was added, as long as kernel loaded > > >addr is different 0x1000000, it will call handle_relocations(). The > > >offset now is added onto each symbols including _text and phys_base > > >becomes 0. > > > > > >It's clearly showing that by checking /proc/kallsyms and value of > > >phys_base. > > > > > > > This really shouldn't have happened this way on x86-64. It has to > > happen this way on i386, but I worry that this may be a serious > > misdesign in kaslr on x86-64. I'm also wondering if there is any > > other fallout of this? Btw, except of this bug, I didn't find other risk of kaslr currently. The code flow is straightforward and clear. > > Yes, this shouldn't happen this way on x86_64. With this patch, those > are fixed as expected. If kernel location is not chosen randomly, we > should not do the relocations handling. If and only if kaslr is enabled > and it relocated the kernel randomly as expected, we do the relocations > handling. > > I think this patch really makes sense and it's simple and won't impact > i386 and other implementations. > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-11 10:34 ` H. Peter Anvin 2014-10-11 12:38 ` Baoquan He @ 2014-10-13 12:52 ` Vivek Goyal 2014-10-13 15:19 ` Vivek Goyal 1 sibling, 1 reply; 25+ messages in thread From: Vivek Goyal @ 2014-10-13 12:52 UTC (permalink / raw) To: H. Peter Anvin Cc: Baoquan He, Kees Cook, linux-kernel, tglx, mingo, x86, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On Sat, Oct 11, 2014 at 03:34:29AM -0700, H. Peter Anvin wrote: > On 10/10/2014 08:14 PM, Baoquan He wrote: > >On 10/08/14 at 03:27pm, Vivek Goyal wrote: > >>On Wed, Oct 08, 2014 at 08:09:59AM -0700, H. Peter Anvin wrote: > > > >>>Sorry... this makes no sense. > >>> > >>>For x86-64, there is no direct connection between the physical and > >>>virtual address spaces that the kernel runs in... > >> > >>I am sorry I did not understand this one. I thought that initial > >>relocatable kernel implementaion did not have any direct connection > >>between virtual and physical address. One could load kernel anywhere > >>and kernel virtual address will not change and we will just adjust > >>page tables to map virtual address to right physical address. > >> > >>Now handle_relocation() stuff seems to introduce a close coupling > >>between physical and virtual address. So if kernel shifts by 16MB > >>in physical address space, then it will shift by equal amount > >>in virtual address space. So there seems to be a direct connection > >>between virtual and physical address space in this case. > > > >Yeah, it's exactly as Vivek said. > > > >Before kaslr was introduced, x86_64 kernel can be put anywhere, and > >always _text is 0xffffffff81000000. Meanwhile phys_base contains the > >offset between the compiled addr (namely 0x1000000) and kernel loaded > >addr. After kaslr implementation was added, as long as kernel loaded > >addr is different 0x1000000, it will call handle_relocations(). The > >offset now is added onto each symbols including _text and phys_base > >becomes 0. > > > >It's clearly showing that by checking /proc/kallsyms and value of > >phys_base. > > > > This really shouldn't have happened this way on x86-64. It has to happen > this way on i386, but I worry that this may be a serious misdesign in kaslr > on x86-64. I'm also wondering if there is any other fallout of this? I agree. On x86_64, we should stick to previous design and this new logic of performing relocations does not sound very clean and makes things very confusing. I am wondering that why couldn't we simply adjust page tables in case of kaslr on x86_64, instead of performing relocations. Thanks Vivek ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-13 12:52 ` Vivek Goyal @ 2014-10-13 15:19 ` Vivek Goyal 2014-10-13 15:43 ` H. Peter Anvin 0 siblings, 1 reply; 25+ messages in thread From: Vivek Goyal @ 2014-10-13 15:19 UTC (permalink / raw) To: H. Peter Anvin Cc: Baoquan He, Kees Cook, linux-kernel, tglx, mingo, x86, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On Mon, Oct 13, 2014 at 08:52:57AM -0400, Vivek Goyal wrote: > On Sat, Oct 11, 2014 at 03:34:29AM -0700, H. Peter Anvin wrote: > > On 10/10/2014 08:14 PM, Baoquan He wrote: > > >On 10/08/14 at 03:27pm, Vivek Goyal wrote: > > >>On Wed, Oct 08, 2014 at 08:09:59AM -0700, H. Peter Anvin wrote: > > > > > >>>Sorry... this makes no sense. > > >>> > > >>>For x86-64, there is no direct connection between the physical and > > >>>virtual address spaces that the kernel runs in... > > >> > > >>I am sorry I did not understand this one. I thought that initial > > >>relocatable kernel implementaion did not have any direct connection > > >>between virtual and physical address. One could load kernel anywhere > > >>and kernel virtual address will not change and we will just adjust > > >>page tables to map virtual address to right physical address. > > >> > > >>Now handle_relocation() stuff seems to introduce a close coupling > > >>between physical and virtual address. So if kernel shifts by 16MB > > >>in physical address space, then it will shift by equal amount > > >>in virtual address space. So there seems to be a direct connection > > >>between virtual and physical address space in this case. > > > > > >Yeah, it's exactly as Vivek said. > > > > > >Before kaslr was introduced, x86_64 kernel can be put anywhere, and > > >always _text is 0xffffffff81000000. Meanwhile phys_base contains the > > >offset between the compiled addr (namely 0x1000000) and kernel loaded > > >addr. After kaslr implementation was added, as long as kernel loaded > > >addr is different 0x1000000, it will call handle_relocations(). The > > >offset now is added onto each symbols including _text and phys_base > > >becomes 0. > > > > > >It's clearly showing that by checking /proc/kallsyms and value of > > >phys_base. > > > > > > > This really shouldn't have happened this way on x86-64. It has to happen > > this way on i386, but I worry that this may be a serious misdesign in kaslr > > on x86-64. I'm also wondering if there is any other fallout of this? > > I agree. On x86_64, we should stick to previous design and this new > logic of performing relocations does not sound very clean and makes > things very confusing. > > I am wondering that why couldn't we simply adjust page tables in case of > kaslr on x86_64, instead of performing relocations. Well, IIUC, if virtual addresses are shifted w.r.t what virtual address kernel was compiled for, then relocation will have to be done. So question will be if physical address shift is enough for kaslr or virtual address shift is necessary. Thanks Vivek ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-13 15:19 ` Vivek Goyal @ 2014-10-13 15:43 ` H. Peter Anvin 2014-10-13 17:22 ` Vivek Goyal 0 siblings, 1 reply; 25+ messages in thread From: H. Peter Anvin @ 2014-10-13 15:43 UTC (permalink / raw) To: Vivek Goyal Cc: Baoquan He, Kees Cook, linux-kernel, tglx, mingo, x86, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On 10/13/2014 08:19 AM, Vivek Goyal wrote: >>> >>> This really shouldn't have happened this way on x86-64. It has to happen >>> this way on i386, but I worry that this may be a serious misdesign in kaslr >>> on x86-64. I'm also wondering if there is any other fallout of this? >> >> I agree. On x86_64, we should stick to previous design and this new >> logic of performing relocations does not sound very clean and makes >> things very confusing. >> >> I am wondering that why couldn't we simply adjust page tables in case of >> kaslr on x86_64, instead of performing relocations. > > Well, IIUC, if virtual addresses are shifted w.r.t what virtual address > kernel was compiled for, then relocation will have to be done. > > So question will be if physical address shift is enough for kaslr or > virtual address shift is necessary. > I would assume that without a virtual address shift kaslr is pretty darn pointless. Without the physical address shift the 1:1 map can be used, and again, kaslr becomes pointless. However, there is absolutely no reason why they should be coupled. They can, in fact, be independently randomized. -hpa ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-13 15:43 ` H. Peter Anvin @ 2014-10-13 17:22 ` Vivek Goyal 2014-10-14 12:49 ` Vivek Goyal 0 siblings, 1 reply; 25+ messages in thread From: Vivek Goyal @ 2014-10-13 17:22 UTC (permalink / raw) To: H. Peter Anvin Cc: Baoquan He, Kees Cook, linux-kernel, tglx, mingo, x86, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On Mon, Oct 13, 2014 at 08:43:00AM -0700, H. Peter Anvin wrote: > On 10/13/2014 08:19 AM, Vivek Goyal wrote: > >>> > >>> This really shouldn't have happened this way on x86-64. It has to happen > >>> this way on i386, but I worry that this may be a serious misdesign in kaslr > >>> on x86-64. I'm also wondering if there is any other fallout of this? > >> > >> I agree. On x86_64, we should stick to previous design and this new > >> logic of performing relocations does not sound very clean and makes > >> things very confusing. > >> > >> I am wondering that why couldn't we simply adjust page tables in case of > >> kaslr on x86_64, instead of performing relocations. > > > > Well, IIUC, if virtual addresses are shifted w.r.t what virtual address > > kernel was compiled for, then relocation will have to be done. > > > > So question will be if physical address shift is enough for kaslr or > > virtual address shift is necessary. > > > > I would assume that without a virtual address shift kaslr is pretty darn > pointless. Without the physical address shift the 1:1 map can be used, > and again, kaslr becomes pointless. However, there is absolutely no > reason why they should be coupled. They can, in fact, be independently > randomized. Agreed. On x86_64, we should be able to randomize virtual address space and physical address space independently. And in that case whole of the physical memory should be available for a possible location for kernel. (As opposed to a small limit (I guess 1GB) now) Thanks Vivek ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-13 17:22 ` Vivek Goyal @ 2014-10-14 12:49 ` Vivek Goyal 2014-10-15 3:37 ` Baoquan He 0 siblings, 1 reply; 25+ messages in thread From: Vivek Goyal @ 2014-10-14 12:49 UTC (permalink / raw) To: H. Peter Anvin Cc: Baoquan He, Kees Cook, linux-kernel, tglx, mingo, x86, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On Mon, Oct 13, 2014 at 01:22:42PM -0400, Vivek Goyal wrote: > On Mon, Oct 13, 2014 at 08:43:00AM -0700, H. Peter Anvin wrote: > > On 10/13/2014 08:19 AM, Vivek Goyal wrote: > > >>> > > >>> This really shouldn't have happened this way on x86-64. It has to happen > > >>> this way on i386, but I worry that this may be a serious misdesign in kaslr > > >>> on x86-64. I'm also wondering if there is any other fallout of this? > > >> > > >> I agree. On x86_64, we should stick to previous design and this new > > >> logic of performing relocations does not sound very clean and makes > > >> things very confusing. > > >> > > >> I am wondering that why couldn't we simply adjust page tables in case of > > >> kaslr on x86_64, instead of performing relocations. > > > > > > Well, IIUC, if virtual addresses are shifted w.r.t what virtual address > > > kernel was compiled for, then relocation will have to be done. > > > > > > So question will be if physical address shift is enough for kaslr or > > > virtual address shift is necessary. > > > > > > > I would assume that without a virtual address shift kaslr is pretty darn > > pointless. Without the physical address shift the 1:1 map can be used, > > and again, kaslr becomes pointless. However, there is absolutely no > > reason why they should be coupled. They can, in fact, be independently > > randomized. > > Agreed. On x86_64, we should be able to randomize virtual address space > and physical address space independently. And in that case whole of > the physical memory should be available for a possible location for > kernel. (As opposed to a small limit (I guess 1GB) now) Hi Peter, So what do we do about this issue in short term to make kexec work. Even if we go for above solution, to make kexec work we will have to pass "nokaslr" as we don't want kernel to move around in physical address space as it might stomp over ELF headers we have stored. If you don't like current patch, should we just disable relocations in x86_64 if "nokaslr" command line is passed. That way kernel will not be moved in physical as well as virtual address space. Thanks Vivek ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-14 12:49 ` Vivek Goyal @ 2014-10-15 3:37 ` Baoquan He 2014-10-15 20:22 ` Vivek Goyal ` (2 more replies) 0 siblings, 3 replies; 25+ messages in thread From: Baoquan He @ 2014-10-15 3:37 UTC (permalink / raw) To: Vivek Goyal Cc: H. Peter Anvin, Kees Cook, linux-kernel, tglx, mingo, x86, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On 10/14/14 at 08:49am, Vivek Goyal wrote: > On Mon, Oct 13, 2014 at 01:22:42PM -0400, Vivek Goyal wrote: > > On Mon, Oct 13, 2014 at 08:43:00AM -0700, H. Peter Anvin wrote: > > > On 10/13/2014 08:19 AM, Vivek Goyal wrote: > > > >>> > > > >>> This really shouldn't have happened this way on x86-64. It has to happen > > > >>> this way on i386, but I worry that this may be a serious misdesign in kaslr > > > >>> on x86-64. I'm also wondering if there is any other fallout of this? > > > >> > > > >> I agree. On x86_64, we should stick to previous design and this new > > > >> logic of performing relocations does not sound very clean and makes > > > >> things very confusing. > > > >> > > > >> I am wondering that why couldn't we simply adjust page tables in case of > > > >> kaslr on x86_64, instead of performing relocations. > > > > > > > > Well, IIUC, if virtual addresses are shifted w.r.t what virtual address > > > > kernel was compiled for, then relocation will have to be done. > > > > > > > > So question will be if physical address shift is enough for kaslr or > > > > virtual address shift is necessary. > > > > > > > > > > I would assume that without a virtual address shift kaslr is pretty darn > > > pointless. Without the physical address shift the 1:1 map can be used, > > > and again, kaslr becomes pointless. However, there is absolutely no > > > reason why they should be coupled. They can, in fact, be independently > > > randomized. > > > > Agreed. On x86_64, we should be able to randomize virtual address space > > and physical address space independently. And in that case whole of > > the physical memory should be available for a possible location for > > kernel. (As opposed to a small limit (I guess 1GB) now) It can be done to randomize virtual address space and physical address space independently. But limited by the 2G of kernel text mapping and module mapping virtual address space, virtual address can be randomized in (0x1000000, 1G) range. While physical address can be randomized in (0x1000000, 4G) according to the identity mapping of normal kernel. Then phys_base still stores an relative value, a different offset than before. This can be easily implement. One thing is still there's a limit for physical addr randomization, only below 4G. So I am wondering if we can extend the identify mapping to complete mapping of 48 bit, using 1G page frame. This can make physical addr be randomized to anywhere. So now there may be 3 options: 1) Fix this bug in current kaslr. Since when randomize the new kernel location in choose_kernel_location(), cmdline options has been checked strictly, e.g if nokaslr is specified, it's safe to do the kernel location randomization. Then in handle_relocations(), we only need to check if the kernel location is changed, comparing with kernel loaded addr. If changed, kaslr is done, let's do the relocation handling. If not changed, no kaslr id done, just skip the relocation handling like before. 2) randomize the virtual addr space and physical addr space independently. But physical addr space must be below 4G. 3) extend the identity mapping to 48bit of addr space. Then we can randomized the virtual addr space in (0x1000000, 1G) and physical addr space in (0x1000000, real physical memory end). If option 3 is doable, it's the best. If not, I think bug fix should be better. > > Hi Peter, > > So what do we do about this issue in short term to make kexec work. Even > if we go for above solution, to make kexec work we will have to pass > "nokaslr" as we don't want kernel to move around in physical address space > as it might stomp over ELF headers we have stored. kexec doesn't need ELF headers. Kdump may need it. But in current kexec-tools implementation, kernel/initrd and other stuffs are placed from top to down, current implementation won't do kaslr since it only happened between kernel loaded addr and 1G. So we don't need to worry about the stomping. > > If you don't like current patch, should we just disable relocations in > x86_64 if "nokaslr" command line is passed. That way kernel will not > be moved in physical as well as virtual address space. > > Thanks > Vivek ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-15 3:37 ` Baoquan He @ 2014-10-15 20:22 ` Vivek Goyal 2014-10-15 20:32 ` H. Peter Anvin 2014-10-28 5:04 ` Baoquan He 2 siblings, 0 replies; 25+ messages in thread From: Vivek Goyal @ 2014-10-15 20:22 UTC (permalink / raw) To: Baoquan He Cc: H. Peter Anvin, Kees Cook, linux-kernel, tglx, mingo, x86, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On Wed, Oct 15, 2014 at 11:37:01AM +0800, Baoquan He wrote: > On 10/14/14 at 08:49am, Vivek Goyal wrote: > > On Mon, Oct 13, 2014 at 01:22:42PM -0400, Vivek Goyal wrote: > > > On Mon, Oct 13, 2014 at 08:43:00AM -0700, H. Peter Anvin wrote: > > > > On 10/13/2014 08:19 AM, Vivek Goyal wrote: > > > > >>> > > > > >>> This really shouldn't have happened this way on x86-64. It has to happen > > > > >>> this way on i386, but I worry that this may be a serious misdesign in kaslr > > > > >>> on x86-64. I'm also wondering if there is any other fallout of this? > > > > >> > > > > >> I agree. On x86_64, we should stick to previous design and this new > > > > >> logic of performing relocations does not sound very clean and makes > > > > >> things very confusing. > > > > >> > > > > >> I am wondering that why couldn't we simply adjust page tables in case of > > > > >> kaslr on x86_64, instead of performing relocations. > > > > > > > > > > Well, IIUC, if virtual addresses are shifted w.r.t what virtual address > > > > > kernel was compiled for, then relocation will have to be done. > > > > > > > > > > So question will be if physical address shift is enough for kaslr or > > > > > virtual address shift is necessary. > > > > > > > > > > > > > I would assume that without a virtual address shift kaslr is pretty darn > > > > pointless. Without the physical address shift the 1:1 map can be used, > > > > and again, kaslr becomes pointless. However, there is absolutely no > > > > reason why they should be coupled. They can, in fact, be independently > > > > randomized. > > > > > > Agreed. On x86_64, we should be able to randomize virtual address space > > > and physical address space independently. And in that case whole of > > > the physical memory should be available for a possible location for > > > kernel. (As opposed to a small limit (I guess 1GB) now) > > It can be done to randomize virtual address space and physical address > space independently. But limited by the 2G of kernel text mapping and > module mapping virtual address space, virtual address can be randomized > in (0x1000000, 1G) range. While physical address can be randomized in > (0x1000000, 4G) according to the identity mapping of normal kernel. Then > phys_base still stores an relative value, a different offset than before. > > This can be easily implement. One thing is still there's a limit for > physical addr randomization, only below 4G. So I am wondering if we can > extend the identify mapping to complete mapping of 48 bit, using 1G page > frame. This can make physical addr be randomized to anywhere. I am wondering where does this 4G limit come from? Without kaslr now we are able to load kernels much higher than 4G. That would suggest that we should be able to pick randomly any address above 4G too? Thanks Vivek ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-15 3:37 ` Baoquan He 2014-10-15 20:22 ` Vivek Goyal @ 2014-10-15 20:32 ` H. Peter Anvin 2014-10-15 23:55 ` Baoquan He 2014-10-28 5:04 ` Baoquan He 2 siblings, 1 reply; 25+ messages in thread From: H. Peter Anvin @ 2014-10-15 20:32 UTC (permalink / raw) To: Baoquan He, Vivek Goyal Cc: Kees Cook, linux-kernel, tglx, mingo, x86, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable I don't see why we can't randomize anywhere in physical space. We already handle the kernel above 4 GB and it wouldn't be hard to do the equivalent for the decompress/relocation code, using a #PF handler. Not all CPUs support 1 GB pages. On October 14, 2014 8:37:01 PM PDT, Baoquan He <bhe@redhat.com> wrote: >On 10/14/14 at 08:49am, Vivek Goyal wrote: >> On Mon, Oct 13, 2014 at 01:22:42PM -0400, Vivek Goyal wrote: >> > On Mon, Oct 13, 2014 at 08:43:00AM -0700, H. Peter Anvin wrote: >> > > On 10/13/2014 08:19 AM, Vivek Goyal wrote: >> > > >>> >> > > >>> This really shouldn't have happened this way on x86-64. It >has to happen >> > > >>> this way on i386, but I worry that this may be a serious >misdesign in kaslr >> > > >>> on x86-64. I'm also wondering if there is any other fallout >of this? >> > > >> >> > > >> I agree. On x86_64, we should stick to previous design and >this new >> > > >> logic of performing relocations does not sound very clean and >makes >> > > >> things very confusing. >> > > >> >> > > >> I am wondering that why couldn't we simply adjust page tables >in case of >> > > >> kaslr on x86_64, instead of performing relocations. >> > > > >> > > > Well, IIUC, if virtual addresses are shifted w.r.t what virtual >address >> > > > kernel was compiled for, then relocation will have to be done. >> > > > >> > > > So question will be if physical address shift is enough for >kaslr or >> > > > virtual address shift is necessary. >> > > > >> > > >> > > I would assume that without a virtual address shift kaslr is >pretty darn >> > > pointless. Without the physical address shift the 1:1 map can be >used, >> > > and again, kaslr becomes pointless. However, there is absolutely >no >> > > reason why they should be coupled. They can, in fact, be >independently >> > > randomized. >> > >> > Agreed. On x86_64, we should be able to randomize virtual address >space >> > and physical address space independently. And in that case whole of >> > the physical memory should be available for a possible location for >> > kernel. (As opposed to a small limit (I guess 1GB) now) > >It can be done to randomize virtual address space and physical address >space independently. But limited by the 2G of kernel text mapping and >module mapping virtual address space, virtual address can be randomized >in (0x1000000, 1G) range. While physical address can be randomized in >(0x1000000, 4G) according to the identity mapping of normal kernel. >Then >phys_base still stores an relative value, a different offset than >before. > >This can be easily implement. One thing is still there's a limit for >physical addr randomization, only below 4G. So I am wondering if we can >extend the identify mapping to complete mapping of 48 bit, using 1G >page >frame. This can make physical addr be randomized to anywhere. > >So now there may be 3 options: > >1) Fix this bug in current kaslr. Since when randomize the new kernel >location in choose_kernel_location(), cmdline options has been checked >strictly, e.g if nokaslr is specified, it's safe to do the kernel >location randomization. Then in handle_relocations(), we only need to >check if the kernel location is changed, comparing with kernel loaded >addr. If changed, kaslr is done, let's do the relocation handling. If >not changed, no kaslr id done, just skip the relocation handling like >before. > >2) randomize the virtual addr space and physical addr space >independently. But physical addr space must be below 4G. > >3) extend the identity mapping to 48bit of addr space. Then we can >randomized the virtual addr space in (0x1000000, 1G) and physical addr >space in (0x1000000, real physical memory end). > >If option 3 is doable, it's the best. If not, I think bug fix should be >better. > >> >> Hi Peter, >> >> So what do we do about this issue in short term to make kexec work. >Even >> if we go for above solution, to make kexec work we will have to pass >> "nokaslr" as we don't want kernel to move around in physical address >space >> as it might stomp over ELF headers we have stored. > >kexec doesn't need ELF headers. Kdump may need it. But in current >kexec-tools implementation, kernel/initrd and other stuffs are placed >from top to down, current implementation won't do kaslr since it only >happened between kernel loaded addr and 1G. So we don't need to worry >about the stomping. > >> >> If you don't like current patch, should we just disable relocations >in >> x86_64 if "nokaslr" command line is passed. That way kernel will not >> be moved in physical as well as virtual address space. >> >> Thanks >> Vivek -- Sent from my mobile phone. Please pardon brevity and lack of formatting. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-15 20:32 ` H. Peter Anvin @ 2014-10-15 23:55 ` Baoquan He 2014-10-15 23:58 ` Baoquan He 0 siblings, 1 reply; 25+ messages in thread From: Baoquan He @ 2014-10-15 23:55 UTC (permalink / raw) To: H. Peter Anvin Cc: Vivek Goyal, Kees Cook, linux-kernel, tglx, mingo, x86, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On 10/15/14 at 01:32pm, H. Peter Anvin wrote: > I don't see why we can't randomize anywhere in physical space. We already handle the kernel above 4 GB and it wouldn't be hard to do the equivalent for the decompress/relocation code, using a #PF handler. Not all CPUs support 1 GB pages. > Well, IIUC, in arch/x86/boot/compressed/head_64.S pgtable contains 6 page frames and is used to build 4G identity mapping. Then this pgtable is loaded into cr3. Later jump into 64bit mode. decompress/relocation code should work in this region. Isn't it right? Currently kernel surely can be put anywhere, even above 4G. This have been done very often in kexec/kdump. However in kexec/kdump the identity mapping is built for all physical memory after jumping to startup_64 directly from purgatory, that's why kexec/kdump kernel can be put anywhere. So for normal kernel, don't we need to extend the physical memory mappng to a larger region if we want to randomize anywhere in physical space? Thanks Baoquan > On October 14, 2014 8:37:01 PM PDT, Baoquan He <bhe@redhat.com> wrote: > >On 10/14/14 at 08:49am, Vivek Goyal wrote: > >> On Mon, Oct 13, 2014 at 01:22:42PM -0400, Vivek Goyal wrote: > >> > On Mon, Oct 13, 2014 at 08:43:00AM -0700, H. Peter Anvin wrote: > >> > > On 10/13/2014 08:19 AM, Vivek Goyal wrote: > >> > > >>> > >> > > >>> This really shouldn't have happened this way on x86-64. It > >has to happen > >> > > >>> this way on i386, but I worry that this may be a serious > >misdesign in kaslr > >> > > >>> on x86-64. I'm also wondering if there is any other fallout > >of this? > >> > > >> > >> > > >> I agree. On x86_64, we should stick to previous design and > >this new > >> > > >> logic of performing relocations does not sound very clean and > >makes > >> > > >> things very confusing. > >> > > >> > >> > > >> I am wondering that why couldn't we simply adjust page tables > >in case of > >> > > >> kaslr on x86_64, instead of performing relocations. > >> > > > > >> > > > Well, IIUC, if virtual addresses are shifted w.r.t what virtual > >address > >> > > > kernel was compiled for, then relocation will have to be done. > >> > > > > >> > > > So question will be if physical address shift is enough for > >kaslr or > >> > > > virtual address shift is necessary. > >> > > > > >> > > > >> > > I would assume that without a virtual address shift kaslr is > >pretty darn > >> > > pointless. Without the physical address shift the 1:1 map can be > >used, > >> > > and again, kaslr becomes pointless. However, there is absolutely > >no > >> > > reason why they should be coupled. They can, in fact, be > >independently > >> > > randomized. > >> > > >> > Agreed. On x86_64, we should be able to randomize virtual address > >space > >> > and physical address space independently. And in that case whole of > >> > the physical memory should be available for a possible location for > >> > kernel. (As opposed to a small limit (I guess 1GB) now) > > > >It can be done to randomize virtual address space and physical address > >space independently. But limited by the 2G of kernel text mapping and > >module mapping virtual address space, virtual address can be randomized > >in (0x1000000, 1G) range. While physical address can be randomized in > >(0x1000000, 4G) according to the identity mapping of normal kernel. > >Then > >phys_base still stores an relative value, a different offset than > >before. > > > >This can be easily implement. One thing is still there's a limit for > >physical addr randomization, only below 4G. So I am wondering if we can > >extend the identify mapping to complete mapping of 48 bit, using 1G > >page > >frame. This can make physical addr be randomized to anywhere. > > > >So now there may be 3 options: > > > >1) Fix this bug in current kaslr. Since when randomize the new kernel > >location in choose_kernel_location(), cmdline options has been checked > >strictly, e.g if nokaslr is specified, it's safe to do the kernel > >location randomization. Then in handle_relocations(), we only need to > >check if the kernel location is changed, comparing with kernel loaded > >addr. If changed, kaslr is done, let's do the relocation handling. If > >not changed, no kaslr id done, just skip the relocation handling like > >before. > > > >2) randomize the virtual addr space and physical addr space > >independently. But physical addr space must be below 4G. > > > >3) extend the identity mapping to 48bit of addr space. Then we can > >randomized the virtual addr space in (0x1000000, 1G) and physical addr > >space in (0x1000000, real physical memory end). > > > >If option 3 is doable, it's the best. If not, I think bug fix should be > >better. > > > >> > >> Hi Peter, > >> > >> So what do we do about this issue in short term to make kexec work. > >Even > >> if we go for above solution, to make kexec work we will have to pass > >> "nokaslr" as we don't want kernel to move around in physical address > >space > >> as it might stomp over ELF headers we have stored. > > > >kexec doesn't need ELF headers. Kdump may need it. But in current > >kexec-tools implementation, kernel/initrd and other stuffs are placed > >from top to down, current implementation won't do kaslr since it only > >happened between kernel loaded addr and 1G. So we don't need to worry > >about the stomping. > > > >> > >> If you don't like current patch, should we just disable relocations > >in > >> x86_64 if "nokaslr" command line is passed. That way kernel will not > >> be moved in physical as well as virtual address space. > >> > >> Thanks > >> Vivek > > -- > Sent from my mobile phone. Please pardon brevity and lack of formatting. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-15 23:55 ` Baoquan He @ 2014-10-15 23:58 ` Baoquan He 0 siblings, 0 replies; 25+ messages in thread From: Baoquan He @ 2014-10-15 23:58 UTC (permalink / raw) To: H. Peter Anvin Cc: Vivek Goyal, Kees Cook, linux-kernel, tglx, mingo, x86, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On 10/16/14 at 07:55am, Baoquan He wrote: > On 10/15/14 at 01:32pm, H. Peter Anvin wrote: > > I don't see why we can't randomize anywhere in physical space. We already handle the kernel above 4 GB and it wouldn't be hard to do the equivalent for the decompress/relocation code, using a #PF handler. Not all CPUs support 1 GB pages. > > > > Well, IIUC, in arch/x86/boot/compressed/head_64.S pgtable contains 6 > page frames and is used to build 4G identity mapping. Then this pgtable > is loaded into cr3. Later jump into 64bit mode. decompress/relocation > code should work in this region. Isn't it right? > > Currently kernel surely can be put anywhere, even above 4G. This have > been done very often in kexec/kdump. However in kexec/kdump the identity > mapping is built for all physical memory after jumping to startup_64 ~~~~~ should be before > directly from purgatory, that's why kexec/kdump kernel can be put > anywhere. > > So for normal kernel, don't we need to extend the physical memory mappng > to a larger region if we want to randomize anywhere in physical space? > > Thanks > Baoquan > > > > On October 14, 2014 8:37:01 PM PDT, Baoquan He <bhe@redhat.com> wrote: > > >On 10/14/14 at 08:49am, Vivek Goyal wrote: > > >> On Mon, Oct 13, 2014 at 01:22:42PM -0400, Vivek Goyal wrote: > > >> > On Mon, Oct 13, 2014 at 08:43:00AM -0700, H. Peter Anvin wrote: > > >> > > On 10/13/2014 08:19 AM, Vivek Goyal wrote: > > >> > > >>> > > >> > > >>> This really shouldn't have happened this way on x86-64. It > > >has to happen > > >> > > >>> this way on i386, but I worry that this may be a serious > > >misdesign in kaslr > > >> > > >>> on x86-64. I'm also wondering if there is any other fallout > > >of this? > > >> > > >> > > >> > > >> I agree. On x86_64, we should stick to previous design and > > >this new > > >> > > >> logic of performing relocations does not sound very clean and > > >makes > > >> > > >> things very confusing. > > >> > > >> > > >> > > >> I am wondering that why couldn't we simply adjust page tables > > >in case of > > >> > > >> kaslr on x86_64, instead of performing relocations. > > >> > > > > > >> > > > Well, IIUC, if virtual addresses are shifted w.r.t what virtual > > >address > > >> > > > kernel was compiled for, then relocation will have to be done. > > >> > > > > > >> > > > So question will be if physical address shift is enough for > > >kaslr or > > >> > > > virtual address shift is necessary. > > >> > > > > > >> > > > > >> > > I would assume that without a virtual address shift kaslr is > > >pretty darn > > >> > > pointless. Without the physical address shift the 1:1 map can be > > >used, > > >> > > and again, kaslr becomes pointless. However, there is absolutely > > >no > > >> > > reason why they should be coupled. They can, in fact, be > > >independently > > >> > > randomized. > > >> > > > >> > Agreed. On x86_64, we should be able to randomize virtual address > > >space > > >> > and physical address space independently. And in that case whole of > > >> > the physical memory should be available for a possible location for > > >> > kernel. (As opposed to a small limit (I guess 1GB) now) > > > > > >It can be done to randomize virtual address space and physical address > > >space independently. But limited by the 2G of kernel text mapping and > > >module mapping virtual address space, virtual address can be randomized > > >in (0x1000000, 1G) range. While physical address can be randomized in > > >(0x1000000, 4G) according to the identity mapping of normal kernel. > > >Then > > >phys_base still stores an relative value, a different offset than > > >before. > > > > > >This can be easily implement. One thing is still there's a limit for > > >physical addr randomization, only below 4G. So I am wondering if we can > > >extend the identify mapping to complete mapping of 48 bit, using 1G > > >page > > >frame. This can make physical addr be randomized to anywhere. > > > > > >So now there may be 3 options: > > > > > >1) Fix this bug in current kaslr. Since when randomize the new kernel > > >location in choose_kernel_location(), cmdline options has been checked > > >strictly, e.g if nokaslr is specified, it's safe to do the kernel > > >location randomization. Then in handle_relocations(), we only need to > > >check if the kernel location is changed, comparing with kernel loaded > > >addr. If changed, kaslr is done, let's do the relocation handling. If > > >not changed, no kaslr id done, just skip the relocation handling like > > >before. > > > > > >2) randomize the virtual addr space and physical addr space > > >independently. But physical addr space must be below 4G. > > > > > >3) extend the identity mapping to 48bit of addr space. Then we can > > >randomized the virtual addr space in (0x1000000, 1G) and physical addr > > >space in (0x1000000, real physical memory end). > > > > > >If option 3 is doable, it's the best. If not, I think bug fix should be > > >better. > > > > > >> > > >> Hi Peter, > > >> > > >> So what do we do about this issue in short term to make kexec work. > > >Even > > >> if we go for above solution, to make kexec work we will have to pass > > >> "nokaslr" as we don't want kernel to move around in physical address > > >space > > >> as it might stomp over ELF headers we have stored. > > > > > >kexec doesn't need ELF headers. Kdump may need it. But in current > > >kexec-tools implementation, kernel/initrd and other stuffs are placed > > >from top to down, current implementation won't do kaslr since it only > > >happened between kernel loaded addr and 1G. So we don't need to worry > > >about the stomping. > > > > > >> > > >> If you don't like current patch, should we just disable relocations > > >in > > >> x86_64 if "nokaslr" command line is passed. That way kernel will not > > >> be moved in physical as well as virtual address space. > > >> > > >> Thanks > > >> Vivek > > > > -- > > Sent from my mobile phone. Please pardon brevity and lack of formatting. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-10-15 3:37 ` Baoquan He 2014-10-15 20:22 ` Vivek Goyal 2014-10-15 20:32 ` H. Peter Anvin @ 2014-10-28 5:04 ` Baoquan He 2 siblings, 0 replies; 25+ messages in thread From: Baoquan He @ 2014-10-28 5:04 UTC (permalink / raw) To: Vivek Goyal, H. Peter Anvin Cc: Kees Cook, linux-kernel, tglx, mingo, x86, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On 10/15/14 at 11:37am, Baoquan He wrote: > On 10/14/14 at 08:49am, Vivek Goyal wrote: > > On Mon, Oct 13, 2014 at 01:22:42PM -0400, Vivek Goyal wrote: > > > Agreed. On x86_64, we should be able to randomize virtual address space > > > and physical address space independently. And in that case whole of > > > the physical memory should be available for a possible location for > > > kernel. (As opposed to a small limit (I guess 1GB) now) > > It can be done to randomize virtual address space and physical address > space independently. But limited by the 2G of kernel text mapping and > module mapping virtual address space, virtual address can be randomized > in (0x1000000, 1G) range. While physical address can be randomized in > (0x1000000, 4G) according to the identity mapping of normal kernel. Then > phys_base still stores an relative value, a different offset than before. > > This can be easily implement. One thing is still there's a limit for > physical addr randomization, only below 4G. So I am wondering if we can > extend the identify mapping to complete mapping of 48 bit, using 1G page > frame. This can make physical addr be randomized to anywhere. > > So now there may be 3 options: Hi hpa, About kaslr, do you have a suggestion or plan? Should we fix bug based on current code, or any other solutions? Thanks Baoquan > > 1) Fix this bug in current kaslr. Since when randomize the new kernel > location in choose_kernel_location(), cmdline options has been checked > strictly, e.g if nokaslr is specified, it's safe to do the kernel > location randomization. Then in handle_relocations(), we only need to > check if the kernel location is changed, comparing with kernel loaded > addr. If changed, kaslr is done, let's do the relocation handling. If > not changed, no kaslr id done, just skip the relocation handling like > before. > > 2) randomize the virtual addr space and physical addr space > independently. But physical addr space must be below 4G. > > 3) extend the identity mapping to 48bit of addr space. Then we can > randomized the virtual addr space in (0x1000000, 1G) and physical addr > space in (0x1000000, real physical memory end). > > If option 3 is doable, it's the best. If not, I think bug fix should be > better. > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-09-30 21:21 ` [resend Patch v3 1/2] kaslr: check if kernel location is changed H. Peter Anvin 2014-10-01 13:52 ` Vivek Goyal @ 2014-10-08 14:40 ` Baoquan He 1 sibling, 0 replies; 25+ messages in thread From: Baoquan He @ 2014-10-08 14:40 UTC (permalink / raw) To: H. Peter Anvin Cc: linux-kernel, tglx, mingo, x86, vgoyal, keescook, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable On 09/30/14 at 02:21pm, H. Peter Anvin wrote: > On 09/30/2014 12:08 AM, 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> > > Acked-by: Vivek Goyal <vgoyal@redhat.com> > > Acked-by: Kees Cook <keescook@chromium.org> > > Tested-by: Thomas D. <whissi@whissi.de> > > Cc: stable@vger.kernel.org > > Could you clarify under what conditions we may end up with 32-bit signed > overflow, and yet have a functional kernel? Hi hpa, As Vivek has replied, and from my test result, in x86_64 when kernel is loaded above 2G by bootloader, the 32-bit signed overflow will happen. Meanwhile the error() in arch/x86/boot/compressed/misc.c will be called, this function will execute 'hlt' instruction, then system halted. In current kaslr, there are 3 hebaviours. Since the candidate slot for randomized kernel location is only between kernel loaded address and 1G, it doesn't choose a new kernel location when kernel is loaded above 1G. 1) Kernel is loaded between 0x1000000 and 1G, kaslr works well. 2)kernel is loaded between 1G and 2G, kernel doesn't choose a new kernel location, so the ourput_orig==output > 1G. But current code compares the output with LOAD_PHYSICAL_ADDR, relocation handling is still taken and this cause kenrel text mapping stomps module mapping space. So in this case form printing we can see kaslr passed, but kernel reboot to bios immediately. 3) When kernel is loaded above 2G, this is the 32-bit signed overflow as we discussed above. So this patch can fix case 2 and case 3. Thanks Baoquan > > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed 2014-09-30 7:08 [resend Patch v3 1/2] kaslr: check if kernel location is changed Baoquan He 2014-09-30 7:08 ` [resend Patch v3 2/2] export the kernel image size KERNEL_IMAGE_SIZE Baoquan He 2014-09-30 21:21 ` [resend Patch v3 1/2] kaslr: check if kernel location is changed H. Peter Anvin @ 2015-01-09 2:09 ` Baoquan He 2 siblings, 0 replies; 25+ messages in thread From: Baoquan He @ 2015-01-09 2:09 UTC (permalink / raw) To: hpa Cc: linux-kernel, tglx, mingo, x86, vgoyal, keescook, ak, ebiederm, kexec, whissi, kumagai-atsushi, stable Hi hpa, Ping. Do you have further plan or idea on this issue? Or could you please merge this patch to fix reported bug for now? Thanks Baoquan On 09/30/14 at 03:08pm, 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> > Acked-by: Vivek Goyal <vgoyal@redhat.com> > Acked-by: Kees Cook <keescook@chromium.org> > Tested-by: Thomas D. <whissi@whissi.de> > Cc: stable@vger.kernel.org > --- > 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] 25+ messages in thread
end of thread, other threads:[~2015-02-09 20:18 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-30 7:08 [resend Patch v3 1/2] kaslr: check if kernel location is changed Baoquan He 2014-09-30 7:08 ` [resend Patch v3 2/2] export the kernel image size KERNEL_IMAGE_SIZE Baoquan He 2015-02-02 7:32 ` Baoquan He 2015-02-09 20:18 ` Kees Cook 2014-09-30 21:21 ` [resend Patch v3 1/2] kaslr: check if kernel location is changed H. Peter Anvin 2014-10-01 13:52 ` Vivek Goyal 2014-10-08 15:09 ` H. Peter Anvin 2014-10-08 19:27 ` Vivek Goyal 2014-10-11 3:14 ` Baoquan He 2014-10-11 10:34 ` H. Peter Anvin 2014-10-11 12:38 ` Baoquan He 2014-10-11 12:44 ` Baoquan He 2014-10-13 12:52 ` Vivek Goyal 2014-10-13 15:19 ` Vivek Goyal 2014-10-13 15:43 ` H. Peter Anvin 2014-10-13 17:22 ` Vivek Goyal 2014-10-14 12:49 ` Vivek Goyal 2014-10-15 3:37 ` Baoquan He 2014-10-15 20:22 ` Vivek Goyal 2014-10-15 20:32 ` H. Peter Anvin 2014-10-15 23:55 ` Baoquan He 2014-10-15 23:58 ` Baoquan He 2014-10-28 5:04 ` Baoquan He 2014-10-08 14:40 ` Baoquan He 2015-01-09 2:09 ` Baoquan He
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).