[PATCH] x86: NX protection for kernel data

[PATCH] 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 scripts are adjusted so .text always starts and end on a page boundary
2. Linker scripts are adjusted so .rodata and .data always starts and
end on a page boundary
3. void mark_nxdata_nx(void) added to arch/x86/mm/init_64.c and
arch/x86/mm/init_32.c with actual functionality: NX is set for all
pages from _etext through _edata
4. mark_nxdata_nx() called from init_post(void) in init/main.c

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

---

Signed-off-by: Siarhei Liakh <sliakh.lkml@gmail.com>
Signed-off-by: Xuxian Jiang <jiang@cs.ncsu.edu>

diff --git a/arch/x86/include/asm/cacheflush.h
b/arch/x86/include/asm/cacheflush.h
index e55dfc1..cce364e 100644
--- a/arch/x86/include/asm/cacheflush.h
+++ b/arch/x86/include/asm/cacheflush.h
@@ -125,6 +125,7 @@ void clflush_cache_range(void *addr, unsigned int size);

 #ifdef CONFIG_DEBUG_RODATA
 void mark_rodata_ro(void);
+void mark_nxdata_nx(void);
 extern const int rodata_test_data;
 void set_kernel_text_rw(void);
 void set_kernel_text_ro(void);
diff --git a/arch/x86/kernel/vmlinux_32.lds.S b/arch/x86/kernel/vmlinux_32.lds.S
index 62ad500..4041522 100644
--- a/arch/x86/kernel/vmlinux_32.lds.S
+++ b/arch/x86/kernel/vmlinux_32.lds.S
@@ -47,6 +47,7 @@ SECTIONS
 	IRQENTRY_TEXT
 	*(.fixup)
 	*(.gnu.warning)
+  	. = ALIGN(PAGE_SIZE);	/* .text should occupy whole number of pages */
   	_etext = .;			/* End of text section */
   } :text = 0x9090

@@ -93,6 +94,7 @@ SECTIONS
 	*(.data.read_mostly)
 	_edata = .;		/* End of data section */
   }
+  . = ALIGN(PAGE_SIZE);		/* needed so we can set NX for .data */

   . = ALIGN(THREAD_SIZE);	/* init_task */
   .data.init_task : AT(ADDR(.data.init_task) - LOAD_OFFSET) {
diff --git a/arch/x86/kernel/vmlinux_64.lds.S b/arch/x86/kernel/vmlinux_64.lds.S
index c874250..a60ce17 100644
--- a/arch/x86/kernel/vmlinux_64.lds.S
+++ b/arch/x86/kernel/vmlinux_64.lds.S
@@ -42,6 +42,7 @@ SECTIONS
 	IRQENTRY_TEXT
 	*(.fixup)
 	*(.gnu.warning)
+  	. = ALIGN(PAGE_SIZE);	/* .text should occupy whole number of pages */
 	_etext = .;		/* End of text section */
   } :text = 0x9090

@@ -61,6 +62,7 @@ SECTIONS
   .data : AT(ADDR(.data) - LOAD_OFFSET) {
 	DATA_DATA
 	CONSTRUCTORS
+  	. = ALIGN(PAGE_SIZE);		/* needed so we can set NX for .data */
 	_edata = .;			/* End of data section */
 	} :data

diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index 749559e..68163dc 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -1119,6 +1119,16 @@ void mark_rodata_ro(void)
 	set_pages_ro(virt_to_page(start), size >> PAGE_SHIFT);
 #endif
 }
+
+void mark_nxdata_nx(void)
+{
+	unsigned long start = PFN_ALIGN(_etext);
+	unsigned long size = PFN_ALIGN(_edata) - start;
+
+	printk(KERN_INFO "NX-protecting the kernel data: %lx, %lu pages\n",
+		start, size >> PAGE_SHIFT);
+	set_pages_nx(virt_to_page(start), size >> PAGE_SHIFT);
+}
 #endif

 int __init reserve_bootmem_generic(unsigned long phys, unsigned long len,
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 1753e80..5b0843f 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -793,6 +793,15 @@ void mark_rodata_ro(void)
 #endif
 }

+void mark_nxdata_nx(void)
+{
+	unsigned long start = PFN_ALIGN(_etext);
+	unsigned long size = PFN_ALIGN(_edata) - start;
+
+	printk(KERN_INFO "NX-protecting the kernel data: %lx, %lu pages\n",
+		start, size >> PAGE_SHIFT);
+	set_pages_nx(virt_to_page(start), size >> PAGE_SHIFT);
+}
 #endif

 int __init reserve_bootmem_generic(unsigned long phys, unsigned long len,
diff --git a/init/main.c b/init/main.c
index d721dad..6c0ee8b 100644
--- a/init/main.c
+++ b/init/main.c
@@ -93,6 +93,7 @@ static inline void acpi_early_init(void) { }
 #endif
 #ifndef CONFIG_DEBUG_RODATA
 static inline void mark_rodata_ro(void) { }
+static inline void mark_nxdata_nx(void) { }
 #endif

 #ifdef CONFIG_TC
@@ -807,6 +808,7 @@ static noinline int init_post(void)
 	free_initmem();
 	unlock_kernel();
 	mark_rodata_ro();
+	mark_nxdata_nx();
 	system_state = SYSTEM_RUNNING;
 	numa_default_policy();

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

[Sam Ravnborg]

On Sun, Jul 19, 2009 at 03:43:06PM -0400, Siarhei Liakh wrote:
> 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 scripts are adjusted so .text always starts and end on a page boundary
> 2. Linker scripts are adjusted so .rodata and .data always starts and
> end on a page boundary
> 3. void mark_nxdata_nx(void) added to arch/x86/mm/init_64.c and
> arch/x86/mm/init_32.c with actual functionality: NX is set for all
> pages from _etext through _edata
> 4. mark_nxdata_nx() called from init_post(void) in init/main.c
> 
> The patch have been developed for Linux 2.6.30 x86 by Siarhei Liakh
> <sliakh.lkml@gmail.com> and Xuxian Jiang <jiang@cs.ncsu.edu>.


The patch no longer applies.
The file vmlinux_32.lds.S and vmlinux_64.lds.S has been unified
into one file.


> --- a/arch/x86/kernel/vmlinux_32.lds.S
> +++ b/arch/x86/kernel/vmlinux_32.lds.S
> @@ -47,6 +47,7 @@ SECTIONS
>  	IRQENTRY_TEXT
>  	*(.fixup)
>  	*(.gnu.warning)
> +  	. = ALIGN(PAGE_SIZE);	/* .text should occupy whole number of pages */
>    	_etext = .;			/* End of text section */

So _etext cover until page boundary - makes sense.

>    } :text = 0x9090
> 
> @@ -93,6 +94,7 @@ SECTIONS
>  	*(.data.read_mostly)
>  	_edata = .;		/* End of data section */
>    }
> +  . = ALIGN(PAGE_SIZE);		/* needed so we can set NX for .data */

But here _edata does not cover until page boundary.
And alignmnet is located outside the output section
definition.
It would be better/more consistent to follow the style you use for .text here.


	Sam

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

[Arjan van de Ven]

On Sun, 19 Jul 2009 15:43:06 -0400
Siarhei Liakh <sliakh.lkml@gmail.com> wrote:

> 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 scripts are adjusted so .text always starts and end on a
> page boundary 2. Linker scripts are adjusted so .rodata and .data
> always starts and end on a page boundary
> 3. void mark_nxdata_nx(void) added to arch/x86/mm/init_64.c and
> arch/x86/mm/init_32.c with actual functionality: NX is set for all
> pages from _etext through _edata
> 4. mark_nxdata_nx() called from init_post(void) in init/main.c
> 
> The patch have been developed for Linux 2.6.30 x86 by Siarhei Liakh
> <sliakh.lkml@gmail.com> and Xuxian Jiang <jiang@cs.ncsu.edu>.

I like the idea, and am happy to see the lack of ifdefs ;)

I wonder if we should have a testcase for this though similar to 
how stackprotector and rodata get tested already....

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

[Thomas Gleixner]

On Sun, 19 Jul 2009, Siarhei Liakh wrote:

> 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 scripts are adjusted so .text always starts and end on a page boundary
> 2. Linker scripts are adjusted so .rodata and .data always starts and
> end on a page boundary
> 3. void mark_nxdata_nx(void) added to arch/x86/mm/init_64.c and
> arch/x86/mm/init_32.c with actual functionality: NX is set for all
> pages from _etext through _edata

Please avoid adding the identical function to both files.
arch/x86/mm/init.c is the correct place.

Thanks,

	tglx

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

[Siarhei Liakh]

On Sun, Jul 19, 2009 at 4:13 PM, Sam Ravnborg<sam@ravnborg.org> wrote:
> On Sun, Jul 19, 2009 at 03:43:06PM -0400, Siarhei Liakh wrote:
>> 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 scripts are adjusted so .text always starts and end on a page boundary
>> 2. Linker scripts are adjusted so .rodata and .data always starts and
>> end on a page boundary
>> 3. void mark_nxdata_nx(void) added to arch/x86/mm/init_64.c and
>> arch/x86/mm/init_32.c with actual functionality: NX is set for all
>> pages from _etext through _edata
>> 4. mark_nxdata_nx() called from init_post(void) in init/main.c
>>
>> The patch have been developed for Linux 2.6.30 x86 by Siarhei Liakh
>> <sliakh.lkml@gmail.com> and Xuxian Jiang <jiang@cs.ncsu.edu>.
>
>
> The patch no longer applies.
> The file vmlinux_32.lds.S and vmlinux_64.lds.S has been unified
> into one file.

That is actually a great news. I will get the latest source and
re-write the patch.

>> --- a/arch/x86/kernel/vmlinux_32.lds.S
>> +++ b/arch/x86/kernel/vmlinux_32.lds.S
>> @@ -47,6 +47,7 @@ SECTIONS
>>       IRQENTRY_TEXT
>>       *(.fixup)
>>       *(.gnu.warning)
>> +     . = ALIGN(PAGE_SIZE);   /* .text should occupy whole number of pages */
>>       _etext = .;                     /* End of text section */
>
> So _etext cover until page boundary - makes sense.
>
>>    } :text = 0x9090
>>
>> @@ -93,6 +94,7 @@ SECTIONS
>>       *(.data.read_mostly)
>>       _edata = .;             /* End of data section */
>>    }
>> +  . = ALIGN(PAGE_SIZE);              /* needed so we can set NX for .data */
>
> But here _edata does not cover until page boundary.
> And alignmnet is located outside the output section
> definition.
> It would be better/more consistent to follow the style you use for .text here.

You are correct. _edata should be the last thing in .data, and
alignment should be done before it. However, this brings up a
question: was there any specific reason to leave .data.init_task
beyond the _edata? Should we move _edata into the the last of the
.data.* sections to have poper view of kernel layout?

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

[Siarhei Liakh]

> Please avoid adding the identical function to both files.
> arch/x86/mm/init.c is the correct place.

Will do.

Thank you.

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

[Siarhei Liakh]

On Sun, Jul 19, 2009 at 4:18 PM, Arjan van de Ven<arjan@infradead.org> wrote:
> On Sun, 19 Jul 2009 15:43:06 -0400
> Siarhei Liakh <sliakh.lkml@gmail.com> wrote:
>
>> 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 scripts are adjusted so .text always starts and end on a
>> page boundary 2. Linker scripts are adjusted so .rodata and .data
>> always starts and end on a page boundary
>> 3. void mark_nxdata_nx(void) added to arch/x86/mm/init_64.c and
>> arch/x86/mm/init_32.c with actual functionality: NX is set for all
>> pages from _etext through _edata
>> 4. mark_nxdata_nx() called from init_post(void) in init/main.c
>>
>> The patch have been developed for Linux 2.6.30 x86 by Siarhei Liakh
>> <sliakh.lkml@gmail.com> and Xuxian Jiang <jiang@cs.ncsu.edu>.
>
> I like the idea, and am happy to see the lack of ifdefs ;)

I was thinking about ifdefs, but could not find a place to put them in ;)

> I wonder if we should have a testcase for this though similar to
> how stackprotector and rodata get tested already....

We probably should. In addition, after looking at the code for a
while, it seems to me that the proper place to enable protection would
be kernel_physical_mapping_init(). This way the kernel could enjoy
protection from the very beginning of init, instead of turning it on
at the end.

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

[Sam Ravnborg]

> >
> > But here _edata does not cover until page boundary.
> > And alignmnet is located outside the output section
> > definition.
> > It would be better/more consistent to follow the style you use for .text here.
> 
> You are correct. _edata should be the last thing in .data, and
> alignment should be done before it. However, this brings up a
> question: was there any specific reason to leave .data.init_task
> beyond the _edata? Should we move _edata into the the last of the
> .data.* sections to have poper view of kernel layout?

.data.init_task is loaded into another segment so this is less trivial.
Jan Beulich has posted a patch some time ago to address this,
but it failed when forwardported to the unified layout.

Btw. -tip (the x86 tree) has a patch included that unify the definition
of _edata for 32 and 64 bit.
So the best thing you can do is to base your patch on top of
the -tip tree.

	Sam

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

[Ingo Molnar]

* Sam Ravnborg <sam@ravnborg.org> wrote:

> > >
> > > But here _edata does not cover until page boundary.
> > > And alignmnet is located outside the output section
> > > definition.
> > > It would be better/more consistent to follow the style you use for .text here.
> > 
> > You are correct. _edata should be the last thing in .data, and
> > alignment should be done before it. However, this brings up a
> > question: was there any specific reason to leave .data.init_task
> > beyond the _edata? Should we move _edata into the the last of the
> > .data.* sections to have poper view of kernel layout?
> 
> .data.init_task is loaded into another segment so this is less trivial.
> Jan Beulich has posted a patch some time ago to address this,
> but it failed when forwardported to the unified layout.
> 
> Btw. -tip (the x86 tree) has a patch included that unify the definition
> of _edata for 32 and 64 bit.
> So the best thing you can do is to base your patch on top of
> the -tip tree.

which can be found at:

  http://people.redhat.com/mingo/tip.git/README

When (re-)submitting the patch please point out the dependency.

	Ingo