[PATCH v2 0/2] x86/mm/KASLR: Fix the wrong size of memory sections

[PATCH v2 0/2] x86/mm/KASLR: Fix the wrong size of memory sections

[Baoquan He]

v1->v2:
  Rewrite log of the two patches. No functionality change.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
v1 background:
The fixes for these two bugs were carried in the earlier patchset, patch
4/6 and patch 5/6:

[PATCH v4 0/6] Several patches to fix code bugs, improve documents and clean up
http://lkml.kernel.org/r/20190314094645.4883-1-bhe@redhat.com

Later, Thomas suggested posting bug fixing patches separately from those
clean up patches. So send both of them in a separate patchset.

For another bug fix patch 6/6, it happened on SGI UV system. Mike and
Frank have sent a machine with cards to our lab and loaned to me, I am
still debugging and discussing with Mike about the verification.

Baoquan He (2):
  x86/mm/KASLR: Fix the size of the direct mapping section
  x86/mm/KASLR: Fix the size of vmemmap section

 arch/x86/mm/kaslr.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

-- 
2.17.2

[PATCH v2 1/2] x86/mm/KASLR: Fix the size of the direct mapping section

[Baoquan He]

kernel_randomize_memory() uses __PHYSICAL_MASK_SHIFT to calculate
the maximum amount of system RAM supported. The size of the direct
mapping section is obtained from the smaller one of the below two
values:

 (actual system RAM size + padding size) vs (max system RAM size supported)

This calculation is wrong since commit:
b83ce5ee91471d ("x86/mm/64: Make __PHYSICAL_MASK_SHIFT always 52").

In commit b83ce5ee91471d, __PHYSICAL_MASK_SHIFT was changed to be 52,
regardless of whether it's using 4-level or 5-level page tables.
It will always use 4 PB as the maximum amount of system RAM, even
in 4-level paging mode where it should be 64 TB.  Thus the size of
the direct mapping section will always be the sum of the actual
system RAM size plus the padding size.

Even when the amount of system RAM is 64 TB, the following layout will
still be used. Obviously KALSR will be weakened significantly.

   |_______actual RAM_______|_padding_|______the rest_______ |
   0            64TB          74TB                    ~120TB

What we want is the following:

   |_______actual RAM_______|_________the rest_______________|
   0            64TB                                  ~120TB

So the code should use MAX_PHYSMEM_BITS instead. Fix it by replacing
__PHYSICAL_MASK_SHIFT with MAX_PHYSMEM_BITS.

Fixes: b83ce5ee9147 ("x86/mm/64: Make __PHYSICAL_MASK_SHIFT always 52")
Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reviewed-by: Thomas Garnier <thgarnie@google.com>
Signed-off-by: Baoquan He <bhe@redhat.com>
---
 arch/x86/mm/kaslr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
index 9a8756517504..387d4ed25d7c 100644
--- a/arch/x86/mm/kaslr.c
+++ b/arch/x86/mm/kaslr.c
@@ -94,7 +94,7 @@ void __init kernel_randomize_memory(void)
 	if (!kaslr_memory_enabled())
 		return;
 
-	kaslr_regions[0].size_tb = 1 << (__PHYSICAL_MASK_SHIFT - TB_SHIFT);
+	kaslr_regions[0].size_tb = 1 << (MAX_PHYSMEM_BITS - TB_SHIFT);
 	kaslr_regions[1].size_tb = VMALLOC_SIZE_TB;
 
 	/*
-- 
2.17.2

[PATCH v2 2/2] x86/mm/KASLR: Fix the size of vmemmap section

[Baoquan He]

kernel_randomize_memory() hardcodes the size of vmemmap section as 1 TB,
to support the maximum amount of system RAM in 4-level paging mode, 64 TB.

However, 1 TB is not enough in 5-level paging mode. Assuming the size of
struct page is 64 Bytes, To support 4 PB system RAM in 5-level, 64 TB of
vmemmap area is needed. The wrong hardcoding may cause vmemmap stamping
into the following cpu_entry_area section, if KASLR puts vmemmap very close
to cpu_entry_area , and the needed area of vmemmap is much bigger than
1 TB.

So here calculate the actual size of vmemmap region, then align up to 1 TB
boundary. In 4-level it's always 1 TB. In 5-level it's adjusted on demand.
The current code reserves 0.5 PB for vmemmap in 5-level. In this way, the
left space can be saved to join randomization to increase the entropy.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 arch/x86/mm/kaslr.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
index 387d4ed25d7c..4679a0075048 100644
--- a/arch/x86/mm/kaslr.c
+++ b/arch/x86/mm/kaslr.c
@@ -52,7 +52,7 @@ static __initdata struct kaslr_memory_region {
 } kaslr_regions[] = {
 	{ &page_offset_base, 0 },
 	{ &vmalloc_base, 0 },
-	{ &vmemmap_base, 1 },
+	{ &vmemmap_base, 0 },
 };
 
 /* Get size in bytes used by the memory region */
@@ -78,6 +78,7 @@ void __init kernel_randomize_memory(void)
 	unsigned long rand, memory_tb;
 	struct rnd_state rand_state;
 	unsigned long remain_entropy;
+	unsigned long vmemmap_size;
 
 	vaddr_start = pgtable_l5_enabled() ? __PAGE_OFFSET_BASE_L5 : __PAGE_OFFSET_BASE_L4;
 	vaddr = vaddr_start;
@@ -109,6 +110,14 @@ void __init kernel_randomize_memory(void)
 	if (memory_tb < kaslr_regions[0].size_tb)
 		kaslr_regions[0].size_tb = memory_tb;
 
+	/**
+	 * Calculate how many TB vmemmap region needs, and aligned to
+	 * 1TB boundary.
+	 */
+	vmemmap_size = (kaslr_regions[0].size_tb << (TB_SHIFT - PAGE_SHIFT)) *
+		sizeof(struct page);
+	kaslr_regions[2].size_tb = DIV_ROUND_UP(vmemmap_size, 1UL << TB_SHIFT);
+
 	/* Calculate entropy available between regions */
 	remain_entropy = vaddr_end - vaddr_start;
 	for (i = 0; i < ARRAY_SIZE(kaslr_regions); i++)
-- 
2.17.2

Re: [PATCH v2 0/2] x86/mm/KASLR: Fix the wrong size of memory sections

[Baoquan He]

On 04/12/19 at 02:55pm, Baoquan He wrote:
> v1->v2:
>   Rewrite log of the two patches. No functionality change.
				   ~~ Sorry, I meant no new code change.
> 
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> v1 background:
> The fixes for these two bugs were carried in the earlier patchset, patch
> 4/6 and patch 5/6:
> 
> [PATCH v4 0/6] Several patches to fix code bugs, improve documents and clean up
> http://lkml.kernel.org/r/20190314094645.4883-1-bhe@redhat.com
> 
> Later, Thomas suggested posting bug fixing patches separately from those
> clean up patches. So send both of them in a separate patchset.
> 
> For another bug fix patch 6/6, it happened on SGI UV system. Mike and
> Frank have sent a machine with cards to our lab and loaned to me, I am
> still debugging and discussing with Mike about the verification.
> 
> Baoquan He (2):
>   x86/mm/KASLR: Fix the size of the direct mapping section
>   x86/mm/KASLR: Fix the size of vmemmap section
> 
>  arch/x86/mm/kaslr.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> -- 
> 2.17.2
> 

Re: [PATCH v2 1/2] x86/mm/KASLR: Fix the size of the direct mapping section

[Baoquan He]

On 04/12/19 at 02:55pm, Baoquan He wrote:
> kernel_randomize_memory() uses __PHYSICAL_MASK_SHIFT to calculate
> the maximum amount of system RAM supported. The size of the direct
> mapping section is obtained from the smaller one of the below two
> values:
> 
>  (actual system RAM size + padding size) vs (max system RAM size supported)
> 
> This calculation is wrong since commit:
> b83ce5ee91471d ("x86/mm/64: Make __PHYSICAL_MASK_SHIFT always 52").
> 
> In commit b83ce5ee91471d, __PHYSICAL_MASK_SHIFT was changed to be 52,
> regardless of whether it's using 4-level or 5-level page tables.
> It will always use 4 PB as the maximum amount of system RAM, even
> in 4-level paging mode where it should be 64 TB.  Thus the size of
> the direct mapping section will always be the sum of the actual
> system RAM size plus the padding size.
> 
> Even when the amount of system RAM is 64 TB, the following layout will
> still be used. Obviously KALSR will be weakened significantly.
> 
>    |_______actual RAM_______|_padding_|______the rest_______ |
>    0            64TB          74TB                    ~120TB
                                ~~ I could use tab, will resend to
correct this.
> 
> What we want is the following:
> 
>    |_______actual RAM_______|_________the rest_______________|
>    0            64TB                                  ~120TB
> 
> So the code should use MAX_PHYSMEM_BITS instead. Fix it by replacing
> __PHYSICAL_MASK_SHIFT with MAX_PHYSMEM_BITS.
> 
> Fixes: b83ce5ee9147 ("x86/mm/64: Make __PHYSICAL_MASK_SHIFT always 52")
> Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Reviewed-by: Thomas Garnier <thgarnie@google.com>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>  arch/x86/mm/kaslr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
> index 9a8756517504..387d4ed25d7c 100644
> --- a/arch/x86/mm/kaslr.c
> +++ b/arch/x86/mm/kaslr.c
> @@ -94,7 +94,7 @@ void __init kernel_randomize_memory(void)
>  	if (!kaslr_memory_enabled())
>  		return;
>  
> -	kaslr_regions[0].size_tb = 1 << (__PHYSICAL_MASK_SHIFT - TB_SHIFT);
> +	kaslr_regions[0].size_tb = 1 << (MAX_PHYSMEM_BITS - TB_SHIFT);
>  	kaslr_regions[1].size_tb = VMALLOC_SIZE_TB;
>  
>  	/*
> -- 
> 2.17.2
> 

Re: [PATCH v2 0/2] x86/mm/KASLR: Fix the wrong size of memory sections

[Kees Cook]

On Fri, Apr 12, 2019 at 1:55 AM Baoquan He <bhe@redhat.com> wrote:
>
> v1->v2:
>   Rewrite log of the two patches. No functionality change.
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> v1 background:
> The fixes for these two bugs were carried in the earlier patchset, patch
> 4/6 and patch 5/6:
>
> [PATCH v4 0/6] Several patches to fix code bugs, improve documents and clean up
> http://lkml.kernel.org/r/20190314094645.4883-1-bhe@redhat.com
>
> Later, Thomas suggested posting bug fixing patches separately from those
> clean up patches. So send both of them in a separate patchset.
>
> For another bug fix patch 6/6, it happened on SGI UV system. Mike and
> Frank have sent a machine with cards to our lab and loaned to me, I am
> still debugging and discussing with Mike about the verification.
>
> Baoquan He (2):
>   x86/mm/KASLR: Fix the size of the direct mapping section
>   x86/mm/KASLR: Fix the size of vmemmap section

These fixes look sane to me. Thanks for solving this!

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

>
>  arch/x86/mm/kaslr.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> --
> 2.17.2
>


-- 
Kees Cook