Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1741241 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 RESEND 2/2] x86/mm/KASLR: Do not adapt the size of the  direct mapping section for SGI UV system Ingo Molnar <mingo@kernel.org> - 2017-09-28 10:00 +0200
    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@redhat.com> - 2017-09-28 10:40 +0200
      Re: [PATCH v2 RESEND 2/2] x86/mm/KASLR: Do not adapt the size of the  direct mapping section for SGI UV system Ingo Molnar <mingo@kernel.org> - 2017-09-28 11:10 +0200
        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@redhat.com> - 2017-09-30 13:30 +0200

#1741241 — 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@redhat.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@redhat.com>
> Reviewed-by: Thomas Garnier <thgarnie@google.com>
> Acked-by: Mike Travis <travis@sgi.com> 
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> Cc: Thomas Garnier <thgarnie@google.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.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

[toc] | [next] | [standalone]

#1741292

Hi Ingo,

On 09/28/17 at 09:56am, Ingo Molnar wrote:
> > 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 for looking into this!

If on SGI UV system, the kaslr_regions[0].size_tb, namely the size of
the direct mapping section, is incorrect. 

Its direct mapping size includes two parts:
#1 RAM size of system
#2 MMIOH region size which only SGI UV system has.

However, the #2 can only be got till uv_system_init() is called in
native_smp_prepare_cpus(). That is too late for mm KASLR calculation.
That's why I made this hack.

I checked uv_system_init() code, seems not easy to know the size of
MMIOH region before or inside kernel_randomize_memory(). I have CCed UV
devel experts, not sure if they have any idea about this. Otherwise,
this patch could be the only way I can think of.

Hi Mike and Russ,

Is there any chance we can get the size of MMIOH region before mm KASLR
code, namely before we call kernel_randomize_memory()?

Thanks
Baoquan

[toc] | [prev] | [next] | [standalone]

#1741311

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

> Hi Ingo,
> 
> On 09/28/17 at 09:56am, Ingo Molnar wrote:
> > > 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 for looking into this!
> 
> If on SGI UV system, the kaslr_regions[0].size_tb, namely the size of
> the direct mapping section, is incorrect. 
> 
> Its direct mapping size includes two parts:
> #1 RAM size of system
> #2 MMIOH region size which only SGI UV system has.
> 
> However, the #2 can only be got till uv_system_init() is called in
> native_smp_prepare_cpus(). That is too late for mm KASLR calculation.
> That's why I made this hack.
> 
> I checked uv_system_init() code, seems not easy to know the size of
> MMIOH region before or inside kernel_randomize_memory(). I have CCed UV
> devel experts, not sure if they have any idea about this. Otherwise,
> this patch could be the only way I can think of.
> 
> Hi Mike and Russ,
> 
> Is there any chance we can get the size of MMIOH region before mm KASLR
> code, namely before we call kernel_randomize_memory()?

I don't mind system specific quirks to hardware enumeration details, as long as 
they don't pollute generic code with such special hacks.

I.e. in this case it's wrong to allow kaslr_regions[0].size_tb to be wrong. Any 
other code that relies on it in the future will be wrong as well on UV systems.

The right quirk would be to fix that up where it gets introduced, or something 
like that.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1742606

Hi Mike,

On 09/28/17 at 07:10am, Mike Travis wrote:
> 
> 
> On 9/28/2017 2:01 AM, Ingo Molnar wrote:
> > 
> > > If on SGI UV system, the kaslr_regions[0].size_tb, namely the size of
> > > the direct mapping section, is incorrect.
> > > 
> > > Its direct mapping size includes two parts:
> > > #1 RAM size of system
> > > #2 MMIOH region size which only SGI UV system has.
> > > 
> > > However, the #2 can only be got till uv_system_init() is called in
> > > native_smp_prepare_cpus(). That is too late for mm KASLR calculation.
> > > That's why I made this hack.
> > > 
> > > I checked uv_system_init() code, seems not easy to know the size of
> > > MMIOH region before or inside kernel_randomize_memory(). I have CCed UV
> > > devel experts, not sure if they have any idea about this. Otherwise,
> > > this patch could be the only way I can think of.
> > > 
> > > Hi Mike and Russ,
> > > 
> > > Is there any chance we can get the size of MMIOH region before mm KASLR
> > > code, namely before we call kernel_randomize_memory()?
> 
> The sizes of the MMIOL and MMIOH areas are tied into the HUB design and how
> it is communicated to BIOS and the kernel.  This is via some of the config
> MMR's found in the HUB and it would be impossible to provide any access to
> these registers as they change with each new UV architecture.
> 
> The kernel does reserve the memory in the EFI memmap.  I can send you a
> console log of the full startup that includes the MMIOH reservations. Note
> that it is dependent on what I/O devices are actually present as UV does not
> map empty slots unless forced (because we'd quickly run out of resources.)
> Also, the EFI memmap entries do not specify the exact usage of the contained
> areas.

Does that mean we can get the size of MMIOH from efi entries? If yes,
please help provide a console log including those. If can get size from
efi, it will be more acceptable.

Or I can ask Frank to loan his uv system to me, not sure if he is doing
testing with them.

Thanks
Baoquan

> 
> > 
> > I don't mind system specific quirks to hardware enumeration details, as long as
> > they don't pollute generic code with such special hacks.
> > 
> > I.e. in this case it's wrong to allow kaslr_regions[0].size_tb to be wrong. Any
> > other code that relies on it in the future will be wrong as well on UV systems.
> 
> Which may come into play on other arches with the new upcoming memory
> technologies.
> > 
> > The right quirk would be to fix that up where it gets introduced, or something
> > like that.
> 
> Yes, does make sense.
> > 
> > Thanks,
> > 
> > 	Ingo
> > 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web