linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Security] [PATCH v5 0/4] x86: clear XD_DISABLED flag on Intel to regain NX
@ 2010-11-10 18:35 Kees Cook
  2010-11-10 18:35 ` [PATCH 1/4] x86: rename verify_cpu_64.S to verify_cpu.S Kees Cook
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Kees Cook @ 2010-11-10 18:35 UTC (permalink / raw)
  To: x86, linux-kernel; +Cc: Pekka Enberg, Alan Cox, Ingo Molnar

Resending with Alan Cox's Acks, at Ingo's request.

Intel CPUs have an additional MSR bit to indicate if the BIOS was
configured to disable NX. This bit was traditionally used for operating
systems that did not understand how to handle the NX bit. Since Linux
understands this, this BIOS flag should be ignored by default.

In a review[1] of reported hardware being used by Ubuntu bug reporters,
almost 10% of systems had an incorrectly configured BIOS, leaving their
systems unable to use the NX features of their CPU.

This change will clear the MSR_IA32_MISC_ENABLE_XD_DISABLE bit so that NX
cannot be inappropriately controlled by the BIOS on Intel CPUs. If, under
very strange hardware configurations, NX actually needs to be disabled,
"noexec=off" can be used to restore the prior behavior.

-Kees

[1] http://www.outflux.net/blog/archives/2010/02/18/data-mining-for-nx-bit/

---
Changelog:
v2 - rearranged use of verify_cpu()
v3 - resent, show stats on systems that need it
v4 - expanded commit messages, added acks
v5 - added acks

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/4] x86: rename verify_cpu_64.S to verify_cpu.S
  2010-11-10 18:35 [Security] [PATCH v5 0/4] x86: clear XD_DISABLED flag on Intel to regain NX Kees Cook
@ 2010-11-10 18:35 ` Kees Cook
  2010-11-10 18:35 ` [PATCH 2/4] x86: clear XD_DISABLED flag on Intel to regain NX Kees Cook
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 18+ messages in thread
From: Kees Cook @ 2010-11-10 18:35 UTC (permalink / raw)
  To: x86, linux-kernel; +Cc: Pekka Enberg, Alan Cox, Ingo Molnar

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

Signed-off-by: Kees Cook <kees.cook@canonical.com>
Acked-by: Pekka Enberg <penberg@kernel.org>
Acked-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 arch/x86/boot/compressed/head_64.S                |    2 +-
 arch/x86/kernel/trampoline_64.S                   |    2 +-
 arch/x86/kernel/{verify_cpu_64.S => verify_cpu.S} |    0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename arch/x86/kernel/{verify_cpu_64.S => verify_cpu.S} (100%)

diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 52f85a1..35af09d 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_64.S b/arch/x86/kernel/verify_cpu.S
similarity index 100%
rename from arch/x86/kernel/verify_cpu_64.S
rename to arch/x86/kernel/verify_cpu.S
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/4] x86: clear XD_DISABLED flag on Intel to regain NX
  2010-11-10 18:35 [Security] [PATCH v5 0/4] x86: clear XD_DISABLED flag on Intel to regain NX Kees Cook
  2010-11-10 18:35 ` [PATCH 1/4] x86: rename verify_cpu_64.S to verify_cpu.S Kees Cook
@ 2010-11-10 18:35 ` Kees Cook
       [not found]   ` <tip-ae84739c27b6b3725993202fe02ff35ab86468e1@git.kernel.org>
  2010-11-10 18:35 ` [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup Kees Cook
  2010-11-10 18:35 ` [PATCH 4/4] x86: only CPU features determine NX capabilities Kees Cook
  3 siblings, 1 reply; 18+ messages in thread
From: Kees Cook @ 2010-11-10 18:35 UTC (permalink / raw)
  To: x86, linux-kernel; +Cc: Pekka Enberg, Alan Cox, Ingo Molnar

Intel CPUs have an additional MSR bit to indicate if the BIOS was
configured to disable the NX cpu feature. This bit was traditionally
used for operating systems that did not understand how to handle the
NX bit. Since Linux understands this, this BIOS flag should be ignored
by default.

In a review[1] of reported hardware being used by Ubuntu bug reporters,
almost 10% of systems had an incorrectly configured BIOS, leaving their
systems unable to use the NX features of their CPU.

This change will clear the MSR_IA32_MISC_ENABLE_XD_DISABLE bit so that NX
cannot be inappropriately controlled by the BIOS on Intel CPUs. If, under
very strange hardware configurations, NX actually needs to be disabled,
"noexec=off" can be used to restore the prior behavior.

[1] http://www.outflux.net/blog/archives/2010/02/18/data-mining-for-nx-bit/

Signed-off-by: Kees Cook <kees.cook@canonical.com>
Acked-by: Pekka Enberg <penberg@kernel.org>
Acked-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 arch/x86/kernel/verify_cpu.S |   48 +++++++++++++++++++++++++++++++++++-------
 1 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/verify_cpu.S b/arch/x86/kernel/verify_cpu.S
index 56a8c2a..ccb4136 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,18 +15,16 @@
  *	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.
  */
@@ -62,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.2.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup
  2010-11-10 18:35 [Security] [PATCH v5 0/4] x86: clear XD_DISABLED flag on Intel to regain NX Kees Cook
  2010-11-10 18:35 ` [PATCH 1/4] x86: rename verify_cpu_64.S to verify_cpu.S Kees Cook
  2010-11-10 18:35 ` [PATCH 2/4] x86: clear XD_DISABLED flag on Intel to regain NX Kees Cook
@ 2010-11-10 18:35 ` Kees Cook
  2010-11-14  6:33   ` Yinghai Lu
  2010-11-10 18:35 ` [PATCH 4/4] x86: only CPU features determine NX capabilities Kees Cook
  3 siblings, 1 reply; 18+ messages in thread
From: Kees Cook @ 2010-11-10 18:35 UTC (permalink / raw)
  To: x86, linux-kernel; +Cc: Pekka Enberg, Alan Cox, Ingo Molnar

The XD_DISABLE-clearing side-effect needs to happen for both 32bit
and 64bit, but the 32bit init routines were not calling verify_cpu()
yet. This adds that call to gain the side-effect.

The longmode/SSE tests being performed in verify_cpu() need to happen very
early for 64bit but not for 32bit. Instead of including it in two places
for 32bit, we can just include it once in arch/x86/kernel/head_32.S.

Signed-off-by: Kees Cook <kees.cook@canonical.com>
Acked-by: Pekka Enberg <penberg@kernel.org>
Acked-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 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 bcece91..fdaea52 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -314,6 +314,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? */
@@ -609,6 +613,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 ccb4136..5644b4b 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.2.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/4] x86: only CPU features determine NX capabilities
  2010-11-10 18:35 [Security] [PATCH v5 0/4] x86: clear XD_DISABLED flag on Intel to regain NX Kees Cook
                   ` (2 preceding siblings ...)
  2010-11-10 18:35 ` [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup Kees Cook
@ 2010-11-10 18:35 ` Kees Cook
  3 siblings, 0 replies; 18+ messages in thread
From: Kees Cook @ 2010-11-10 18:35 UTC (permalink / raw)
  To: x86, linux-kernel; +Cc: Pekka Enberg, Alan Cox, Ingo Molnar

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>
Acked-by: Pekka Enberg <penberg@kernel.org>
Acked-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 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.2.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup
  2010-11-10 18:35 ` [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup Kees Cook
@ 2010-11-14  6:33   ` Yinghai Lu
  0 siblings, 0 replies; 18+ messages in thread
From: Yinghai Lu @ 2010-11-14  6:33 UTC (permalink / raw)
  To: Kees Cook; +Cc: x86, linux-kernel, Pekka Enberg, Alan Cox, Ingo Molnar

On Wed, Nov 10, 2010 at 10:35 AM, Kees Cook <kees.cook@canonical.com> wrote:
> The XD_DISABLE-clearing side-effect needs to happen for both 32bit
> and 64bit, but the 32bit init routines were not calling verify_cpu()
> yet. This adds that call to gain the side-effect.
>
> The longmode/SSE tests being performed in verify_cpu() need to happen very
> early for 64bit but not for 32bit. Instead of including it in two places
> for 32bit, we can just include it once in arch/x86/kernel/head_32.S.
>
> Signed-off-by: Kees Cook <kees.cook@canonical.com>
> Acked-by: Pekka Enberg <penberg@kernel.org>
> Acked-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
> ---
>  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 bcece91..fdaea52 100644
> --- a/arch/x86/kernel/head_32.S
> +++ b/arch/x86/kernel/head_32.S
> @@ -314,6 +314,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? */
> @@ -609,6 +613,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 ccb4136..5644b4b 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

should change those x86_64 to x86.

Yinghai

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [tip:x86/cpu] x86, cpu: Clear XD_DISABLED flag on Intel to regain NX
       [not found]   ` <tip-ae84739c27b6b3725993202fe02ff35ab86468e1@git.kernel.org>
@ 2010-12-08 22:27     ` Kees Cook
  2010-12-22 11:17       ` Ingo Molnar
  0 siblings, 1 reply; 18+ messages in thread
From: Kees Cook @ 2010-12-08 22:27 UTC (permalink / raw)
  To: linux-kernel; +Cc: hpa, mingo, penberg, alan, tglx, hpa

On Thu, Nov 11, 2010 at 02:49:04AM +0000, tip-bot for Kees Cook wrote:
> Commit-ID:  ae84739c27b6b3725993202fe02ff35ab86468e1
> Gitweb:     http://git.kernel.org/tip/ae84739c27b6b3725993202fe02ff35ab86468e1
> Author:     Kees Cook <kees.cook@canonical.com>
> AuthorDate: Wed, 10 Nov 2010 10:35:52 -0800
> Committer:  H. Peter Anvin <hpa@linux.intel.com>
> CommitDate: Wed, 10 Nov 2010 15:42:54 -0800
> 
> x86, cpu: Clear XD_DISABLED flag on Intel to regain NX

Just some quick positive feedback on this patch series: I've had at least
one confirmed report of a user with an Asus system that had NX disabled
(in the BIOS which offered no toggle to switch it on) is able to use
the NX bit on their CPU finally. Additionally, no regressions have been
identified either, and there has been a fair number of people running
with the patch for a few weeks now in Ubuntu's development kernel.

-Kees

-- 
Kees Cook
Ubuntu Security Team

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [tip:x86/cpu] x86, cpu: Clear XD_DISABLED flag on Intel to regain NX
  2010-12-08 22:27     ` [tip:x86/cpu] x86, cpu: Clear " Kees Cook
@ 2010-12-22 11:17       ` Ingo Molnar
  0 siblings, 0 replies; 18+ messages in thread
From: Ingo Molnar @ 2010-12-22 11:17 UTC (permalink / raw)
  To: Kees Cook; +Cc: linux-kernel, hpa, mingo, penberg, alan, tglx, hpa


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

> On Thu, Nov 11, 2010 at 02:49:04AM +0000, tip-bot for Kees Cook wrote:
> > Commit-ID:  ae84739c27b6b3725993202fe02ff35ab86468e1
> > Gitweb:     http://git.kernel.org/tip/ae84739c27b6b3725993202fe02ff35ab86468e1
> > Author:     Kees Cook <kees.cook@canonical.com>
> > AuthorDate: Wed, 10 Nov 2010 10:35:52 -0800
> > Committer:  H. Peter Anvin <hpa@linux.intel.com>
> > CommitDate: Wed, 10 Nov 2010 15:42:54 -0800
> > 
> > x86, cpu: Clear XD_DISABLED flag on Intel to regain NX
> 
> Just some quick positive feedback on this patch series: I've had at least one 
> confirmed report of a user with an Asus system that had NX disabled (in the BIOS 
> which offered no toggle to switch it on) is able to use the NX bit on their CPU 
> finally. Additionally, no regressions have been identified either, and there has 
> been a fair number of people running with the patch for a few weeks now in 
> Ubuntu's development kernel.

Nice!

	Ingo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup
  2010-11-09 22:17 [Security] [PATCH v4 0/4] x86: clear XD_DISABLED flag on Intel to regain NX Kees Cook
@ 2010-11-09 22:18 ` Kees Cook
  0 siblings, 0 replies; 18+ messages in thread
From: Kees Cook @ 2010-11-09 22:18 UTC (permalink / raw)
  To: x86, linux-kernel

The XD_DISABLE-clearing side-effect needs to happen for both 32bit
and 64bit, but the 32bit init routines were not calling verify_cpu()
yet. This adds that call to gain the side-effect.

The longmode/SSE tests being performed in verify_cpu() need to happen very
early for 64bit but not for 32bit. Instead of including it in two places
for 32bit, we can just include it once in arch/x86/kernel/head_32.S.

Signed-off-by: Kees Cook <kees.cook@canonical.com>
Acked-by: Pekka Enberg <penberg@kernel.org>
---
 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 bcece91..fdaea52 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -314,6 +314,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? */
@@ -609,6 +613,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 ccb4136..5644b4b 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.2.3


-- 
Kees Cook
Ubuntu Security Team

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup
  2010-11-09 20:48             ` Kees Cook
@ 2010-11-09 20:50               ` Pekka Enberg
  0 siblings, 0 replies; 18+ messages in thread
From: Pekka Enberg @ 2010-11-09 20:50 UTC (permalink / raw)
  To: Kees Cook; +Cc: x86, linux-kernel

On 9.11.2010 22.48, Kees Cook wrote:
> Hi Pekka,
>
> On Tue, Nov 09, 2010 at 10:28:16PM +0200, Pekka Enberg wrote:
>> So why can't we do patch 2/4 XD_DISABLE clearing in
>> early_init_intel()?
> Because it's too late, unfortunately. I went around a few times about this
> with Peter Anvin, and ultimately he agreed that it needed to go in
> verify_cpu() for now.

OK, thanks for the explanation. A link to the previous discussion would 
have been helpful here.

Acked-by: Pekka Enberg <penberg@kernel.org>

for the whole series.

             Pekka

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup
  2010-11-09 20:28           ` Pekka Enberg
@ 2010-11-09 20:48             ` Kees Cook
  2010-11-09 20:50               ` Pekka Enberg
  0 siblings, 1 reply; 18+ messages in thread
From: Kees Cook @ 2010-11-09 20:48 UTC (permalink / raw)
  To: Pekka Enberg; +Cc: x86, linux-kernel

Hi Pekka,

On Tue, Nov 09, 2010 at 10:28:16PM +0200, Pekka Enberg wrote:
> So why can't we do patch 2/4 XD_DISABLE clearing in
> early_init_intel()?

Because it's too late, unfortunately. I went around a few times about this
with Peter Anvin, and ultimately he agreed that it needed to go in
verify_cpu() for now.

> Why do we want to call verify_cpu() from
> arch/x86/kernel/head_32.S and not from
> arch/x86/boot/compressed/head_32.S like we do on 64-bit?

Because the longmode/SSE tests being performed in verify_cpu() need to
happen that early for 64bit. Instead of including it in two places for
32bit, we can just include it once in arch/x86/kernel/head_32.S.

-Kees

-- 
Kees Cook
Ubuntu Security Team

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup
  2010-11-09 19:56         ` Kees Cook
@ 2010-11-09 20:28           ` Pekka Enberg
  2010-11-09 20:48             ` Kees Cook
  0 siblings, 1 reply; 18+ messages in thread
From: Pekka Enberg @ 2010-11-09 20:28 UTC (permalink / raw)
  To: Kees Cook; +Cc: x86, linux-kernel

Hi Kees,

On Tue, Nov 09, 2010 at 09:46:39PM +0200, Pekka Enberg wrote:
>> I actually don't see 0/4 or 2/4 on LKML yet. It would be better to
>> have the rationale in the patch that changes the code for future
>> reference. The summary email will be lost in the noise much more
>> easily.

On Tue, Nov 9, 2010 at 9:56 PM, Kees Cook <kees.cook@canonical.com> wrote:
> Looks like delivery delays somewhere. I see it in the archive:
> http://lkml.org/lkml/2010/11/9/380

So why can't we do patch 2/4 XD_DISABLE clearing in
early_init_intel()? Why do we want to call verify_cpu() from
arch/x86/kernel/head_32.S and not from
arch/x86/boot/compressed/head_32.S like we do on 64-bit?

                        Pekka

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup
  2010-11-09 19:46       ` Pekka Enberg
@ 2010-11-09 19:56         ` Kees Cook
  2010-11-09 20:28           ` Pekka Enberg
  0 siblings, 1 reply; 18+ messages in thread
From: Kees Cook @ 2010-11-09 19:56 UTC (permalink / raw)
  To: Pekka Enberg; +Cc: x86, linux-kernel

On Tue, Nov 09, 2010 at 09:46:39PM +0200, Pekka Enberg wrote:
> I actually don't see 0/4 or 2/4 on LKML yet. It would be better to
> have the rationale in the patch that changes the code for future
> reference. The summary email will be lost in the noise much more
> easily.

Looks like delivery delays somewhere. I see it in the archive:
http://lkml.org/lkml/2010/11/9/380

But sure, I can improve the commit messages further.

-Kees

-- 
Kees Cook
Ubuntu Security Team

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup
  2010-11-09 19:19     ` Kees Cook
@ 2010-11-09 19:46       ` Pekka Enberg
  2010-11-09 19:56         ` Kees Cook
  0 siblings, 1 reply; 18+ messages in thread
From: Pekka Enberg @ 2010-11-09 19:46 UTC (permalink / raw)
  To: Kees Cook; +Cc: x86, linux-kernel

> On Tue, Nov 09, 2010 at 09:09:18PM +0200, Pekka Enberg wrote:
>> On Tue, Nov 9, 2010 at 8:15 PM, Kees Cook <kees.cook@canonical.com> wrote:
>> > 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>
>>
>> The patch description here is pretty damn terse. Why do we need the
>> clearing for? Does not clearing XD_DISABLE cause some problem?

On Tue, Nov 9, 2010 at 9:19 PM, Kees Cook <kees.cook@canonical.com> wrote:
> The clearing needs to happen for both 32bit and 64bit, but the 32bit init
> routines were not calling verify_cpu() yet. This adds that path to gain the
> side-effect. (See patch 0 for why clearing XD_DISABLE is important.)

I actually don't see 0/4 or 2/4 on LKML yet. It would be better to
have the rationale in the patch that changes the code for future
reference. The summary email will be lost in the noise much more
easily.

                        Pekka

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup
  2010-11-09 19:09   ` Pekka Enberg
@ 2010-11-09 19:19     ` Kees Cook
  2010-11-09 19:46       ` Pekka Enberg
  0 siblings, 1 reply; 18+ messages in thread
From: Kees Cook @ 2010-11-09 19:19 UTC (permalink / raw)
  To: Pekka Enberg; +Cc: x86, linux-kernel

On Tue, Nov 09, 2010 at 09:09:18PM +0200, Pekka Enberg wrote:
> On Tue, Nov 9, 2010 at 8:15 PM, Kees Cook <kees.cook@canonical.com> wrote:
> > 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>
> 
> The patch description here is pretty damn terse. Why do we need the
> clearing for? Does not clearing XD_DISABLE cause some problem?

The clearing needs to happen for both 32bit and 64bit, but the 32bit init
routines were not calling verify_cpu() yet. This adds that path to gain the
side-effect. (See patch 0 for why clearing XD_DISABLE is important.)

-- 
Kees Cook
Ubuntu Security Team

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup
  2010-11-09 18:15 ` [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup Kees Cook
@ 2010-11-09 19:09   ` Pekka Enberg
  2010-11-09 19:19     ` Kees Cook
  0 siblings, 1 reply; 18+ messages in thread
From: Pekka Enberg @ 2010-11-09 19:09 UTC (permalink / raw)
  To: Kees Cook; +Cc: x86, linux-kernel

On Tue, Nov 9, 2010 at 8:15 PM, Kees Cook <kees.cook@canonical.com> wrote:
> 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>

The patch description here is pretty damn terse. Why do we need the
clearing for? Does not clearing XD_DISABLE cause some problem?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup
  2010-11-09 18:11 [Security] [PATCH v3 0/4] x86: clear XD_DISABLED flag on Intel to regain NX Kees Cook
@ 2010-11-09 18:15 ` Kees Cook
  2010-11-09 19:09   ` Pekka Enberg
  0 siblings, 1 reply; 18+ messages in thread
From: Kees Cook @ 2010-11-09 18:15 UTC (permalink / raw)
  To: x86, linux-kernel

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 bcece91..fdaea52 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -314,6 +314,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? */
@@ -609,6 +613,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 ccb4136..5644b4b 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.2.3


-- 
Kees Cook
Ubuntu Security Team

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup
  2010-06-19  5:50 [PATCH v2 0/4] x86: clear XD_DISABLED flag on Intel to regain NX Kees Cook
@ 2010-06-19  5:52 ` Kees Cook
  0 siblings, 0 replies; 18+ messages in thread
From: Kees Cook @ 2010-06-19  5:52 UTC (permalink / raw)
  To: x86
  Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Alexander Potashev,
	Tim Abbott, Sam Ravnborg, Jan Beulich, Jeremy Fitzhardinge,
	linux-kernel

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2010-12-22 11:17 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-10 18:35 [Security] [PATCH v5 0/4] x86: clear XD_DISABLED flag on Intel to regain NX Kees Cook
2010-11-10 18:35 ` [PATCH 1/4] x86: rename verify_cpu_64.S to verify_cpu.S Kees Cook
2010-11-10 18:35 ` [PATCH 2/4] x86: clear XD_DISABLED flag on Intel to regain NX Kees Cook
     [not found]   ` <tip-ae84739c27b6b3725993202fe02ff35ab86468e1@git.kernel.org>
2010-12-08 22:27     ` [tip:x86/cpu] x86, cpu: Clear " Kees Cook
2010-12-22 11:17       ` Ingo Molnar
2010-11-10 18:35 ` [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup Kees Cook
2010-11-14  6:33   ` Yinghai Lu
2010-11-10 18:35 ` [PATCH 4/4] x86: only CPU features determine NX capabilities Kees Cook
  -- strict thread matches above, loose matches on Subject: below --
2010-11-09 22:17 [Security] [PATCH v4 0/4] x86: clear XD_DISABLED flag on Intel to regain NX Kees Cook
2010-11-09 22:18 ` [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup Kees Cook
2010-11-09 18:11 [Security] [PATCH v3 0/4] x86: clear XD_DISABLED flag on Intel to regain NX Kees Cook
2010-11-09 18:15 ` [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup Kees Cook
2010-11-09 19:09   ` Pekka Enberg
2010-11-09 19:19     ` Kees Cook
2010-11-09 19:46       ` Pekka Enberg
2010-11-09 19:56         ` Kees Cook
2010-11-09 20:28           ` Pekka Enberg
2010-11-09 20:48             ` Kees Cook
2010-11-09 20:50               ` Pekka Enberg
2010-06-19  5:50 [PATCH v2 0/4] x86: clear XD_DISABLED flag on Intel to regain NX Kees Cook
2010-06-19  5:52 ` [PATCH 3/4] x86: call verify_cpu during 32bit CPU startup Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).