[PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Baoquan He]

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. 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: linux-efi@vger.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>
Cc: Dave Young <dyoung@redhat.com>
Cc: Bhupesh Sharma <bhsharma@redhat.com>
---
v1->v2:
  No code change, just update patch log according to reviewer's comment.

 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

[PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Baoquan He]

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. 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: linux-efi@vger.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>
Cc: Dave Young <dyoung@redhat.com>
Cc: Bhupesh Sharma <bhsharma@redhat.com>
---
v1->v2:
  No code change, just update patch log according to reviewer's comment.

 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

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Ingo Molnar]

* Baoquan He <bhe@redhat.com> 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. 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: linux-efi@vger.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>
> Cc: Dave Young <dyoung@redhat.com>
> Cc: Bhupesh Sharma <bhsharma@redhat.com>

So I applied this kexec fix and extended the changelog to clearly show why this 
fix matters in practice.

Also, to make sure I understood it correctly: these addresses are all dynamic on 
64-bit kernels, i.e. we are establishing and then tearing down these page tables 
around EFI calls, and they are 'normally' not present at all, right?

Thanks,

	Ingo

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Ingo Molnar]

* Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 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. 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> #4.8+
> Signed-off-by: Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Acked-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Bhupesh Sharma <bhsharma-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Acked-by: Thomas Garnier <thgarnie-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> Cc: Ingo Molnar <mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: "H. Peter Anvin" <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
> Cc: x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
> Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Thomas Garnier <thgarnie-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
> Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> Cc: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
> Cc: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Bhupesh Sharma <bhsharma-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

So I applied this kexec fix and extended the changelog to clearly show why this 
fix matters in practice.

Also, to make sure I understood it correctly: these addresses are all dynamic on 
64-bit kernels, i.e. we are establishing and then tearing down these page tables 
around EFI calls, and they are 'normally' not present at all, right?

Thanks,

	Ingo

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Baoquan He]

On 03/24/17 at 09:08am, Ingo Molnar wrote:
> > 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: linux-efi@vger.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>
> > Cc: Dave Young <dyoung@redhat.com>
> > Cc: Bhupesh Sharma <bhsharma@redhat.com>
> 
> So I applied this kexec fix and extended the changelog to clearly show why this 
> fix matters in practice.

I thought it only impacts kexec, but Dave thought it will impact 1st
kenrel either.
> 
> Also, to make sure I understood it correctly: these addresses are all dynamic on 
> 64-bit kernels, i.e. we are establishing and then tearing down these page tables 
> around EFI calls, and they are 'normally' not present at all, right?

The EFI region is reserved for EFI runtime services virtual mapping, the
original purpose is to preserve this region so that they can be reused
by kexec-ed kernel. This was introduced by Boris in commit d2f7cbe7b26a7
("x86/efi: Runtime services virtual mapping").

So it will be establishing and stay there. According to Dave's telling,
efi will still fetch efi variables or time/date by runtime service by
switching the efi pgd and entering into efi mode. And then switch back
to normal OS. Not sure if I am right for efi part in 1st kernel. For 2nd
kernel, if want to reuse the them, the efi region has to be kept.

Thanks
Baoquan

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Baoquan He]

On 03/24/17 at 09:08am, Ingo Molnar wrote:
> > Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> #4.8+
> > Signed-off-by: Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Acked-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: Bhupesh Sharma <bhsharma-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Acked-by: Thomas Garnier <thgarnie-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> > Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> > Cc: Ingo Molnar <mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Cc: "H. Peter Anvin" <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
> > Cc: x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
> > Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: Thomas Garnier <thgarnie-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> > Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
> > Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> > Cc: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
> > Cc: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Cc: Bhupesh Sharma <bhsharma-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> 
> So I applied this kexec fix and extended the changelog to clearly show why this 
> fix matters in practice.

I thought it only impacts kexec, but Dave thought it will impact 1st
kenrel either.
> 
> Also, to make sure I understood it correctly: these addresses are all dynamic on 
> 64-bit kernels, i.e. we are establishing and then tearing down these page tables 
> around EFI calls, and they are 'normally' not present at all, right?

The EFI region is reserved for EFI runtime services virtual mapping, the
original purpose is to preserve this region so that they can be reused
by kexec-ed kernel. This was introduced by Boris in commit d2f7cbe7b26a7
("x86/efi: Runtime services virtual mapping").

So it will be establishing and stay there. According to Dave's telling,
efi will still fetch efi variables or time/date by runtime service by
switching the efi pgd and entering into efi mode. And then switch back
to normal OS. Not sure if I am right for efi part in 1st kernel. For 2nd
kernel, if want to reuse the them, the efi region has to be kept.

Thanks
Baoquan

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Dave Young]

On 03/24/17 at 04:34pm, Baoquan He wrote:
> On 03/24/17 at 09:08am, Ingo Molnar wrote:
> > > 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: linux-efi@vger.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>
> > > Cc: Dave Young <dyoung@redhat.com>
> > > Cc: Bhupesh Sharma <bhsharma@redhat.com>
> > 
> > So I applied this kexec fix and extended the changelog to clearly show why this 
> > fix matters in practice.
> 
> I thought it only impacts kexec, but Dave thought it will impact 1st
> kenrel either.

Yes, I think no need to mention kexec, it is a general issue.

First, the space is reserved for EFI, so kernel should not use it for
kaslr. Second, EFI page tables sync the low kernel page tables into its
own page tables, if others use this space for non-EFI then those part
will be missing.

arch/x86/platform/efi/efi_64.c
efi_sync_low_kernel_mappings() is syncing kernel page tables to efi's.
>From the function comment below:
Add low kernel mappings for passing arguments to EFI functions.

Suppose some arguments use kaslr randomized address which is within efi
ranges then we will hit bugs. But we do not see actual bug reports in
real world yet. This is found during patch review.

Anyway, since this area is EFI reserved, no reason to add it to kaslr
pool.

> > 
> > Also, to make sure I understood it correctly: these addresses are all dynamic on 
> > 64-bit kernels, i.e. we are establishing and then tearing down these page tables 
> > around EFI calls, and they are 'normally' not present at all, right?
> 
> The EFI region is reserved for EFI runtime services virtual mapping, the
> original purpose is to preserve this region so that they can be reused
> by kexec-ed kernel. This was introduced by Boris in commit d2f7cbe7b26a7
> ("x86/efi: Runtime services virtual mapping").
> 
> So it will be establishing and stay there. According to Dave's telling,
> efi will still fetch efi variables or time/date by runtime service by
> switching the efi pgd and entering into efi mode. And then switch back
> to normal OS. Not sure if I am right for efi part in 1st kernel. For 2nd
> kernel, if want to reuse the them, the efi region has to be kept.
> 
> Thanks
> Baoquan

Thanks
Dave

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Dave Young]

On 03/24/17 at 04:34pm, Baoquan He wrote:
> On 03/24/17 at 09:08am, Ingo Molnar wrote:
> > > Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> #4.8+
> > > Signed-off-by: Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > Acked-by: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > Reviewed-by: Bhupesh Sharma <bhsharma-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > Acked-by: Thomas Garnier <thgarnie-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> > > Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> > > Cc: Ingo Molnar <mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > Cc: "H. Peter Anvin" <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
> > > Cc: x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
> > > Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > Cc: Thomas Garnier <thgarnie-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> > > Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > > Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
> > > Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> > > Cc: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
> > > Cc: Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > Cc: Bhupesh Sharma <bhsharma-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > 
> > So I applied this kexec fix and extended the changelog to clearly show why this 
> > fix matters in practice.
> 
> I thought it only impacts kexec, but Dave thought it will impact 1st
> kenrel either.

Yes, I think no need to mention kexec, it is a general issue.

First, the space is reserved for EFI, so kernel should not use it for
kaslr. Second, EFI page tables sync the low kernel page tables into its
own page tables, if others use this space for non-EFI then those part
will be missing.

arch/x86/platform/efi/efi_64.c
efi_sync_low_kernel_mappings() is syncing kernel page tables to efi's.
>From the function comment below:
Add low kernel mappings for passing arguments to EFI functions.

Suppose some arguments use kaslr randomized address which is within efi
ranges then we will hit bugs. But we do not see actual bug reports in
real world yet. This is found during patch review.

Anyway, since this area is EFI reserved, no reason to add it to kaslr
pool.

> > 
> > Also, to make sure I understood it correctly: these addresses are all dynamic on 
> > 64-bit kernels, i.e. we are establishing and then tearing down these page tables 
> > around EFI calls, and they are 'normally' not present at all, right?
> 
> The EFI region is reserved for EFI runtime services virtual mapping, the
> original purpose is to preserve this region so that they can be reused
> by kexec-ed kernel. This was introduced by Boris in commit d2f7cbe7b26a7
> ("x86/efi: Runtime services virtual mapping").
> 
> So it will be establishing and stay there. According to Dave's telling,
> efi will still fetch efi variables or time/date by runtime service by
> switching the efi pgd and entering into efi mode. And then switch back
> to normal OS. Not sure if I am right for efi part in 1st kernel. For 2nd
> kernel, if want to reuse the them, the efi region has to be kept.
> 
> Thanks
> Baoquan

Thanks
Dave

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Ingo Molnar]

* Dave Young <dyoung@redhat.com> wrote:

> > > So I applied this kexec fix and extended the changelog to clearly show why 
> > > this fix matters in practice.
> > 
> > I thought it only impacts kexec, but Dave thought it will impact 1st kenrel 
> > either.
> 
> Yes, I think no need to mention kexec, it is a general issue.
> 
> First, the space is reserved for EFI, so kernel should not use it for kaslr. 

It's the kernel's EFI code, and we map whatever address we want (and then pass 
that to the EFI runtime), so wether it's randomized or not is the Linux kernel's 
policy decision...

So that's my question: can these memory regions include security sensitive data, 
and if yes, how can we best randomize it while kexec and other kernel and EFI 
features still work?

Preserving virtual addresses for kexec is a red herring: the randomized offset 
could be passed to the kexec-ed kernel just fine.

Thanks,

	Ingo

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Borislav Petkov]

On Fri, Mar 24, 2017 at 10:24:34AM +0100, Ingo Molnar wrote:
> Preserving virtual addresses for kexec is a red herring: the randomized offset 
> could be passed to the kexec-ed kernel just fine.

Not only that - kexec'ed kernel gets the addresses from sysfs so we can randomize.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Ard Biesheuvel]

On 24 March 2017 at 09:24, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Dave Young <dyoung@redhat.com> wrote:
>
>> > > So I applied this kexec fix and extended the changelog to clearly show why
>> > > this fix matters in practice.
>> >
>> > I thought it only impacts kexec, but Dave thought it will impact 1st kenrel
>> > either.
>>
>> Yes, I think no need to mention kexec, it is a general issue.
>>
>> First, the space is reserved for EFI, so kernel should not use it for kaslr.
>
> It's the kernel's EFI code, and we map whatever address we want (and then pass
> that to the EFI runtime), so wether it's randomized or not is the Linux kernel's
> policy decision...
>

No. It is the firmware's EFI code, and the virtual translation applied
by the OS is made known to the firmware by means of a call into the
runtime service SetVirtualAddressMap(). This service can only be
called once after each boot, and so kexec kernels are forced to use
the same VA mapping for runtime services as the first kernel. This is
the whole point of having a VA region reserved for this, so that kexec
kernels are guaranteed to be able to use the same VA mapping.

> So that's my question: can these memory regions include security sensitive data,
> and if yes, how can we best randomize it while kexec and other kernel and EFI
> features still work?
>
> Preserving virtual addresses for kexec is a red herring: the randomized offset
> could be passed to the kexec-ed kernel just fine.
>
> Thanks,
>
>         Ingo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Ard Biesheuvel]

On 24 March 2017 at 09:24, Ingo Molnar <mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
> * Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
>> > > So I applied this kexec fix and extended the changelog to clearly show why
>> > > this fix matters in practice.
>> >
>> > I thought it only impacts kexec, but Dave thought it will impact 1st kenrel
>> > either.
>>
>> Yes, I think no need to mention kexec, it is a general issue.
>>
>> First, the space is reserved for EFI, so kernel should not use it for kaslr.
>
> It's the kernel's EFI code, and we map whatever address we want (and then pass
> that to the EFI runtime), so wether it's randomized or not is the Linux kernel's
> policy decision...
>

No. It is the firmware's EFI code, and the virtual translation applied
by the OS is made known to the firmware by means of a call into the
runtime service SetVirtualAddressMap(). This service can only be
called once after each boot, and so kexec kernels are forced to use
the same VA mapping for runtime services as the first kernel. This is
the whole point of having a VA region reserved for this, so that kexec
kernels are guaranteed to be able to use the same VA mapping.

> So that's my question: can these memory regions include security sensitive data,
> and if yes, how can we best randomize it while kexec and other kernel and EFI
> features still work?
>
> Preserving virtual addresses for kexec is a red herring: the randomized offset
> could be passed to the kexec-ed kernel just fine.
>
> Thanks,
>
>         Ingo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Borislav Petkov]

On Fri, Mar 24, 2017 at 09:37:36AM +0000, Ard Biesheuvel wrote:
> No. It is the firmware's EFI code, and the virtual translation applied
> by the OS is made known to the firmware by means of a call into the
> runtime service SetVirtualAddressMap().

We can still randomize within those 64G before calling
SetVirtualAddressMap(). The question is, do we want to or need to, even?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Ard Biesheuvel]

On 24 March 2017 at 09:40, Borislav Petkov <bp@alien8.de> wrote:
> On Fri, Mar 24, 2017 at 09:37:36AM +0000, Ard Biesheuvel wrote:
>> No. It is the firmware's EFI code, and the virtual translation applied
>> by the OS is made known to the firmware by means of a call into the
>> runtime service SetVirtualAddressMap().
>
> We can still randomize within those 64G before calling
> SetVirtualAddressMap(). The question is, do we want to or need to, even?
>

That is a different matter. If the regions are only mapped while
runtime services invocations are in progress (as we do on ARM), I am
not sure if it matters that much, given how rarely that occurs in
normal use.

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Borislav Petkov]

On Fri, Mar 24, 2017 at 09:42:40AM +0000, Ard Biesheuvel wrote:
> That is a different matter. If the regions are only mapped while
> runtime services invocations are in progress (as we do on ARM), I am
> not sure if it matters that much, given how rarely that occurs in
> normal use.

Question is, is there anything worth protecting with ASLR or we don't
care? I wanna say, we should randomize just in case, especially as it
shouldn't be that expensive to do.

Also, how does the whole EFI-in-the-kexec-ed-kernel work on ARM? Runtime
services get mapped on-demand in the kexec-ed kernel too?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Borislav Petkov]

On Fri, Mar 24, 2017 at 09:42:40AM +0000, Ard Biesheuvel wrote:
> That is a different matter. If the regions are only mapped while
> runtime services invocations are in progress (as we do on ARM), I am
> not sure if it matters that much, given how rarely that occurs in
> normal use.

Question is, is there anything worth protecting with ASLR or we don't
care? I wanna say, we should randomize just in case, especially as it
shouldn't be that expensive to do.

Also, how does the whole EFI-in-the-kexec-ed-kernel work on ARM? Runtime
services get mapped on-demand in the kexec-ed kernel too?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Ard Biesheuvel]

On 24 March 2017 at 09:46, Borislav Petkov <bp@alien8.de> wrote:
> On Fri, Mar 24, 2017 at 09:42:40AM +0000, Ard Biesheuvel wrote:
>> That is a different matter. If the regions are only mapped while
>> runtime services invocations are in progress (as we do on ARM), I am
>> not sure if it matters that much, given how rarely that occurs in
>> normal use.
>
> Question is, is there anything worth protecting with ASLR or we don't
> care? I wanna say, we should randomize just in case, especially as it
> shouldn't be that expensive to do.
>

Well, given that in many cases, these pages are mapped R+W+X, I would
say that there is a risk involved in having these data structures at
fixed offsets.

Since UEFI v2.6, we have a new firmware table that describes strict
permission attributes for these regions, so everything can be mapped
writable or executable but never both. (Sai wired up the support for
this for x86 in v4.10)

> Also, how does the whole EFI-in-the-kexec-ed-kernel work on ARM? Runtime
> services get mapped on-demand in the kexec-ed kernel too?
>

Yes. On ARM, we use an ordinary mm_struct and just do a switch_mm()
with preemption disabled. So there is no need to reserve kernel VA
ranges, all UEFI runtime mappings are in the user area.

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Ard Biesheuvel]

On 24 March 2017 at 09:46, Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org> wrote:
> On Fri, Mar 24, 2017 at 09:42:40AM +0000, Ard Biesheuvel wrote:
>> That is a different matter. If the regions are only mapped while
>> runtime services invocations are in progress (as we do on ARM), I am
>> not sure if it matters that much, given how rarely that occurs in
>> normal use.
>
> Question is, is there anything worth protecting with ASLR or we don't
> care? I wanna say, we should randomize just in case, especially as it
> shouldn't be that expensive to do.
>

Well, given that in many cases, these pages are mapped R+W+X, I would
say that there is a risk involved in having these data structures at
fixed offsets.

Since UEFI v2.6, we have a new firmware table that describes strict
permission attributes for these regions, so everything can be mapped
writable or executable but never both. (Sai wired up the support for
this for x86 in v4.10)

> Also, how does the whole EFI-in-the-kexec-ed-kernel work on ARM? Runtime
> services get mapped on-demand in the kexec-ed kernel too?
>

Yes. On ARM, we use an ordinary mm_struct and just do a switch_mm()
with preemption disabled. So there is no need to reserve kernel VA
ranges, all UEFI runtime mappings are in the user area.

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Ingo Molnar]

* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> No. It is the firmware's EFI code, and the virtual translation applied by the OS 
> is made known to the firmware by means of a call into the runtime service 
> SetVirtualAddressMap(). This service can only be called once after each boot, 
> and so kexec kernels are forced to use the same VA mapping for runtime services 
> as the first kernel. This is the whole point of having a VA region reserved for 
> this, so that kexec kernels are guaranteed to be able to use the same VA 
> mapping.

Yes, but it's the kernel's EFI code that determines the area! So my suggestion:

> > Preserving virtual addresses for kexec is a red herring: the randomized offset 
> > could be passed to the kexec-ed kernel just fine.

Would solve the kexec problem, right?

I.e. the first kernel that boots randomizes the address range - and passes that 
offset off to any subsequent kernels.

Turning KASLR off actively degrades that randomization of the kernel virtual 
addresses.

Am I missing anything?

Thanks,

	Ingo

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Ingo Molnar]

* Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:

> No. It is the firmware's EFI code, and the virtual translation applied by the OS 
> is made known to the firmware by means of a call into the runtime service 
> SetVirtualAddressMap(). This service can only be called once after each boot, 
> and so kexec kernels are forced to use the same VA mapping for runtime services 
> as the first kernel. This is the whole point of having a VA region reserved for 
> this, so that kexec kernels are guaranteed to be able to use the same VA 
> mapping.

Yes, but it's the kernel's EFI code that determines the area! So my suggestion:

> > Preserving virtual addresses for kexec is a red herring: the randomized offset 
> > could be passed to the kexec-ed kernel just fine.

Would solve the kexec problem, right?

I.e. the first kernel that boots randomizes the address range - and passes that 
offset off to any subsequent kernels.

Turning KASLR off actively degrades that randomization of the kernel virtual 
addresses.

Am I missing anything?

Thanks,

	Ingo

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Ard Biesheuvel]

On 24 March 2017 at 10:36, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> No. It is the firmware's EFI code, and the virtual translation applied by the OS
>> is made known to the firmware by means of a call into the runtime service
>> SetVirtualAddressMap(). This service can only be called once after each boot,
>> and so kexec kernels are forced to use the same VA mapping for runtime services
>> as the first kernel. This is the whole point of having a VA region reserved for
>> this, so that kexec kernels are guaranteed to be able to use the same VA
>> mapping.
>
> Yes, but it's the kernel's EFI code that determines the area!

Indeed. It seems I misunderstood you there.

There are some known limitations, though, which prevent us from using
userland mappings on x86 like we do on ARM (Macs don't support it),
but I don't think randomizing the mappings inside this 64 GB window is
going to trigger any latent firmware bugs.

> So my suggestion:
>
>> > Preserving virtual addresses for kexec is a red herring: the randomized offset
>> > could be passed to the kexec-ed kernel just fine.
>
> Would solve the kexec problem, right?
>
> I.e. the first kernel that boots randomizes the address range - and passes that
> offset off to any subsequent kernels.
>

Yes, that sounds feasible to me.

> Turning KASLR off actively degrades that randomization of the kernel virtual
> addresses.
>
> Am I missing anything?
>

No, I think you are right. UEFI runtime services region are likely to
consist of R+W+X mappings for the foreseeable future on x86, and the
more we tighten down security in other places, the more appealing the
UEFI regions become for exploitation (even if they are only mapped
while runtime services calls are in progress).

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Ingo Molnar]

* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> > Turning KASLR off actively degrades that randomization of the kernel virtual 
> > addresses.
> >
> > Am I missing anything?
> >
> 
> No, I think you are right. UEFI runtime services region are likely to consist of 
> R+W+X mappings for the foreseeable future on x86, and the more we tighten down 
> security in other places, the more appealing the UEFI regions become for 
> exploitation (even if they are only mapped while runtime services calls are in 
> progress).

Ok, so I'm fine with the current proposed patch as a temporary workaround, but 
only if we are going to get a real fix as well, ASAP.

Thanks,

	Ingo

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Baoquan He]

On 03/24/17 at 10:24am, Ingo Molnar wrote:
> 
> * Dave Young <dyoung@redhat.com> wrote:
> 
> > > > So I applied this kexec fix and extended the changelog to clearly show why 
> > > > this fix matters in practice.
> > > 
> > > I thought it only impacts kexec, but Dave thought it will impact 1st kenrel 
> > > either.
> > 
> > Yes, I think no need to mention kexec, it is a general issue.
> > 
> > First, the space is reserved for EFI, so kernel should not use it for kaslr. 
> 
> It's the kernel's EFI code, and we map whatever address we want (and then pass 
> that to the EFI runtime), so wether it's randomized or not is the Linux kernel's 
> policy decision...
> 
> So that's my question: can these memory regions include security sensitive data, 
> and if yes, how can we best randomize it while kexec and other kernel and EFI 
> features still work?

It doesn't corrupt kernel possibly because:
1) there's espfixup stack region between them if CONFIG_X86_ESPFIX64 is
enabled, then the upper limit of kaslr VA space for randomization could be
ESPFIX_BASE_ADDR. Fedora default to enable ESPFIX_BASE_ADDR.

ffffe90000000000 - ffffe9ffffffffff (=40 bits) hole
ffffea0000000000 - ffffeaffffffffff (=40 bits) virtual memory map (1TB)
... unused hole ...
ffffec0000000000 - fffffbffffffffff (=44 bits) kasan shadow memory (16TB)
... unused hole ...
ffffff0000000000 - ffffff7fffffffff (=39 bits) %esp fixup stacks                                                                                 
... unused hole ...
ffffffef00000000 - fffffffeffffffff (=64 GB) EFI region mapping space

2) Though ESPFIX_BASE_ADDR is disabled, efi region is included into
kaslr mm va space, efi is allocated top-down in 64G, while vmemmap is 1T
large and allocated bottom-up, it rarely used up 1TB space to map struct
page.

3) most of the time, it might not randomize vmemmap to occupy the efi
region.

So up to now, no corruption is heard about this bug.

Thanks
Baoquan

Re: [PATCH v2] x86/mm/KASLR: EFI region is mistakenly included into KASLR VA space for randomization

[Dave Young]

On 03/24/17 at 09:08am, Ingo Molnar wrote:
> 
> * Baoquan He <bhe@redhat.com> 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. 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: linux-efi@vger.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>
> > Cc: Dave Young <dyoung@redhat.com>
> > Cc: Bhupesh Sharma <bhsharma@redhat.com>
> 
> So I applied this kexec fix and extended the changelog to clearly show why this 
> fix matters in practice.
> 
> Also, to make sure I understood it correctly: these addresses are all dynamic on 
> 64-bit kernels, i.e. we are establishing and then tearing down these page tables 
> around EFI calls, and they are 'normally' not present at all, right?

Ingo, if I understand the question right "these addresses" means EFI va addresses
then it is right, EFI switch to its own page tables, so they are not
present in kernel page tables.

> 
> Thanks,
> 
> 	Ingo

[tip:x86/urgent] x86/mm/KASLR: Exclude EFI region from KASLR VA space randomization

[tip-bot for Baoquan He]

Commit-ID:  a46f60d76004965e5669dbf3fc21ef3bc3632eb4
Gitweb:     http://git.kernel.org/tip/a46f60d76004965e5669dbf3fc21ef3bc3632eb4
Author:     Baoquan He <bhe@redhat.com>
AuthorDate: Fri, 24 Mar 2017 12:59:52 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 24 Mar 2017 09:04:27 +0100

x86/mm/KASLR: Exclude EFI region from KASLR VA space randomization

Currently KASLR is enabled on three regions: the direct mapping of physical
memory, vamlloc and vmemmap. However the 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.

(This breaks kexec and possibly other things that rely on stable addresses.)

The EFI region is reserved for EFI runtime services virtual mapping which
should not be included in KASLR ranges. In Documentation/x86/x86_64/mm.txt,
we can see:

  ffffffef00000000 - fffffffeffffffff (=64 GB) EFI region mapping space

EFI uses 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.

Signed-off-by: Baoquan He <bhe@redhat.com>
Reviewed-by: Bhupesh Sharma <bhsharma@redhat.com>
Acked-by: Dave Young <dyoung@redhat.com>
Acked-by: Thomas Garnier <thgarnie@google.com>
Cc: <stable@vger.kernel.org> #4.8+
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/1490331592-31860-1-git-send-email-bhe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 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);