[PATCH v2 0/4] x86: clear XD_DISABLED flag on Intel to regain NX

[PATCH v2 0/4] x86: clear XD_DISABLED flag on Intel to regain NX

[Kees Cook]

This will clear the MSR_IA32_MISC_ENABLE_XD_DISABLE bit so that NX cannot
be inappropriately controlled by the BIOS on Intel CPUs.  If NX actually
needs to be disabled, "noexec=off" can be used.

Based on feedback from HPA, this was reworked to extend the existing
"verify_cpu" routines, and to more tightly confine which CPUs will call
MSR_IA32_MISC_ENABLE.  Since it includes some re-arrangements of files, I
tried to break the patches up into their logical steps.

-Kees

-- 
Kees Cook
Ubuntu Security Team

[PATCH 1/4] x86: rename verify_cpu_64.S to verify_cpu.S

[Kees Cook]

The code is 32bit already, and can be used in 32bit routines.

Signed-off-by: Kees Cook <kees.cook@canonical.com>
---
 arch/x86/boot/compressed/head_64.S |    2 +-
 arch/x86/kernel/trampoline_64.S    |    2 +-
 arch/x86/kernel/verify_cpu.S       |  105 ++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/verify_cpu_64.S    |  105 ------------------------------------
 4 files changed, 107 insertions(+), 107 deletions(-)
 create mode 100644 arch/x86/kernel/verify_cpu.S
 delete mode 100644 arch/x86/kernel/verify_cpu_64.S

diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index faff0dc..17f9620 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -182,7 +182,7 @@ no_longmode:
 	hlt
 	jmp     1b
 
-#include "../../kernel/verify_cpu_64.S"
+#include "../../kernel/verify_cpu.S"
 
 	/*
 	 * Be careful here startup_64 needs to be at a predictable
diff --git a/arch/x86/kernel/trampoline_64.S b/arch/x86/kernel/trampoline_64.S
index 3af2dff..075d130 100644
--- a/arch/x86/kernel/trampoline_64.S
+++ b/arch/x86/kernel/trampoline_64.S
@@ -127,7 +127,7 @@ startup_64:
 no_longmode:
 	hlt
 	jmp no_longmode
-#include "verify_cpu_64.S"
+#include "verify_cpu.S"
 
 	# Careful these need to be in the same 64K segment as the above;
 tidt:
diff --git a/arch/x86/kernel/verify_cpu.S b/arch/x86/kernel/verify_cpu.S
new file mode 100644
index 0000000..45b6f8a
--- /dev/null
+++ b/arch/x86/kernel/verify_cpu.S
@@ -0,0 +1,105 @@
+/*
+ *
+ *	verify_cpu.S - Code for cpu long mode and SSE verification. This
+ *	code has been borrowed from boot/setup.S and was introduced by
+ * 	Andi Kleen.
+ *
+ *	Copyright (c) 2007  Andi Kleen (ak@suse.de)
+ *	Copyright (c) 2007  Eric Biederman (ebiederm@xmission.com)
+ *	Copyright (c) 2007  Vivek Goyal (vgoyal@in.ibm.com)
+ *
+ * 	This source code is licensed under the GNU General Public License,
+ * 	Version 2.  See the file COPYING for more details.
+ *
+ *	This is a common code for verification whether CPU supports
+ * 	long mode and SSE or not. It is not called directly instead this
+ *	file is included at various places and compiled in that context.
+ * 	Following are the current usage.
+ *
+ * 	This file is included by both 16bit and 32bit code.
+ *
+ *	arch/x86_64/boot/setup.S : Boot cpu verification (16bit)
+ *	arch/x86_64/boot/compressed/head.S: Boot cpu verification (32bit)
+ *	arch/x86_64/kernel/trampoline.S: secondary processor verfication (16bit)
+ *	arch/x86_64/kernel/acpi/wakeup.S:Verfication at resume (16bit)
+ *
+ *	verify_cpu, returns the status of cpu check in register %eax.
+ *		0: Success    1: Failure
+ *
+ * 	The caller needs to check for the error code and take the action
+ * 	appropriately. Either display a message or halt.
+ */
+
+#include <asm/cpufeature.h>
+
+verify_cpu:
+	pushfl				# Save caller passed flags
+	pushl	$0			# Kill any dangerous flags
+	popfl
+
+	pushfl				# standard way to check for cpuid
+	popl	%eax
+	movl	%eax,%ebx
+	xorl	$0x200000,%eax
+	pushl	%eax
+	popfl
+	pushfl
+	popl	%eax
+	cmpl	%eax,%ebx
+	jz	verify_cpu_no_longmode	# cpu has no cpuid
+
+	movl	$0x0,%eax		# See if cpuid 1 is implemented
+	cpuid
+	cmpl	$0x1,%eax
+	jb	verify_cpu_no_longmode	# no cpuid 1
+
+	xor	%di,%di
+	cmpl	$0x68747541,%ebx	# AuthenticAMD
+	jnz	verify_cpu_noamd
+	cmpl	$0x69746e65,%edx
+	jnz	verify_cpu_noamd
+	cmpl	$0x444d4163,%ecx
+	jnz	verify_cpu_noamd
+	mov	$1,%di			# cpu is from AMD
+
+verify_cpu_noamd:
+	movl    $0x1,%eax		# Does the cpu have what it takes
+	cpuid
+	andl	$REQUIRED_MASK0,%edx
+	xorl	$REQUIRED_MASK0,%edx
+	jnz	verify_cpu_no_longmode
+
+	movl    $0x80000000,%eax	# See if extended cpuid is implemented
+	cpuid
+	cmpl    $0x80000001,%eax
+	jb      verify_cpu_no_longmode	# no extended cpuid
+
+	movl    $0x80000001,%eax	# Does the cpu have what it takes
+	cpuid
+	andl    $REQUIRED_MASK1,%edx
+	xorl    $REQUIRED_MASK1,%edx
+	jnz     verify_cpu_no_longmode
+
+verify_cpu_sse_test:
+	movl	$1,%eax
+	cpuid
+	andl	$SSE_MASK,%edx
+	cmpl	$SSE_MASK,%edx
+	je	verify_cpu_sse_ok
+	test	%di,%di
+	jz	verify_cpu_no_longmode	# only try to force SSE on AMD
+	movl	$0xc0010015,%ecx	# HWCR
+	rdmsr
+	btr	$15,%eax		# enable SSE
+	wrmsr
+	xor	%di,%di			# don't loop
+	jmp	verify_cpu_sse_test	# try again
+
+verify_cpu_no_longmode:
+	popfl				# Restore caller passed flags
+	movl $1,%eax
+	ret
+verify_cpu_sse_ok:
+	popfl				# Restore caller passed flags
+	xorl %eax, %eax
+	ret
diff --git a/arch/x86/kernel/verify_cpu_64.S b/arch/x86/kernel/verify_cpu_64.S
deleted file mode 100644
index 45b6f8a..0000000
--- a/arch/x86/kernel/verify_cpu_64.S
+++ /dev/null
@@ -1,105 +0,0 @@
-/*
- *
- *	verify_cpu.S - Code for cpu long mode and SSE verification. This
- *	code has been borrowed from boot/setup.S and was introduced by
- * 	Andi Kleen.
- *
- *	Copyright (c) 2007  Andi Kleen (ak@suse.de)
- *	Copyright (c) 2007  Eric Biederman (ebiederm@xmission.com)
- *	Copyright (c) 2007  Vivek Goyal (vgoyal@in.ibm.com)
- *
- * 	This source code is licensed under the GNU General Public License,
- * 	Version 2.  See the file COPYING for more details.
- *
- *	This is a common code for verification whether CPU supports
- * 	long mode and SSE or not. It is not called directly instead this
- *	file is included at various places and compiled in that context.
- * 	Following are the current usage.
- *
- * 	This file is included by both 16bit and 32bit code.
- *
- *	arch/x86_64/boot/setup.S : Boot cpu verification (16bit)
- *	arch/x86_64/boot/compressed/head.S: Boot cpu verification (32bit)
- *	arch/x86_64/kernel/trampoline.S: secondary processor verfication (16bit)
- *	arch/x86_64/kernel/acpi/wakeup.S:Verfication at resume (16bit)
- *
- *	verify_cpu, returns the status of cpu check in register %eax.
- *		0: Success    1: Failure
- *
- * 	The caller needs to check for the error code and take the action
- * 	appropriately. Either display a message or halt.
- */
-
-#include <asm/cpufeature.h>
-
-verify_cpu:
-	pushfl				# Save caller passed flags
-	pushl	$0			# Kill any dangerous flags
-	popfl
-
-	pushfl				# standard way to check for cpuid
-	popl	%eax
-	movl	%eax,%ebx
-	xorl	$0x200000,%eax
-	pushl	%eax
-	popfl
-	pushfl
-	popl	%eax
-	cmpl	%eax,%ebx
-	jz	verify_cpu_no_longmode	# cpu has no cpuid
-
-	movl	$0x0,%eax		# See if cpuid 1 is implemented
-	cpuid
-	cmpl	$0x1,%eax
-	jb	verify_cpu_no_longmode	# no cpuid 1
-
-	xor	%di,%di
-	cmpl	$0x68747541,%ebx	# AuthenticAMD
-	jnz	verify_cpu_noamd
-	cmpl	$0x69746e65,%edx
-	jnz	verify_cpu_noamd
-	cmpl	$0x444d4163,%ecx
-	jnz	verify_cpu_noamd
-	mov	$1,%di			# cpu is from AMD
-
-verify_cpu_noamd:
-	movl    $0x1,%eax		# Does the cpu have what it takes
-	cpuid
-	andl	$REQUIRED_MASK0,%edx
-	xorl	$REQUIRED_MASK0,%edx
-	jnz	verify_cpu_no_longmode
-
-	movl    $0x80000000,%eax	# See if extended cpuid is implemented
-	cpuid
-	cmpl    $0x80000001,%eax
-	jb      verify_cpu_no_longmode	# no extended cpuid
-
-	movl    $0x80000001,%eax	# Does the cpu have what it takes
-	cpuid
-	andl    $REQUIRED_MASK1,%edx
-	xorl    $REQUIRED_MASK1,%edx
-	jnz     verify_cpu_no_longmode
-
-verify_cpu_sse_test:
-	movl	$1,%eax
-	cpuid
-	andl	$SSE_MASK,%edx
-	cmpl	$SSE_MASK,%edx
-	je	verify_cpu_sse_ok
-	test	%di,%di
-	jz	verify_cpu_no_longmode	# only try to force SSE on AMD
-	movl	$0xc0010015,%ecx	# HWCR
-	rdmsr
-	btr	$15,%eax		# enable SSE
-	wrmsr
-	xor	%di,%di			# don't loop
-	jmp	verify_cpu_sse_test	# try again
-
-verify_cpu_no_longmode:
-	popfl				# Restore caller passed flags
-	movl $1,%eax
-	ret
-verify_cpu_sse_ok:
-	popfl				# Restore caller passed flags
-	xorl %eax, %eax
-	ret
-- 
1.7.1


-- 
Kees Cook
Ubuntu Security Team

[PATCH 2/4] x86: clear XD_DISABLED flag on Intel to regain NX

[Kees Cook]

This will clear the MSR_IA32_MISC_ENABLE_XD_DISABLE bit so that NX cannot
be inappropriately controlled by the BIOS on Intel CPUs.  If NX actually
needs to be disabled, "noexec=off" can be used.

Signed-off-by: Kees Cook <kees.cook@canonical.com>
---
 arch/x86/kernel/verify_cpu.S |   49 +++++++++++++++++++++++++++++++++++------
 1 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/verify_cpu.S b/arch/x86/kernel/verify_cpu.S
index 45b6f8a..d6a0be6 100644
--- a/arch/x86/kernel/verify_cpu.S
+++ b/arch/x86/kernel/verify_cpu.S
@@ -7,6 +7,7 @@
  *	Copyright (c) 2007  Andi Kleen (ak@suse.de)
  *	Copyright (c) 2007  Eric Biederman (ebiederm@xmission.com)
  *	Copyright (c) 2007  Vivek Goyal (vgoyal@in.ibm.com)
+ *	Copyright (c) 2010  Kees Cook (kees.cook@canonical.com)
  *
  * 	This source code is licensed under the GNU General Public License,
  * 	Version 2.  See the file COPYING for more details.
@@ -14,23 +15,22 @@
  *	This is a common code for verification whether CPU supports
  * 	long mode and SSE or not. It is not called directly instead this
  *	file is included at various places and compiled in that context.
- * 	Following are the current usage.
+ *	This file is expected to run in 32bit code.  Currently:
  *
- * 	This file is included by both 16bit and 32bit code.
+ *	arch/x86_64/boot/compressed/head_64.S: Boot cpu verification
+ *	arch/x86_64/kernel/trampoline_64.S: secondary processor verfication
  *
- *	arch/x86_64/boot/setup.S : Boot cpu verification (16bit)
- *	arch/x86_64/boot/compressed/head.S: Boot cpu verification (32bit)
- *	arch/x86_64/kernel/trampoline.S: secondary processor verfication (16bit)
- *	arch/x86_64/kernel/acpi/wakeup.S:Verfication at resume (16bit)
- *
- *	verify_cpu, returns the status of cpu check in register %eax.
+ *	verify_cpu, returns the status of longmode and SSE in register %eax.
  *		0: Success    1: Failure
  *
+ *	On Intel, the XD_DISABLE flag will be cleared as a side-effect.
+ *
  * 	The caller needs to check for the error code and take the action
  * 	appropriately. Either display a message or halt.
  */
 
 #include <asm/cpufeature.h>
+#include <asm/msr-index.h>
 
 verify_cpu:
 	pushfl				# Save caller passed flags
@@ -61,8 +61,41 @@ verify_cpu:
 	cmpl	$0x444d4163,%ecx
 	jnz	verify_cpu_noamd
 	mov	$1,%di			# cpu is from AMD
+	jmp	verify_cpu_check
 
 verify_cpu_noamd:
+	cmpl	$0x756e6547,%ebx        # GenuineIntel?
+	jnz	verify_cpu_check
+	cmpl	$0x49656e69,%edx
+	jnz	verify_cpu_check
+	cmpl	$0x6c65746e,%ecx
+	jnz	verify_cpu_check
+
+	# only call IA32_MISC_ENABLE when:
+	# family > 6 || (family == 6 && model >= 0xd)
+	movl	$0x1, %eax		# check CPU family and model
+	cpuid
+	movl	%eax, %ecx
+
+	andl	$0x0ff00f00, %eax	# mask family and extended family
+	shrl	$8, %eax
+	cmpl	$6, %eax
+	ja	verify_cpu_clear_xd	# family > 6, ok
+	jb	verify_cpu_check	# family < 6, skip
+
+	andl	$0x000f00f0, %ecx	# mask model and extended model
+	shrl	$4, %ecx
+	cmpl	$0xd, %ecx
+	jb	verify_cpu_check	# family == 6, model < 0xd, skip
+
+verify_cpu_clear_xd:
+	movl	$MSR_IA32_MISC_ENABLE, %ecx
+	rdmsr
+	btrl	$2, %edx		# clear MSR_IA32_MISC_ENABLE_XD_DISABLE
+	jnc	verify_cpu_check	# only write MSR if bit was changed
+	wrmsr
+
+verify_cpu_check:
 	movl    $0x1,%eax		# Does the cpu have what it takes
 	cpuid
 	andl	$REQUIRED_MASK0,%edx
-- 
1.7.1


-- 
Kees Cook
Ubuntu Security Team

[PATCH 3/4] x86: call verify_cpu during 32bit CPU startup

[Kees Cook]

The XD_DISABLE-clearing side-effect needs to happen on 32bit CPU
start-up as well.

Signed-off-by: Kees Cook <kees.cook@canonical.com>
---
 arch/x86/kernel/head_32.S    |    6 ++++++
 arch/x86/kernel/verify_cpu.S |    1 +
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 37c3d4b..0dec923 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -309,6 +309,10 @@ ENTRY(startup_32_smp)
 	subl $0x80000001, %eax
 	cmpl $(0x8000ffff-0x80000001), %eax
 	ja 6f
+
+	/* Clear bogus XD_DISABLE bits */
+	call verify_cpu
+
 	mov $0x80000001, %eax
 	cpuid
 	/* Execute Disable bit supported? */
@@ -604,6 +608,8 @@ ignore_int:
 #endif
 	iret
 
+#include "verify_cpu.S"
+
 	__REFDATA
 .align 4
 ENTRY(initial_code)
diff --git a/arch/x86/kernel/verify_cpu.S b/arch/x86/kernel/verify_cpu.S
index d6a0be6..29a6357 100644
--- a/arch/x86/kernel/verify_cpu.S
+++ b/arch/x86/kernel/verify_cpu.S
@@ -19,6 +19,7 @@
  *
  *	arch/x86_64/boot/compressed/head_64.S: Boot cpu verification
  *	arch/x86_64/kernel/trampoline_64.S: secondary processor verfication
+ *	arch/x86_64/kernel/head_32.S: processor startup
  *
  *	verify_cpu, returns the status of longmode and SSE in register %eax.
  *		0: Success    1: Failure
-- 
1.7.1


-- 
Kees Cook
Ubuntu Security Team

[PATCH 4/4] x86: only CPU features determine NX capabilities

[Kees Cook]

Fix the NX feature boot warning when NX is missing to correctly
reflect that BIOSes cannot disable NX now.

Signed-off-by: Kees Cook <kees.cook@canonical.com>
---
 arch/x86/mm/setup_nx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c
index a3250aa..410531d 100644
--- a/arch/x86/mm/setup_nx.c
+++ b/arch/x86/mm/setup_nx.c
@@ -41,7 +41,7 @@ void __init x86_report_nx(void)
 {
 	if (!cpu_has_nx) {
 		printk(KERN_NOTICE "Notice: NX (Execute Disable) protection "
-		       "missing in CPU or disabled in BIOS!\n");
+		       "missing in CPU!\n");
 	} else {
 #if defined(CONFIG_X86_64) || defined(CONFIG_X86_PAE)
 		if (disable_nx) {
-- 
1.7.1


-- 
Kees Cook
Ubuntu Security Team

Re: [PATCH v2 0/4] x86: clear XD_DISABLED flag on Intel to regain NX

[Andi Kleen]

Kees Cook <kees.cook@canonical.com> writes:

> This will clear the MSR_IA32_MISC_ENABLE_XD_DISABLE bit so that NX cannot
> be inappropriately controlled by the BIOS on Intel CPUs.  If NX actually
> needs to be disabled, "noexec=off" can be used.

The patch still seems like a bad idea to me. What happens if 
the NX bit is broken for some reason and the BIOS is right
to disable it?

If there's some VM which doesn't ignore unknown MSR writes
it could also break early, and at best you get an ugly
message and at worst a crash.

Do you have evidence for a lot of systems where NX is disabled
this way without BIOS option? Really such information
should be in the patch description.

If you really need to apply it apply it in some place where
exception handling is possible at least.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH v2 0/4] x86: clear XD_DISABLED flag on Intel to regain NX

[Arjan van de Ven]

On Sat, 19 Jun 2010 10:21:29 +0200
Andi Kleen <andi@firstfloor.org> wrote:

> Kees Cook <kees.cook@canonical.com> writes:
> 
> > This will clear the MSR_IA32_MISC_ENABLE_XD_DISABLE bit so that NX
> > cannot be inappropriately controlled by the BIOS on Intel CPUs.  If
> > NX actually needs to be disabled, "noexec=off" can be used.
> 
> The patch still seems like a bad idea to me. What happens if 
> the NX bit is broken for some reason and the BIOS is right
> to disable it?


overriding the bios like this is almost always a bad idea.
(you're doing a blanket override, not a specific, verified override)

you have no idea if the SMM code can deal with NX, etc etc.

the real answer is "fix your bios setting".
Don't as owner of the machine turn something off in the bios that you
actually want.


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH v2 0/4] x86: clear XD_DISABLED flag on Intel to regain NX

[Kees Cook]

Hi,

On Sat, Jun 19, 2010 at 08:16:42AM -0700, Arjan van de Ven wrote:
> On Sat, 19 Jun 2010 10:21:29 +0200
> Andi Kleen <andi@firstfloor.org> wrote:
> 
> > Kees Cook <kees.cook@canonical.com> writes:
> > 
> > > This will clear the MSR_IA32_MISC_ENABLE_XD_DISABLE bit so that NX
> > > cannot be inappropriately controlled by the BIOS on Intel CPUs.  If
> > > NX actually needs to be disabled, "noexec=off" can be used.
> > 
> > The patch still seems like a bad idea to me. What happens if 
> > the NX bit is broken for some reason and the BIOS is right
> > to disable it?
> 
> 
> overriding the bios like this is almost always a bad idea.
> (you're doing a blanket override, not a specific, verified override)

I've seen other things in the BIOS ignored (IDE bus settings jumps to
mind), so I figured it wasn't strictly bad.  From what I've been able to
gather, this setting is never correct.  If there are situations where it
must be left alone, we could add those as exceptions.

> you have no idea if the SMM code can deal with NX, etc etc.

The pages don't get marked as actually NX until setup_nx() is called, at
which point "noexec=off" would have already been handled, so if that
happens, a system can still boot with that cmdline option.

> the real answer is "fix your bios setting".

Well, the "best" answer is "fix the bios", which is why I got Dell to
fix their BIOSes.  Unfortunately, there are still systems with this
misconfigured.

> Don't as owner of the machine turn something off in the bios that you
> actually want.

Most people don't know/care, so if they do and it's a problem, I thought
using "noexec=off" would be sufficient while still allowing the bulk of
systems to end up with NX correctly enabled.

-Kees

-- 
Kees Cook
Ubuntu Security Team

Re: [PATCH v2 0/4] x86: clear XD_DISABLED flag on Intel to regain NX

[H. Peter Anvin]

SMM is not affected; it doesn't use the kernel page tables.

"Kees Cook" <kees.cook@canonical.com> wrote:

>Hi,
>
>On Sat, Jun 19, 2010 at 08:16:42AM -0700, Arjan van de Ven wrote:
>> On Sat, 19 Jun 2010 10:21:29 +0200
>> Andi Kleen <andi@firstfloor.org> wrote:
>> 
>> > Kees Cook <kees.cook@canonical.com> writes:
>> > 
>> > > This will clear the MSR_IA32_MISC_ENABLE_XD_DISABLE bit so that NX
>> > > cannot be inappropriately controlled by the BIOS on Intel CPUs.  If
>> > > NX actually needs to be disabled, "noexec=off" can be used.
>> > 
>> > The patch still seems like a bad idea to me. What happens if 
>> > the NX bit is broken for some reason and the BIOS is right
>> > to disable it?
>> 
>> 
>> overriding the bios like this is almost always a bad idea.
>> (you're doing a blanket override, not a specific, verified override)
>
>I've seen other things in the BIOS ignored (IDE bus settings jumps to
>mind), so I figured it wasn't strictly bad.  From what I've been able to
>gather, this setting is never correct.  If there are situations where it
>must be left alone, we could add those as exceptions.
>
>> you have no idea if the SMM code can deal with NX, etc etc.
>
>The pages don't get marked as actually NX until setup_nx() is called, at
>which point "noexec=off" would have already been handled, so if that
>happens, a system can still boot with that cmdline option.
>
>> the real answer is "fix your bios setting".
>
>Well, the "best" answer is "fix the bios", which is why I got Dell to
>fix their BIOSes.  Unfortunately, there are still systems with this
>misconfigured.
>
>> Don't as owner of the machine turn something off in the bios that you
>> actually want.
>
>Most people don't know/care, so if they do and it's a problem, I thought
>using "noexec=off" would be sufficient while still allowing the bulk of
>systems to end up with NX correctly enabled.
>
>-Kees
>
>-- 
>Kees Cook
>Ubuntu Security Team

-- 
Sent from my mobile phone.  Please pardon any lack of formatting.

Re: [PATCH v2 0/4] x86: clear XD_DISABLED flag on Intel to regain NX

[Kees Cook]

Hi,

On Sat, Jun 19, 2010 at 10:21:29AM +0200, Andi Kleen wrote:
> Kees Cook <kees.cook@canonical.com> writes:
> 
> > This will clear the MSR_IA32_MISC_ENABLE_XD_DISABLE bit so that NX cannot
> > be inappropriately controlled by the BIOS on Intel CPUs.  If NX actually
> > needs to be disabled, "noexec=off" can be used.
> 
> The patch still seems like a bad idea to me. What happens if 
> the NX bit is broken for some reason and the BIOS is right
> to disable it?

I would think this would be the exception; such broken systems should be
worked around when they are discovered.

> If there's some VM which doesn't ignore unknown MSR writes
> it could also break early, and at best you get an ugly
> message and at worst a crash.

That would be a bug in the VM, but since intel_early_init() already does
the MSR call, such a problem would already exist (and this patch wouldn't
make it worse).

> Do you have evidence for a lot of systems where NX is disabled
> this way without BIOS option? Really such information
> should be in the patch description.

All Dell systems shipping Ubuntu (and RedHat before then) until maybe
mid-2009 had XD_DISABLE set, and several bare boards with AMI BIOSes
have shipped with it too.  It is an unfortunately common scenario from
what I've seen.  Note that both have an option, but most users don't know
or care about it.  Since Linux handles NX fine, it should clear the bit.

> If you really need to apply it apply it in some place where
> exception handling is possible at least.

I think this needs to happen before the EFER gets set, which happens very
early.  If this can be moved somewhere else, I would be very happy to put
it there; I'd like to avoid any possible glitches (though clearing an MSR
bit seems safe, and using that MSR is already known to be safe).

-Kees

-- 
Kees Cook
Ubuntu Security Team