This should also cc linux-efi

On 03/24/17 at 10:29am, Dave Young wrote:
> Hi, Baoquan
> 
> On 03/23/17 at 11:27am, Baoquan He wrote:
> > Currently KASLR is enabled on three regions: the direct mapping of physical
> > memory, vamlloc and vmemmap. However EFI region is also mistakenly included
> > for VA space randomization because of misusing EFI_VA_START macro and
> > assuming EFI_VA_START < EFI_VA_END.
> > 
> > The EFI region is reserved for EFI runtime services virtual mapping which
> > should not be included in kaslr ranges. It will be re-used by kexec/kdump
> > kernel, the mistake may cause failure when jump to kexec/kdump kernel if
> > vmemmap allocation stomps on the allocated efi mapping region.
> 
> No need to mention kexec/kdump in changelog although it is true that
> kexec kernel will use the persistent efi runtime mapping. The main point
> is it is wrong to use the reserved vm space for efi.

Explain more about this:

It is a general issue instead of a kexec/kdump issue and it is a real
bug. Although efi has its own page tables, it will still sync kernel
page tables along with the mapping of efi reserved area. So if vmalloc
etc use the vm space of efi reserved area, then some of them will be
missed when efi sync the low kernel page tables..

> 
> Also I think this patch can be sent as a standalone patch, no need to be
> a patch series. For the second patch I think it depends on efi
> maintainer's opinion, personally I think only this simple fix for kaslr only
> will be better.
> 
> > 
> > In Documentation/x86/x86_64/mm.txt, we can see:
> >   ffffffef00000000 - fffffffeffffffff (=64 GB) EFI region mapping space
> > EFI use the space from -4G to -64G thus EFI_VA_START > EFI_VA_END
> > Here EFI_VA_START = -4G, and EFI_VA_END = -64G
> > 
> > Changing EFI_VA_START to EFI_VA_END in mm/kaslr.c fixes this problem.
> > 
> > Cc: <[email protected]> #4.8+
> > Signed-off-by: Baoquan He <[email protected]>
> > Acked-by: Dave Young <[email protected]>
> > Reviewed-by: Bhupesh Sharma <[email protected]>
> > Acked-by: Thomas Garnier <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Ingo Molnar <[email protected]>
> > Cc: "H. Peter Anvin" <[email protected]> 
> > Cc: [email protected]
> > Cc: Thomas Garnier <[email protected]>
> > Cc: Kees Cook <[email protected]>
> > Cc: Borislav Petkov <[email protected]>
> > Cc: Andrew Morton <[email protected]>
> > Cc: Masahiro Yamada <[email protected]>
> > ---
> >  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
> > 
> 
> Thanks
> Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to