linux-kernel - Re: [PATCH v2 RESEND 2/2] x86/mm/KASLR: Do not adapt the size of the direct mapping section for SGI UV system

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]

Date:   Thu, 28 Sep 2017 09:56:05 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Baoquan He <bhe@...hat.com>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org, mingo@...hat.com,
        tglx@...utronix.de, hpa@...or.com, thgarnie@...gle.com,
        keescook@...omium.org, akpm@...ux-foundation.org,
        yamada.masahiro@...ionext.com, rja@....com, frank.ramsay@....com
Subject: Re: [PATCH v2 RESEND 2/2] x86/mm/KASLR: Do not adapt the size of the
 direct mapping section for SGI UV system


* Baoquan He <bhe@...hat.com> wrote:

> On SGI UV system, kernel often hangs when KASLR is enabled. Disabling
> KASLR makes kernel work well.
> 
> The back trace is:
> 
> kernel BUG at arch/x86/mm/init_64.c:311!
> invalid opcode: 0000 [#1] SMP
> [...]
> RIP: 0010:__init_extra_mapping+0x188/0x196
> [...]
> Call Trace:
>  init_extra_mapping_uc+0x13/0x15
>  map_high+0x67/0x75
>  map_mmioh_high_uv3+0x20a/0x219
>  uv_system_init_hub+0x12d9/0x1496
>  uv_system_init+0x27/0x29
>  native_smp_prepare_cpus+0x28d/0x2d8
>  kernel_init_freeable+0xdd/0x253
>  ? rest_init+0x80/0x80
>  kernel_init+0xe/0x110
>  ret_from_fork+0x2c/0x40
> 
> This is because the SGI UV system need map its MMIOH region to the direct
> mapping section, and the mapping happens in rest_init() which is much
> later than the calling of kernel_randomize_memory() to do mm KASLR. So
> mm KASLR can't count in the size of the MMIOH region when caculate the
> needed size of address space for the direct mapping section.
> 
> When KASLR is disabled, there are 64TB address space for both system RAM
> and the MMIOH regions to share. When KASLR is enabled, the current code
> of mm KASLR only reserves the actual size of system RAM plus extra 10TB
> for the direct mapping. Thus later the MMIOH mapping could go beyond
> the upper bound of the direct mapping to step into VMALLOC or VMEMMAP area.
> Then BUG_ON() in __init_extra_mapping() will be triggered.
> 
> E.g on the SGI UV3 machine where this bug is reported , there are two MMIOH
> regions:
> 
> [    1.519001] UV: Map MMIOH0_HI 0xffc00000000 - 0x100000000000
> [    1.523001] UV: Map MMIOH1_HI 0x100000000000 - 0x200000000000
> 
> They are [16TB-16G, 16TB) and [16TB, 32TB). On this machine, 512G RAM are
> spread out to 1TB regions. Then above two SGI MMIOH regions also will be
> mapped into the direct mapping section.
> 
> To fix it, we need check if it's SGI UV system by calling is_early_uv_system()
> in kernel_randomize_memory(). If yes, do not adapt the size of the direct
> mapping section, just keep it as 64TB.
> 
> Signed-off-by: Baoquan He <bhe@...hat.com>
> Reviewed-by: Thomas Garnier <thgarnie@...gle.com>
> Acked-by: Mike Travis <travis@....com> 
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: "H. Peter Anvin" <hpa@...or.com>
> Cc: x86@...nel.org
> Cc: Thomas Garnier <thgarnie@...gle.com>
> Cc: Kees Cook <keescook@...omium.org>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: Masahiro Yamada <yamada.masahiro@...ionext.com>
> ---
>  arch/x86/mm/kaslr.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
> index af599167fe3c..4d68c08df82d 100644
> --- a/arch/x86/mm/kaslr.c
> +++ b/arch/x86/mm/kaslr.c
> @@ -27,6 +27,7 @@
>  #include <asm/pgtable.h>
>  #include <asm/setup.h>
>  #include <asm/kaslr.h>
> +#include <asm/uv/uv.h>
>  
>  #include "mm_internal.h"
>  
> @@ -123,7 +124,7 @@ void __init kernel_randomize_memory(void)
>  		CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING;
>  
>  	/* Adapt phyiscal memory region size based on available memory */
> -	if (memory_tb < kaslr_regions[0].size_tb)
> +	if (memory_tb < kaslr_regions[0].size_tb && !is_early_uv_system())
>  		kaslr_regions[0].size_tb = memory_tb;

This is really an ugly hack. Is kaslr_regions[] incorrect? If so then it should be 
corrected instead of uglifying the code that uses it...

Thanks,

	Ingo