linux-kernel - Re: [PATCH 2/2] x86/mm/KASLR: Correct the upper boundary of KALSR mm regions if adjacent to EFI

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20170315061357.GB1938@x1>
Date:   Wed, 15 Mar 2017 14:13:57 +0800
From:   Baoquan He <bhe@...hat.com>
To:     linux-kernel@...r.kernel.org
Cc:     linux-efi@...r.kernel.org, thgarnie@...gle.com,
        keescook@...omium.org, tglx@...utronix.de, mingo@...hat.com,
        hpa@...or.com, x86@...nel.org, dyoung@...hat.com
Subject: Re: [PATCH 2/2] x86/mm/KASLR: Correct the upper boundary of KALSR mm
 regions if adjacent to EFI

PING!

Is there any suggestion for this code bug fix?

Boris added comment in patch 1/2 thread that it can also be fixed by
swapping the naming - EFI_VA_START and EFI_VA_END. As he said the
swapping can remove the confusion about the naming, while the con is
changing it now could confuse more people who have the current
mental picture of the mapping direction.

And there's also a well known similar use case, stack, like stack_end
naming in arch/x86/boot/main.c which is the low addr boundary of stack
region.

Any idea?

Thanks
Baoquan

On 03/08/17 at 03:47pm, Baoquan He wrote:
> EFI allocates runtime services regions top-down, starting from EFI_VA_START
> to EFI_VA_END. So EFI_VA_START is bigger than EFI_VA_END and is the end of
> EFI region. The upper boundary of memory regions randomized by KASLR should
> be EFI_VA_END if it's adjacent to EFI region, but not EFI_VA_START.
> 
> Correct it in this patch.
> 
> Signed-off-by: Baoquan He <bhe@...hat.com>
> ---
>  arch/x86/mm/kaslr.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
> index 887e571..aed2064 100644
> --- a/arch/x86/mm/kaslr.c
> +++ b/arch/x86/mm/kaslr.c
> @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = __PAGE_OFFSET_BASE;
>  #if defined(CONFIG_X86_ESPFIX64)
>  static const unsigned long vaddr_end = ESPFIX_BASE_ADDR;
>  #elif defined(CONFIG_EFI)
> -static const unsigned long vaddr_end = EFI_VA_START;
> +static const unsigned long vaddr_end = EFI_VA_END;
>  #else
>  static const unsigned long vaddr_end = __START_KERNEL_map;
>  #endif
> @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void)
>  	 */
>  	BUILD_BUG_ON(vaddr_start >= vaddr_end);
>  	BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) &&
> -		     vaddr_end >= EFI_VA_START);
> +		     vaddr_end >= EFI_VA_END);
>  	BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) ||
>  		      IS_ENABLED(CONFIG_EFI)) &&
>  		     vaddr_end >= __START_KERNEL_map);
> -- 
> 2.5.5
>