* [PATCH v1 RESEND 1/2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization [not found] <1490239655-20902-1-git-send-email-bhe@redhat.com> @ 2017-03-23 3:27 ` Baoquan He 2017-03-24 0:41 ` Baoquan He 2017-03-24 2:29 ` Dave Young 0 siblings, 2 replies; 5+ messages in thread From: Baoquan He @ 2017-03-23 3:27 UTC (permalink / raw) To: linux-kernel Cc: Baoquan He, stable, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Thomas Garnier, Kees Cook, Borislav Petkov, Andrew Morton, Masahiro Yamada 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. 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: <stable@vger.kernel.org> #4.8+ Signed-off-by: Baoquan He <bhe@redhat.com> Acked-by: Dave Young <dyoung@redhat.com> Reviewed-by: Bhupesh Sharma <bhsharma@redhat.com> Acked-by: Thomas Garnier <thgarnie@google.com> Cc: Thomas Gleixner <tglx@linutronix.de> 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: Borislav Petkov <bp@alien8.de> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Masahiro Yamada <yamada.masahiro@socionext.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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1 RESEND 1/2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization 2017-03-23 3:27 ` [PATCH v1 RESEND 1/2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization Baoquan He @ 2017-03-24 0:41 ` Baoquan He 2017-03-24 2:29 ` Dave Young 1 sibling, 0 replies; 5+ messages in thread From: Baoquan He @ 2017-03-24 0:41 UTC (permalink / raw) To: linux-kernel Cc: stable, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Thomas Garnier, Kees Cook, Borislav Petkov, Andrew Morton, Masahiro Yamada Hi, Ping! Since our kaslr mm back port has been held to wait for this upstream fix, and post deadline is very close, can this patch be merged or picked up into tip-bot treeu? This is an obvious code bug, it has risk to cause kexec/kdump boot failure, and the current code change won't bring other risk. Thanks 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. > > 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: <stable@vger.kernel.org> #4.8+ > Signed-off-by: Baoquan He <bhe@redhat.com> > Acked-by: Dave Young <dyoung@redhat.com> > Reviewed-by: Bhupesh Sharma <bhsharma@redhat.com> > Acked-by: Thomas Garnier <thgarnie@google.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > 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: Borislav Petkov <bp@alien8.de> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Masahiro Yamada <yamada.masahiro@socionext.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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 RESEND 1/2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization 2017-03-23 3:27 ` [PATCH v1 RESEND 1/2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization Baoquan He 2017-03-24 0:41 ` Baoquan He @ 2017-03-24 2:29 ` Dave Young 2017-03-24 3:05 ` Dave Young 2017-03-24 4:35 ` Baoquan He 1 sibling, 2 replies; 5+ messages in thread From: Dave Young @ 2017-03-24 2:29 UTC (permalink / raw) To: Baoquan He Cc: linux-kernel, stable, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Thomas Garnier, Kees Cook, Borislav Petkov, Andrew Morton, Masahiro Yamada 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. 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: <stable@vger.kernel.org> #4.8+ > Signed-off-by: Baoquan He <bhe@redhat.com> > Acked-by: Dave Young <dyoung@redhat.com> > Reviewed-by: Bhupesh Sharma <bhsharma@redhat.com> > Acked-by: Thomas Garnier <thgarnie@google.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > 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: Borislav Petkov <bp@alien8.de> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Masahiro Yamada <yamada.masahiro@socionext.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 > Thanks Dave ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 RESEND 1/2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization 2017-03-24 2:29 ` Dave Young @ 2017-03-24 3:05 ` Dave Young 2017-03-24 4:35 ` Baoquan He 1 sibling, 0 replies; 5+ messages in thread From: Dave Young @ 2017-03-24 3:05 UTC (permalink / raw) To: Baoquan He Cc: linux-kernel, stable, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Thomas Garnier, Kees Cook, Borislav Petkov, Andrew Morton, linux-efi, Masahiro Yamada 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: <stable@vger.kernel.org> #4.8+ > > Signed-off-by: Baoquan He <bhe@redhat.com> > > Acked-by: Dave Young <dyoung@redhat.com> > > Reviewed-by: Bhupesh Sharma <bhsharma@redhat.com> > > Acked-by: Thomas Garnier <thgarnie@google.com> > > Cc: Thomas Gleixner <tglx@linutronix.de> > > 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: Borislav Petkov <bp@alien8.de> > > Cc: Andrew Morton <akpm@linux-foundation.org> > > Cc: Masahiro Yamada <yamada.masahiro@socionext.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 > > > > Thanks > Dave ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 RESEND 1/2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization 2017-03-24 2:29 ` Dave Young 2017-03-24 3:05 ` Dave Young @ 2017-03-24 4:35 ` Baoquan He 1 sibling, 0 replies; 5+ messages in thread From: Baoquan He @ 2017-03-24 4:35 UTC (permalink / raw) To: Dave Young Cc: linux-kernel, stable, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Thomas Garnier, Kees Cook, Borislav Petkov, Andrew Morton, Masahiro Yamada 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. I only say the consequence from kdump point of view and point out that. Anyway I am fine w/o kexec/kdump text. Will repost this patch only without kexec-ed kernel saying. > > 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: <stable@vger.kernel.org> #4.8+ > > Signed-off-by: Baoquan He <bhe@redhat.com> > > Acked-by: Dave Young <dyoung@redhat.com> > > Reviewed-by: Bhupesh Sharma <bhsharma@redhat.com> > > Acked-by: Thomas Garnier <thgarnie@google.com> > > Cc: Thomas Gleixner <tglx@linutronix.de> > > 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: Borislav Petkov <bp@alien8.de> > > Cc: Andrew Morton <akpm@linux-foundation.org> > > Cc: Masahiro Yamada <yamada.masahiro@socionext.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 > > > > Thanks > Dave ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-24 4:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1490239655-20902-1-git-send-email-bhe@redhat.com>
2017-03-23 3:27 ` [PATCH v1 RESEND 1/2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization Baoquan He
2017-03-24 0:41 ` Baoquan He
2017-03-24 2:29 ` Dave Young
2017-03-24 3:05 ` Dave Young
2017-03-24 4:35 ` Baoquan He
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).