x86/mm/KASLR: Fix the wrong size of memory sections

PATCH Superseded Baoquan He 2 participants 6 messages Apr 12, 2019 – Apr 15, 2019

Subsystems (2)

x86 x86 MM

lore

Share & tools

lore

Baoquan He

author

2019-04-12 06:55 UTC #0 lore raw

v2 cover x86/mm/KASLR: Fix the wrong size of memory sections

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/[email protected]

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(-)

Signature · 1 line

2.17.2

Baoquan He

author

2019-04-12 06:56 UTC #1 lore raw

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

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 <[email protected]>

Reviewed-by Thomas Garnier <[email protected]>

Signed-off-by Baoquan He <[email protected]>

---
 arch/x86/mm/kaslr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/x86/mm/kaslr.c +1 -1

index 9a8756517504..387d4ed25d7c 100644
@@ -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;
 
 	/*

Signature · 1 line

2.17.2

Baoquan He

author

2019-04-14 07:02 UTC #2 lore raw

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

Quoted 17 lines · 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.

Quoted 5 lines

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.

Quoted 33 lines

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 <[email protected]>
Reviewed-by Thomas Garnier <[email protected]>
Signed-off-by Baoquan He <[email protected]>
---
 arch/x86/mm/kaslr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/mm/kaslr.c +1 -1
index 9a8756517504..387d4ed25d7c 100644
@@ -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;
 
 	/*
Signature · 1 line
2.17.2

Baoquan He

author

2019-04-12 06:56 UTC #3 lore raw

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

In reply to 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 <[email protected]>

---
 arch/x86/mm/kaslr.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

arch/x86/mm/kaslr.c +10 -1

index 387d4ed25d7c..4679a0075048 100644
@@ -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++)

Signature · 1 line

2.17.2

Baoquan He

author

2019-04-12 07:03 UTC #4 lore raw

In reply to Baoquan He

Quoted 2 lines · Baoquan He

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

~~ Sorry, I meant no new code change.

Quoted 26 lines

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
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/[email protected]
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(-)
Signature · 1 line
2.17.2

Kees Cook

Reviewed-by on the v2 series

2019-04-15 14:40 UTC #5 lore raw

In reply to Baoquan He

Quoted 15 lines · 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/[email protected]
Later, Thomas suggested posting bug fixing patches separately from those clean up patches. So send both of them in a separate patchset.

Quoted 7 lines

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 <[email protected]>

-Kees

Quoted 7 lines

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

--
2.17.2

Signature · 1 line

Kees Cook