x86, mm: set NX across entire PMD at boot

PATCH Superseded Kees Cook 4 participants 4 messages Nov 14, 2014 – Nov 18, 2014

Subsystems (2)

x86 x86 MM

lore

Share & tools

lore

Kees Cook

author

2014-11-14 19:48 UTC #0 lore raw

PATCH 1/1 x86, mm: set NX across entire PMD at boot

When setting up permissions on kernel memory at boot, the end of the PMD that was split from bss remained executable. It should be NX like the rest. This performs a PMD alignment instead of a PAGE alignment to get the correct span of memory.

Before:
---[ High Kernel Mapping ]---
...

0xffffffff8202d000-0xffffffff82200000  1868K     RW       GLB NX pte
0xffffffff82200000-0xffffffff82c00000    10M     RW   PSE GLB NX pmd
0xffffffff82c00000-0xffffffff82df5000  2004K     RW       GLB NX pte
0xffffffff82df5000-0xffffffff82e00000    44K     RW       GLB x  pte

0xffffffff82e00000-0xffffffffc0000000 978M pmd

After:
---[ High Kernel Mapping ]---
...
0xffffffff8202d000-0xffffffff82200000 1868K RW GLB NX pte
0xffffffff82200000-0xffffffff82e00000 12M RW PSE GLB NX pmd
0xffffffff82e00000-0xffffffffc0000000 978M pmd

Signed-off-by Kees Cook <[email protected]>

---
 arch/x86/mm/init_64.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

arch/x86/mm/init_64.c +3 -1

index 4cb8763868fc..7da7a4ab46f7 100644
@@ -1123,7 +1123,9 @@ void mark_rodata_ro(void)
 	unsigned long end = (unsigned long) &__end_rodata_hpage_align;
 	unsigned long text_end = PFN_ALIGN(&__stop___ex_table);
 	unsigned long rodata_end = PFN_ALIGN(&__end_rodata);
-	unsigned long all_end = PFN_ALIGN(&_end);
+	/* End of kernel memory will span a PMD, so align to PMD. */
+	unsigned long all_end = (((unsigned long)(&_end) + (PMD_SIZE - 1))
+				 & PMD_MASK);
 
 	printk(KERN_INFO "Write protecting the kernel read-only data: %luk\n",
 	       (end - start) >> 10);

Signature · 6 lines

1.9.1


-- 
Kees Cook
Chrome OS Security

Thomas Gleixner

2014-11-14 20:12 UTC #1 lore raw

Quoted 31 lines · Kees Cook

When setting up permissions on kernel memory at boot, the end of the PMD that was split from bss remained executable. It should be NX like the rest. This performs a PMD alignment instead of a PAGE alignment to get the correct span of memory.
Before:
---[ High Kernel Mapping ]---
...
0xffffffff8202d000-0xffffffff82200000  1868K     RW       GLB NX pte
0xffffffff82200000-0xffffffff82c00000    10M     RW   PSE GLB NX pmd
0xffffffff82c00000-0xffffffff82df5000  2004K     RW       GLB NX pte
0xffffffff82df5000-0xffffffff82e00000    44K     RW       GLB x  pte
0xffffffff82e00000-0xffffffffc0000000 978M pmd
After:
---[ High Kernel Mapping ]---
...
0xffffffff8202d000-0xffffffff82200000 1868K RW GLB NX pte
0xffffffff82200000-0xffffffff82e00000 12M RW PSE GLB NX pmd
0xffffffff82e00000-0xffffffffc0000000 978M pmd
Signed-off-by Kees Cook <[email protected]>
---
 arch/x86/mm/init_64.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
arch/x86/mm/init_64.c +0 -0
index 4cb8763868fc..7da7a4ab46f7 100644

arch/x86/mm/init_64.c void mark_rodata_ro(void) · line 1128

Quoted 7 lines

arch/x86/mm/init_64.c +3 -1

@@ -1123,7 +1123,9 @@ void mark_rodata_ro(void)
 	unsigned long end = (unsigned long) &__end_rodata_hpage_align;
 	unsigned long text_end = PFN_ALIGN(&__stop___ex_table);
 	unsigned long rodata_end = PFN_ALIGN(&__end_rodata);
-	unsigned long all_end = PFN_ALIGN(&_end);
+	/* End of kernel memory will span a PMD, so align to PMD. */
+	unsigned long all_end = (((unsigned long)(&_end) + (PMD_SIZE - 1))
+				 & PMD_MASK);

I prefer to free the leftover pages like we do with the init sections. In the above example it's only 44k, but it can be 2044k in the worst case, which was enough to boot a embedded box 15 years ago :)

Thanks,

tglx

tip-bot for Kees Cook

merge bot

2014-11-18 17:41 UTC #2 lore raw

[tip:x86/urgent] x86, mm: Set NX across entire PMD at boot

In reply to Kees Cook

x86, mm: Set NX across entire PMD at boot

Commit-ID:  45e2a9d4701d8c624d4a4bcdd1084eae31e92f58
Gitweb:     http://git.kernel.org/tip/45e2a9d4701d8c624d4a4bcdd1084eae31e92f58
Author:     Kees Cook <[email protected]>
AuthorDate: Fri, 14 Nov 2014 11:47:37 -0800
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 18 Nov 2014 18:32:24 +0100

x86, mm: Set NX across entire PMD at boot

Before:
---[ High Kernel Mapping ]---
...

0xffffffff8202d000-0xffffffff82200000  1868K     RW       GLB NX pte
0xffffffff82200000-0xffffffff82c00000    10M     RW   PSE GLB NX pmd
0xffffffff82c00000-0xffffffff82df5000  2004K     RW       GLB NX pte
0xffffffff82df5000-0xffffffff82e00000    44K     RW       GLB x  pte

0xffffffff82e00000-0xffffffffc0000000 978M pmd

[ tglx: Changed it to roundup(_brk_end, PMD_SIZE) and added a comment.
We really should unmap the reminder along with the holes caused by init,initdata etc. but thats a different issue ]

Signed-off-by Kees Cook <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Toshi Kani <[email protected]>

Cc Yasuaki Ishimatsu <[email protected]>

Cc David Vrabel <[email protected]>

Cc Wang Nan <[email protected]>

Cc Yinghai Lu <[email protected]>

Cc [email protected]

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Thomas Gleixner <[email protected]>

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

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

index 4cb8763..4e5dfec 100644
@@ -1123,7 +1123,7 @@ void mark_rodata_ro(void)
 	unsigned long end = (unsigned long) &__end_rodata_hpage_align;
 	unsigned long text_end = PFN_ALIGN(&__stop___ex_table);
 	unsigned long rodata_end = PFN_ALIGN(&__end_rodata);
-	unsigned long all_end = PFN_ALIGN(&_end);
+	unsigned long all_end;
 
 	printk(KERN_INFO "Write protecting the kernel read-only data: %luk\n",
 	       (end - start) >> 10);

@@ -1134,7 +1134,16 @@ void mark_rodata_ro(void)
 	/*
 	 * The rodata/data/bss/brk section (but not the kernel text!)
 	 * should also be not-executable.
+	 *
+	 * We align all_end to PMD_SIZE because the existing mapping
+	 * is a full PMD. If we would align _brk_end to PAGE_SIZE we
+	 * split the PMD and the reminder between _brk_end and the end
+	 * of the PMD will remain mapped executable.
+	 *
+	 * Any PMD which was setup after the one which covers _brk_end
+	 * has been zapped already via cleanup_highmem().
 	 */
+	all_end = roundup((unsigned long)_brk_end, PMD_SIZE);
 	set_memory_nx(rodata_start, (all_end - rodata_start) >> PAGE_SHIFT);
 
 	rodata_test();

Yinghai Lu

2014-11-18 17:56 UTC #3 lore raw

Re: [tip:x86/urgent] x86, mm: Set NX across entire PMD at boot

Quoted 63 lines · tip-bot for Kees Cook

Commit-ID:  45e2a9d4701d8c624d4a4bcdd1084eae31e92f58
Gitweb:     http://git.kernel.org/tip/45e2a9d4701d8c624d4a4bcdd1084eae31e92f58
Author:     Kees Cook <[email protected]>
AuthorDate: Fri, 14 Nov 2014 11:47:37 -0800
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 18 Nov 2014 18:32:24 +0100
x86, mm: Set NX across entire PMD at boot
When setting up permissions on kernel memory at boot, the end of the PMD that was split from bss remained executable. It should be NX like the rest. This performs a PMD alignment instead of a PAGE alignment to get the correct span of memory.
Before:
---[ High Kernel Mapping ]---
...
0xffffffff8202d000-0xffffffff82200000  1868K     RW       GLB NX pte
0xffffffff82200000-0xffffffff82c00000    10M     RW   PSE GLB NX pmd
0xffffffff82c00000-0xffffffff82df5000  2004K     RW       GLB NX pte
0xffffffff82df5000-0xffffffff82e00000    44K     RW       GLB x  pte
0xffffffff82e00000-0xffffffffc0000000 978M pmd
After:
---[ High Kernel Mapping ]---
...
0xffffffff8202d000-0xffffffff82200000 1868K RW GLB NX pte
0xffffffff82200000-0xffffffff82e00000 12M RW PSE GLB NX pmd
0xffffffff82e00000-0xffffffffc0000000 978M pmd
[ tglx: Changed it to roundup(_brk_end, PMD_SIZE) and added a comment.
We really should unmap the reminder along with the holes caused by init,initdata etc. but thats a different issue ]
Signed-off-by Kees Cook <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Toshi Kani <[email protected]>
Cc Yasuaki Ishimatsu <[email protected]>
Cc David Vrabel <[email protected]>
Cc Wang Nan <[email protected]>
Cc Yinghai Lu <[email protected]>
Cc [email protected]
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Thomas Gleixner <[email protected]>
---
 arch/x86/mm/init_64.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
arch/x86/mm/init_64.c +1 -1
index 4cb8763..4e5dfec 100644
@@ -1123,7 +1123,7 @@ void mark_rodata_ro(void)
        unsigned long end = (unsigned long) &__end_rodata_hpage_align;
        unsigned long text_end = PFN_ALIGN(&__stop___ex_table);
        unsigned long rodata_end = PFN_ALIGN(&__end_rodata);
-       unsigned long all_end = PFN_ALIGN(&_end);
+       unsigned long all_end;
 
        printk(KERN_INFO "Write protecting the kernel read-only data: %luk\n",
               (end - start) >> 10);
@@ -1134,7 +1134,16 @@ void mark_rodata_ro(void)
        /*

arch/x86/mm/init_64.c void mark_rodata_ro(void) · line 1144

Quoted 10 lines

arch/x86/mm/init_64.c +8 -0

         * The rodata/data/bss/brk section (but not the kernel text!)
         * should also be not-executable.
+        *
+        * We align all_end to PMD_SIZE because the existing mapping
+        * is a full PMD. If we would align _brk_end to PAGE_SIZE we
+        * split the PMD and the reminder between _brk_end and the end
+        * of the PMD will remain mapped executable.
+        *
+        * Any PMD which was setup after the one which covers _brk_end
+        * has been zapped already via cleanup_highmem().

should be cleanup_highmap()

arch/x86/mm/init_64.c void mark_rodata_ro(void) · line 1146

Quoted 2 lines

arch/x86/mm/init_64.c +1 -0

         */
+       all_end = roundup((unsigned long)_brk_end, PMD_SIZE);

Why do you need cast here ?

Quoted 3 lines

arch/x86/mm/init_64.c +0 -0

        set_memory_nx(rodata_start, (all_end - rodata_start) >> PAGE_SHIFT);
 
        rodata_test();