linux-kernel - Re: [Patch v3 1/2] kaslr: check if kernel location is changed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140912153308.GB4408@redhat.com>
Date:	Fri, 12 Sep 2014 11:33:08 -0400
From:	Vivek Goyal <vgoyal@...hat.com>
To:	Baoquan He <bhe@...hat.com>, "Thomas D." <whissi@...ssi.de>
Cc:	linux-kernel@...r.kernel.org, kexec@...ts.infradead.org,
	ebiederm@...ssion.com, hpa@...or.com, tglx@...utronix.de,
	mingo@...hat.com, keescook@...omium.org, ak@...ux.intel.com,
	kumagai-atsushi@....nes.nec.co.jp
Subject: Re: [Patch v3 1/2] kaslr: check if kernel location is changed

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@...hat.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@...hat.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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/