public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [tip:x86/mm] x86, mm: NX protection for kernel data
@ 2010-03-13 12:12 matthieu castet
  2010-03-15 18:20 ` Siarhei Liakh
  0 siblings, 1 reply; 16+ messages in thread
From: matthieu castet @ 2010-03-13 12:12 UTC (permalink / raw)
  To: Linux Kernel list; +Cc: Siarhei Liakh, Ingo Molnar

Hi,

> > looking for c17ebdb8 in system.map points to a location in pgd_lock:
> > ============================================
> > $grep c17ebd System.map
> > c17ebd68 d bios_check_work
> > c17ebda8 d highmem_pages
> > c17ebdac D pgd_lock
> > c17ebdc8 D pgd_list
> > c17ebdd0 D show_unhandled_signals
> > c17ebdd4 d cpa_lock
> > c17ebdf0 d memtype_lock
> > ============================================
> >
> > I've looked at the lock debugging and could not find any place that
> > would look like an attempt to execute data. This would lead me to
> > think that calling set_memory_nx from kernel_init somehow confuses the
> > lock debugging subsystem, or set_memory_nx does not change page
> > attributes in a safe manner (for example when a lock is stored inside
> > the page whose attributes are being changed).
> 
> I've done some extra debugging and it really does look like the crash
> happens when we are setting NX on a large page which has pgd_lock
> inside it.
> 
> Here is a trace of printk's that I added to troubleshoot this issue:
> =========================
> [    3.072003] try_preserve_large_page - enter
> [    3.073185] try_preserve_large_page - address: 0xc1600000
> [    3.074513] try_preserve_large_page - 2M page
> [    3.075606] try_preserve_large_page - about to call static_protections
> [    3.076000] try_preserve_large_page - back from static_protections
> [    3.076000] try_preserve_large_page - past loop
> [    3.076000] try_preserve_large_page - new_prot != old_prot
> [    3.076000] try_preserve_large_page - the address is aligned and
> the number of pages covers the full range
> [    3.076000] try_preserve_large_page - about to call __set_pmd_pte
> [    3.076000] __set_pmd_pte - enter
> [    3.076000] __set_pmd_pte - address: 0xc1600000
> [    3.076000] __set_pmd_pte - about to call
> set_pte_atomic(*0xc18c0058(low=0x16001e3, high=0x0), (low=0x16001e1,
> high=0x80000000))
> [lock-up here]
> =========================
> 

This may be stupid but :


0xc1600000 2MB page is in 0xc1600000-0xc1800000 range.  pgd_lock (0xc17ebdac) seems to be in that range.

You change attribute from (low=0x16001e3, high=0x0) to (low=0x16001e1, high=0x80000000). IE you set
NX bit (bit 63), but you also clear R/W bit (bit 2). So the page become read only, but you are using a lock
inside this page that need RW access. So you got a page fault.


Now I don't know what should be done.
Is that normal we set the page RO ?

Matthieu

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [tip:x86/mm] x86, mm: NX protection for kernel data
@ 2010-03-02 16:23 castet.matthieu
  2010-03-02 17:51 ` Siarhei Liakh
  0 siblings, 1 reply; 16+ messages in thread
From: castet.matthieu @ 2010-03-02 16:23 UTC (permalink / raw)
  To: Siarhei Liakh; +Cc: linux-kernel@vger.kernel.org

[-- Attachment #1: Type: text/plain, Size: 303 bytes --]



> At this point I need some help and guidance on how to track down what
> exactly happens there, as I am not very familiar with what goes into
> .data and why are we trying to execute it.
Can't you add debug printk in the fault handler before any exception processing

Something like that.

Matthieu


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: test.diff --]
[-- Type: text/x-diff; name="test.diff", Size: 401 bytes --]

diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index f627779..578ba52 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -629,6 +629,8 @@ no_context(struct pt_regs *regs, unsigned long error_code,
 	unsigned long flags;
 	int sig;
 
+	if (error_code & PF_INSTR)
+		printk("exec on NX\n");
 	/* Are we prepared to handle this kernel fault? */
 	if (fixup_exception(regs))
 		return;

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH V6] x86: NX protection for kernel data
@ 2010-01-31 23:27 Siarhei Liakh
  2010-02-17 19:51 ` [tip:x86/mm] x86, mm: " tip-bot for Siarhei Liakh
  0 siblings, 1 reply; 16+ messages in thread
From: Siarhei Liakh @ 2010-01-31 23:27 UTC (permalink / raw)
  To: linux-kernel, linux-security-module, linux-next
  Cc: Arjan van de Ven, James Morris, Andrew Morton, Andi Kleen,
	Thomas Gleixner, H. Peter Anvin, Ingo Molnar, Rusty Russell,
	Stephen Rothwell, Dave Jones

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2010-03-15 21:41 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-13 12:12 [tip:x86/mm] x86, mm: NX protection for kernel data matthieu castet
2010-03-15 18:20 ` Siarhei Liakh
2010-03-15 21:40   ` Siarhei Liakh
  -- strict thread matches above, loose matches on Subject: below --
2010-03-02 16:23 castet.matthieu
2010-03-02 17:51 ` Siarhei Liakh
2010-03-02 18:03   ` Siarhei Liakh
2010-01-31 23:27 [PATCH V6] x86: " Siarhei Liakh
2010-02-17 19:51 ` [tip:x86/mm] x86, mm: " tip-bot for Siarhei Liakh
2010-02-22 10:54   ` Ingo Molnar
2010-02-22 11:01     ` Ingo Molnar
2010-02-22 17:19       ` H. Peter Anvin
2010-02-22 17:21         ` Ingo Molnar
2010-03-06 19:44           ` Siarhei Liakh
2010-03-10 13:32             ` Ingo Molnar
2010-03-10 15:06               ` Siarhei Liakh
2010-03-12  3:12             ` Siarhei Liakh
2010-03-02 15:13     ` Siarhei Liakh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox