2010-01-31 23:27:58

by Siarhei Liakh

[permalink] [raw]
Subject: [PATCH V6] x86: NX protection for kernel data

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
<[email protected]> and Xuxian Jiang <[email protected]>.

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 <[email protected]>
Signed-off-by: Xuxian Jiang <[email protected]>
Acked-by: Arjan van de Ven <[email protected]>

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


2010-02-01 21:41:17

by James Morris

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

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 <[email protected]>
> Signed-off-by: Xuxian Jiang <[email protected]>
> Acked-by: Arjan van de Ven <[email protected]>

Reviewed-by: James Morris <[email protected]>

--
James Morris
<[email protected]>

2010-02-17 19:54:15

by Siarhei Liakh

[permalink] [raw]
Subject: [tip:x86/mm] x86, mm: NX protection for kernel data

Commit-ID: 01ab31371da90a795b774d87edf2c21bb3a64dda
Gitweb: http://git.kernel.org/tip/01ab31371da90a795b774d87edf2c21bb3a64dda
Author: Siarhei Liakh <[email protected]>
AuthorDate: Sun, 31 Jan 2010 18:27:55 -0500
Committer: H. Peter Anvin <[email protected]>
CommitDate: Wed, 17 Feb 2010 10:11:24 -0800

x86, mm: NX protection for kernel data

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

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 <[email protected]>
Signed-off-by: Xuxian Jiang <[email protected]>
Acked-by: Arjan van de Ven <[email protected]>
Reviewed-by: James Morris <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/kernel/vmlinux.lds.S | 6 +++++-
arch/x86/mm/init.c | 21 ++++++++++++++++++++-
2 files changed, 25 insertions(+), 2 deletions(-)

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

2010-02-22 10:55:23

by Ingo Molnar

[permalink] [raw]
Subject: Re: [tip:x86/mm] x86, mm: NX protection for kernel data


* tip-bot for Siarhei Liakh <[email protected]> wrote:

> Commit-ID: 01ab31371da90a795b774d87edf2c21bb3a64dda
> Gitweb: http://git.kernel.org/tip/01ab31371da90a795b774d87edf2c21bb3a64dda
> Author: Siarhei Liakh <[email protected]>
> AuthorDate: Sun, 31 Jan 2010 18:27:55 -0500
> Committer: H. Peter Anvin <[email protected]>
> CommitDate: Wed, 17 Feb 2010 10:11:24 -0800
>
> x86, mm: NX protection for kernel data
>
> This patch expands functionality of CONFIG_DEBUG_RODATA to set main
> (static) kernel data area as NX.

-tip testing is seeing boot hangs along the lines of:

[ 15.568108] EXT3-fs (sda1): recovery complete
[ 15.573064] EXT3-fs (sda1): mounted filesystem with ordered data mode
[ 15.580313] VFS: Mounted root (ext3 filesystem) readonly on device 8:1.
[ 15.584021] async_waiting @ 1
[ 15.588008] async_continuing @ 1 after 0 usec
[ 15.592163] Freeing unused kernel memory: 540k freed
[ 15.600126] NX-protecting the kernel data: c15ab000, 2919 pages

which i suspect could be due to the commit above.

Config attached. Athlon64 testbox.

Ingo


Attachments:
(No filename) (1.06 kB)
config (68.90 kB)
Download all attachments

2010-02-22 11:01:29

by Ingo Molnar

[permalink] [raw]
Subject: Re: [tip:x86/mm] x86, mm: NX protection for kernel data


* Ingo Molnar <[email protected]> wrote:

>
> * tip-bot for Siarhei Liakh <[email protected]> wrote:
>
> > Commit-ID: 01ab31371da90a795b774d87edf2c21bb3a64dda
> > Gitweb: http://git.kernel.org/tip/01ab31371da90a795b774d87edf2c21bb3a64dda
> > Author: Siarhei Liakh <[email protected]>
> > AuthorDate: Sun, 31 Jan 2010 18:27:55 -0500
> > Committer: H. Peter Anvin <[email protected]>
> > CommitDate: Wed, 17 Feb 2010 10:11:24 -0800
> >
> > x86, mm: NX protection for kernel data
> >
> > This patch expands functionality of CONFIG_DEBUG_RODATA to set main
> > (static) kernel data area as NX.
>
> -tip testing is seeing boot hangs along the lines of:
>
> [ 15.568108] EXT3-fs (sda1): recovery complete
> [ 15.573064] EXT3-fs (sda1): mounted filesystem with ordered data mode
> [ 15.580313] VFS: Mounted root (ext3 filesystem) readonly on device 8:1.
> [ 15.584021] async_waiting @ 1
> [ 15.588008] async_continuing @ 1 after 0 usec
> [ 15.592163] Freeing unused kernel memory: 540k freed
> [ 15.600126] NX-protecting the kernel data: c15ab000, 2919 pages
>
> which i suspect could be due to the commit above.

Yep, that's confirmed now, applying these 3 reverts makes it boot fine:

833e0ca: Revert "x86, mm: NX protection for kernel data"
ce4b6b4: Revert "x86: RO/NX protection for loadable kernel modules"
e357312: Revert "module: fix () used as prototype in include/linux/module.h"

Ingo

2010-02-22 17:20:25

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [tip:x86/mm] x86, mm: NX protection for kernel data

On 02/22/2010 03:01 AM, Ingo Molnar wrote:
>>
>>> Commit-ID: 01ab31371da90a795b774d87edf2c21bb3a64dda
>>> Gitweb: http://git.kernel.org/tip/01ab31371da90a795b774d87edf2c21bb3a64dda
>>> Author: Siarhei Liakh <[email protected]>
>>> AuthorDate: Sun, 31 Jan 2010 18:27:55 -0500
>>> Committer: H. Peter Anvin <[email protected]>
>>> CommitDate: Wed, 17 Feb 2010 10:11:24 -0800
>>>
>>> x86, mm: NX protection for kernel data
>>>
>>> This patch expands functionality of CONFIG_DEBUG_RODATA to set main
>>> (static) kernel data area as NX.
>>
>> -tip testing is seeing boot hangs along the lines of:
>>
>> [ 15.568108] EXT3-fs (sda1): recovery complete
>> [ 15.573064] EXT3-fs (sda1): mounted filesystem with ordered data mode
>> [ 15.580313] VFS: Mounted root (ext3 filesystem) readonly on device 8:1.
>> [ 15.584021] async_waiting @ 1
>> [ 15.588008] async_continuing @ 1 after 0 usec
>> [ 15.592163] Freeing unused kernel memory: 540k freed
>> [ 15.600126] NX-protecting the kernel data: c15ab000, 2919 pages
>>
>> which i suspect could be due to the commit above.
>
> Yep, that's confirmed now, applying these 3 reverts makes it boot fine:
>
> 833e0ca: Revert "x86, mm: NX protection for kernel data"
> ce4b6b4: Revert "x86: RO/NX protection for loadable kernel modules"
> e357312: Revert "module: fix () used as prototype in include/linux/module.h"
>

Given that e357312 is a () -> (void) prototype fix, is hardly seems
likely to be at fault. The RO/NX stuff, on the other hand, make a lot
of sense.

-hpa

--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.

2010-02-22 17:22:07

by Ingo Molnar

[permalink] [raw]
Subject: Re: [tip:x86/mm] x86, mm: NX protection for kernel data


* H. Peter Anvin <[email protected]> wrote:

> On 02/22/2010 03:01 AM, Ingo Molnar wrote:
> >>
> >>> Commit-ID: 01ab31371da90a795b774d87edf2c21bb3a64dda
> >>> Gitweb: http://git.kernel.org/tip/01ab31371da90a795b774d87edf2c21bb3a64dda
> >>> Author: Siarhei Liakh <[email protected]>
> >>> AuthorDate: Sun, 31 Jan 2010 18:27:55 -0500
> >>> Committer: H. Peter Anvin <[email protected]>
> >>> CommitDate: Wed, 17 Feb 2010 10:11:24 -0800
> >>>
> >>> x86, mm: NX protection for kernel data
> >>>
> >>> This patch expands functionality of CONFIG_DEBUG_RODATA to set main
> >>> (static) kernel data area as NX.
> >>
> >> -tip testing is seeing boot hangs along the lines of:
> >>
> >> [ 15.568108] EXT3-fs (sda1): recovery complete
> >> [ 15.573064] EXT3-fs (sda1): mounted filesystem with ordered data mode
> >> [ 15.580313] VFS: Mounted root (ext3 filesystem) readonly on device 8:1.
> >> [ 15.584021] async_waiting @ 1
> >> [ 15.588008] async_continuing @ 1 after 0 usec
> >> [ 15.592163] Freeing unused kernel memory: 540k freed
> >> [ 15.600126] NX-protecting the kernel data: c15ab000, 2919 pages
> >>
> >> which i suspect could be due to the commit above.
> >
> > Yep, that's confirmed now, applying these 3 reverts makes it boot fine:
> >
> > 833e0ca: Revert "x86, mm: NX protection for kernel data"
> > ce4b6b4: Revert "x86: RO/NX protection for loadable kernel modules"
> > e357312: Revert "module: fix () used as prototype in include/linux/module.h"
> >
>
> Given that e357312 is a () -> (void) prototype fix, is hardly seems
> likely to be at fault. The RO/NX stuff, on the other hand, make a lot
> of sense.

Yes, i reverted e357312 because it was a dependent change.

Thanks,

Ingo