Skip to contentSkip to conversation

NX protection for kernel data : fix xen boot

PATCH In review matthieu castet 2 participants 2 messages Jan 27, 2011 – Feb 2, 2011

Subsystems (2)
x86 x86 MM
lore
Share & tools
lore

matthieu castet

author
#0 lore raw

PATCH 1/1 NX protection for kernel data : fix xen boot

I think it should be applied before 2.6.38 release, because without this patch xen doesn't boot on x86_32 with CONFIG_DEBUG_RODATA.

Quoted 1 line

From 08f54ec4007ab0df231d81ed436632aa7f605feb Mon Sep 17 00:00:00 2001

From: Matthieu CASTET <[email protected]>
Date: Thu, 20 Jan 2011 21:11:45 +0100
Subject: [PATCH] NX protection for kernel data : fix xen boot

Xen want page table pages read only.

But the initial page table (from head_*.S) live in .data or .bss.

That was broken by 64edc8ed5ffae999d8d413ba006850e9e34166cb. Revert the problematic part as it is only a protection and not really needed.

Signed-off-by Matthieu CASTET <[email protected]>
Tested-by Konrad Rzeszutek Wilk <[email protected]>
---
 arch/x86/mm/pageattr.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)
arch/x86/mm/pageattr.c +0 -8
index 2ad6c48..951eb4a 100644
@@ -256,7 +256,6 @@ static inline pgprot_t static_protections(pgprot_t prot, unsigned long address,
 				   unsigned long pfn)
 {
 	pgprot_t forbidden = __pgprot(0);
-	pgprot_t required = __pgprot(0);
 
 	/*
 	 * The BIOS area between 640k and 1Mb needs to be executable for

@@ -286,12 +285,6 @@ static inline pgprot_t static_protections(pgprot_t prot, unsigned long address,
 	if (within(pfn, __pa((unsigned long)__start_rodata) >> PAGE_SHIFT,
 		   __pa((unsigned long)__end_rodata) >> PAGE_SHIFT))
 		pgprot_val(forbidden) |= _PAGE_RW;
-	/*
-	 * .data and .bss should always be writable.
-	 */
-	if (within(address, (unsigned long)_sdata, (unsigned long)_edata) ||
-	    within(address, (unsigned long)__bss_start, (unsigned long)__bss_stop))
-		pgprot_val(required) |= _PAGE_RW;
 
 #if defined(CONFIG_X86_64) && defined(CONFIG_DEBUG_RODATA)
 	/*

@@ -331,7 +324,6 @@ static inline pgprot_t static_protections(pgprot_t prot, unsigned long address,
 #endif
 
 	prot = __pgprot(pgprot_val(prot) & ~pgprot_val(forbidden));
-	prot = __pgprot(pgprot_val(prot) | pgprot_val(required));
 
 	return prot;
 }
Signature · 1 line
1.7.2.3
#1 lore raw

for tip-bugfix? Re: [PATCH] NX protection for kernel data : fix xen boot

Quoted 3 lines · matthieu castet

I think it should be applied before 2.6.38 release, because without this patch xen doesn't boot on x86_32 with CONFIG_DEBUG_RODATA.

Hey tglx + hpa,

The title is a bit wrong, the issue here is not with the NX bit but with setting RW on RO areas in the .bss area.

I was wondering if you guys would be OK picking this up in your "tip-2.6.38-rc4-bug-fix" branch for Linus? I've done the baremetal tests on this patch (x86, x86_64) and have not found any regressions with it.

Quoted 55 lines
Quoted 1 line

From 08f54ec4007ab0df231d81ed436632aa7f605feb Mon Sep 17 00:00:00 2001

From: Matthieu CASTET <[email protected]>
Date: Thu, 20 Jan 2011 21:11:45 +0100
Subject: [PATCH] NX protection for kernel data : fix xen boot

Xen want page table pages read only.

But the initial page table (from head_*.S) live in .data or .bss.

That was broken by 64edc8ed5ffae999d8d413ba006850e9e34166cb. Revert the problematic part as it is only a protection and not really needed.

Signed-off-by Matthieu CASTET <[email protected]>
Tested-by Konrad Rzeszutek Wilk <[email protected]>
---
 arch/x86/mm/pageattr.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)
arch/x86/mm/pageattr.c +0 -8
index 2ad6c48..951eb4a 100644
@@ -256,7 +256,6 @@ static inline pgprot_t static_protections(pgprot_t prot, unsigned long address,
 				   unsigned long pfn)
 {
 	pgprot_t forbidden = __pgprot(0);
-	pgprot_t required = __pgprot(0);
 
 	/*
 	 * The BIOS area between 640k and 1Mb needs to be executable for

@@ -286,12 +285,6 @@ static inline pgprot_t static_protections(pgprot_t prot, unsigned long address,
 	if (within(pfn, __pa((unsigned long)__start_rodata) >> PAGE_SHIFT,
 		   __pa((unsigned long)__end_rodata) >> PAGE_SHIFT))
 		pgprot_val(forbidden) |= _PAGE_RW;
-	/*
-	 * .data and .bss should always be writable.
-	 */
-	if (within(address, (unsigned long)_sdata, (unsigned long)_edata) ||
-	    within(address, (unsigned long)__bss_start, (unsigned long)__bss_stop))
-		pgprot_val(required) |= _PAGE_RW;
 
 #if defined(CONFIG_X86_64) && defined(CONFIG_DEBUG_RODATA)
 	/*

@@ -331,7 +324,6 @@ static inline pgprot_t static_protections(pgprot_t prot, unsigned long address,
 #endif
 
 	prot = __pgprot(pgprot_val(prot) & ~pgprot_val(forbidden));
-	prot = __pgprot(pgprot_val(prot) | pgprot_val(required));
 
 	return prot;
 }
Signature · 1 line
1.7.2.3