[PATCH V6] x86: NX protection for kernel data

[PATCH V6] x86: NX protection for kernel data

[Siarhei Liakh]

This patch expands functionality of CONFIG_DEBUG_RODATA to set main
(static) kernel data area as NX.
The following steps are taken to achieve this:
1. Linker script is adjusted so .text always starts and ends on a page boundary
2. Linker script is adjusted so .rodata and .data always start and
end on a page boundary
3. void mark_nxdata_nx(void) added to arch/x86/mm/init.c with actual
functionality: NX is set for all pages from _etext through _end.
4. mark_nxdata_nx() called from free_initmem() (after init has been released)
5. free_init_pages() sets released memory NX in arch/x86/mm/init.c

The results of patch application may be observed in the diff of kernel page
table dumps:
--- data_nx_pt_before.txt	2009-10-13 07:48:59.000000000 -0400
+++ data_nx_pt_after.txt	2009-10-13 07:26:46.000000000 -0400
@@ -2,8 +2,9 @@
 0x00000000-0xc0000000           3G                           pmd
 ---[ Kernel Mapping ]---
 0xc0000000-0xc0100000           1M     RW             GLB x  pte
-0xc0100000-0xc048d000        3636K     ro             GLB x  pte
-0xc048d000-0xc0600000        1484K     RW             GLB x  pte
+0xc0100000-0xc0381000        2564K     ro             GLB x  pte
+0xc0381000-0xc048d000        1072K     ro             GLB NX pte
+0xc048d000-0xc0600000        1484K     RW             GLB NX pte
 0xc0600000-0xf7800000         882M     RW         PSE GLB NX pmd
 0xf7800000-0xf79fe000        2040K     RW             GLB NX pte
 0xf79fe000-0xf7a00000           8K                           pte


The patch have been developed for Linux 2.6.31-rc7 x86 by Siarhei Liakh
<sliakh.lkml@gmail.com> and Xuxian Jiang <jiang@cs.ncsu.edu>.

V1:  initial patch for 2.6.30
V2:  patch for 2.6.31-rc7
V3:  moved all code into arch/x86, adjusted credits
V4:  fixed ifdef, removed credits from CREDITS
V5:  fixed an address calculation bug in mark_nxdata_nx()
V6:  added acked-by and PT dump diff to commit log
---

Signed-off-by: Siarhei Liakh <sliakh.lkml@gmail.com>
Signed-off-by: Xuxian Jiang <jiang@cs.ncsu.edu>
Acked-by: Arjan van de Ven <arjan@linux.intel.com>

diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 78d185d..83ae734 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -43,14 +43,14 @@ jiffies_64 = jiffies;

 PHDRS {
       text PT_LOAD FLAGS(5);          /* R_E */
-       data PT_LOAD FLAGS(7);          /* RWE */
+       data PT_LOAD FLAGS(6);          /* RW_ */
 #ifdef CONFIG_X86_64
-       user PT_LOAD FLAGS(7);          /* RWE */
-       data.init PT_LOAD FLAGS(7);     /* RWE */
+       user PT_LOAD FLAGS(6);          /* RW_ */
+       data.init PT_LOAD FLAGS(6);     /* RW_ */
 #ifdef CONFIG_SMP
-       percpu PT_LOAD FLAGS(7);        /* RWE */
+       percpu PT_LOAD FLAGS(6);        /* RW_ */
 #endif
-       data.init2 PT_LOAD FLAGS(7);    /* RWE */
+       data.init2 PT_LOAD FLAGS(6);    /* RW_ */
 #endif
       note PT_NOTE FLAGS(0);          /* ___ */
 }
@@ -89,6 +89,8 @@ SECTIONS
               IRQENTRY_TEXT
               *(.fixup)
               *(.gnu.warning)
+               /* .text should occupy whole number of pages */
+               . = ALIGN(PAGE_SIZE);
               /* End of text section */
               _etext = .;
       } :text = 0x9090
@@ -151,6 +153,8 @@ SECTIONS
       .data.read_mostly : AT(ADDR(.data.read_mostly) - LOAD_OFFSET) {
               *(.data.read_mostly)

+               /* .data should occupy whole number of pages */
+               . = ALIGN(PAGE_SIZE);
               /* End of data section */
               _edata = .;
       }
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 0607119..7bfd411 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -423,9 +423,10 @@ void free_init_pages(char *what, unsigned long
begin, unsigned long end)
       /*
        * We just marked the kernel text read only above, now that
        * we are going to free part of that, we need to make that
-        * writeable first.
+        * writeable and non-executable first.
        */
       set_memory_rw(begin, (end - begin) >> PAGE_SHIFT);
+       set_memory_nx(begin, (end - begin) >> PAGE_SHIFT);

       printk(KERN_INFO "Freeing %s: %luk freed\n", what, (end - begin) >> 10);

@@ -440,11 +441,29 @@ void free_init_pages(char *what, unsigned long
begin, unsigned long end)
 #endif
 }

+void mark_nxdata_nx(void)
+{
+#ifdef CONFIG_DEBUG_RODATA
+       /*
+        * When this called, init has already been executed and released,
+        * so everything past _etext sould be NX.
+        */
+       unsigned long start = PAGE_ALIGN((unsigned long)(&_etext));
+       unsigned long size = PAGE_ALIGN((unsigned long)(&_end)) - start;
+
+       printk(KERN_INFO "NX-protecting the kernel data: %lx, %lu pages\n",
+               start, size >> PAGE_SHIFT);
+       set_memory_nx(start, size >> PAGE_SHIFT);
+#endif
+}
+
 void free_initmem(void)
 {
       free_init_pages("unused kernel memory",
                       (unsigned long)(&__init_begin),
                       (unsigned long)(&__init_end));
+       /* Set kernel's data as NX */
+       mark_nxdata_nx();
 }

 #ifdef CONFIG_BLK_DEV_INITRD

Re: [PATCH V6] x86: NX protection for kernel data

[Ingo Molnar]

* Siarhei Liakh <sliakh.lkml@gmail.com> wrote:

> This patch expands functionality of CONFIG_DEBUG_RODATA to set main 
> (static) kernel data area as NX.

thanks - this one looks good now.

Would be nice if you could send this with the patch that also deals with 
the first 1 MB.

	Ingo

Re: [PATCH V6] x86: NX protection for kernel data

[Siarhei Liakh]

> thanks - this one looks good now.
>
> Would be nice if you could send this with the patch that also deals with
> the first 1 MB.

I do plan to have that first megabyte fixed. But for now, let's keep
these patches separate, as there are many things that rely on that 1MB
area and I want to make sure they all still work properly with the
patch.

Re: [PATCH V6] x86: NX protection for kernel data

[Ingo Molnar]

* Siarhei Liakh <sliakh.lkml@gmail.com> wrote:

> > thanks - this one looks good now.
> >
> > Would be nice if you could send this with the patch that also deals 
> > with the first 1 MB.
> 
> I do plan to have that first megabyte fixed. But for now, let's keep 
> these patches separate, as there are many things that rely on that 1MB 
> area and I want to make sure they all still work properly with the 
> patch.

Well, if we are going to touch this area i'd like to see them addressed 
together.

The 1MB thing would obviously be a default-off Kconfig option so in that 
sense it should not break anything by default.

	Ingo

Re: [PATCH V6] x86: NX protection for kernel data

[Siarhei Liakh]

On Fri, Oct 16, 2009 at 8:27 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
[...]
> Well, if we are going to touch this area i'd like to see them addressed
> together.
>
> The 1MB thing would obviously be a default-off Kconfig option so in that
> sense it should not break anything by default.

Ingo,

I've got this 1MB thing fixed. I have also adjusted my previous
patches to be compatible with 2.6.33-rc5.
I will be re-posting this patch set to LKML shortly.
Let me know if there is anything else you would like me to adjust.

Thanks.

[PATCH V6] x86: NX protection for kernel data

[Siarhei Liakh]

This patch expands functionality of CONFIG_DEBUG_RODATA to set main
(static) kernel data area as NX.
The following steps are taken to achieve this:
1. Linker script is adjusted so .text always starts and ends on a page boundary
2. Linker script is adjusted so .rodata and .data always start and
end on a page boundary
3. void mark_nxdata_nx(void) added to arch/x86/mm/init.c with actual
functionality: NX is set for all pages from _etext through _end.
4. mark_nxdata_nx() called from free_initmem() (after init has been released)
5. free_init_pages() sets released memory NX in arch/x86/mm/init.c

The patch have been developed for Linux 2.6.31-rc7 x86 by Siarhei Liakh
<sliakh.lkml@gmail.com> and Xuxian Jiang <jiang@cs.ncsu.edu>.

V1:  initial patch for 2.6.30
V2:  patch for 2.6.31-rc7
V3:  moved all code into arch/x86, adjusted credits
V4:  fixed ifdef, removed credits from CREDITS
V5:  fixed an address calculation bug in mark_nxdata_nx()
V6:  updated for compatibility with 2.6.33-rc5
---

Signed-off-by: Siarhei Liakh <sliakh.lkml@gmail.com>
Signed-off-by: Xuxian Jiang <jiang@cs.ncsu.edu>
Acked-by: Arjan van de Ven <arjan@linux.intel.com>

diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index f92a0da..2cb7369 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -69,7 +69,7 @@ jiffies_64 = jiffies;

 PHDRS {
 	text PT_LOAD FLAGS(5);          /* R_E */
-	data PT_LOAD FLAGS(7);          /* RWE */
+	data PT_LOAD FLAGS(6);          /* RW_ */
 #ifdef CONFIG_X86_64
 	user PT_LOAD FLAGS(5);          /* R_E */
 #ifdef CONFIG_SMP
@@ -108,6 +108,8 @@ SECTIONS
 		IRQENTRY_TEXT
 		*(.fixup)
 		*(.gnu.warning)
+		/* .text should occupy whole number of pages */
+		. = ALIGN(PAGE_SIZE);
 		/* End of text section */
 		_etext = .;
 	} :text = 0x9090
@@ -143,6 +145,8 @@ SECTIONS
 		/* rarely changed data like cpu maps */
 		READ_MOSTLY_DATA(INTERNODE_CACHE_BYTES)

+		/* .data should occupy whole number of pages */
+		. = ALIGN(PAGE_SIZE);
 		/* End of data section */
 		_edata = .;
 	} :data
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index d406c52..d613d0a 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -356,9 +356,10 @@ void free_init_pages(char *what, unsigned long
begin, unsigned long end)
 	/*
 	 * We just marked the kernel text read only above, now that
 	 * we are going to free part of that, we need to make that
-	 * writeable first.
+	 * writeable and non-executable first.
 	 */
 	set_memory_rw(begin, (end - begin) >> PAGE_SHIFT);
+	set_memory_nx(begin, (end - begin) >> PAGE_SHIFT);

 	printk(KERN_INFO "Freeing %s: %luk freed\n", what, (end - begin) >> 10);

@@ -373,11 +374,29 @@ void free_init_pages(char *what, unsigned long
begin, unsigned long end)
 #endif
 }

+void mark_nxdata_nx(void)
+{
+#ifdef CONFIG_DEBUG_RODATA
+	/*
+	 * When this called, init has already been executed and released,
+	 * so everything past _etext sould be NX.
+	 */
+	unsigned long start = PAGE_ALIGN((unsigned long)(&_etext));
+	unsigned long size = PAGE_ALIGN((unsigned long)(&_end)) - start;
+
+	printk(KERN_INFO "NX-protecting the kernel data: %lx, %lu pages\n",
+		start, size >> PAGE_SHIFT);
+	set_memory_nx(start, size >> PAGE_SHIFT);
+#endif
+}
+
 void free_initmem(void)
 {
 	free_init_pages("unused kernel memory",
 			(unsigned long)(&__init_begin),
 			(unsigned long)(&__init_end));
+	/* Set kernel's data as NX */
+	mark_nxdata_nx();
 }

 #ifdef CONFIG_BLK_DEV_INITRD

Re: [PATCH V6] x86: NX protection for kernel data

[James Morris]

On Sun, 31 Jan 2010, Siarhei Liakh wrote:

> V1:  initial patch for 2.6.30
> V2:  patch for 2.6.31-rc7
> V3:  moved all code into arch/x86, adjusted credits
> V4:  fixed ifdef, removed credits from CREDITS
> V5:  fixed an address calculation bug in mark_nxdata_nx()
> V6:  updated for compatibility with 2.6.33-rc5
> ---
> 
> Signed-off-by: Siarhei Liakh <sliakh.lkml@gmail.com>
> Signed-off-by: Xuxian Jiang <jiang@cs.ncsu.edu>
> Acked-by: Arjan van de Ven <arjan@linux.intel.com>

Reviewed-by: James Morris <jmorris@namei.org>

-- 
James Morris
<jmorris@namei.org>