On Fri, Sep 12, 2014 at 8:33 AM, Vivek Goyal <vgoyal at 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 at 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 at 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 at chromium.org>
Cc: stable at 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
