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
<[email protected]> and Xuxian Jiang <[email protected]>.
---
Signed-off-by: Siarhei Liakh <[email protected]>
Signed-off-by: Xuxian Jiang <[email protected]>
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();
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
> <[email protected]> and Xuxian Jiang <[email protected]>.
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
On Sun, 19 Jul 2009 15:43:06 -0400
Siarhei Liakh <[email protected]> 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
> <[email protected]> and Xuxian Jiang <[email protected]>.
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....
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
On Sun, Jul 19, 2009 at 4:13 PM, Sam Ravnborg<[email protected]> 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
>> <[email protected]> and Xuxian Jiang <[email protected]>.
>
>
> 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?
> Please avoid adding the identical function to both files.
> arch/x86/mm/init.c is the correct place.
Will do.
Thank you.
On Sun, Jul 19, 2009 at 4:18 PM, Arjan van de Ven<[email protected]> wrote:
> On Sun, 19 Jul 2009 15:43:06 -0400
> Siarhei Liakh <[email protected]> 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
>> <[email protected]> and Xuxian Jiang <[email protected]>.
>
> 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.
> >
> > 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
* Sam Ravnborg <[email protected]> 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