From: H. Peter Anvin on
On 06/17/2010 03:13 PM, Kees Cook wrote:
> 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(a)canonical.com>
> ---
> arch/x86/kernel/head_32.S | 19 +++++++++++++++++++
> arch/x86/kernel/head_64.S | 18 ++++++++++++++++++
> arch/x86/mm/setup_nx.c | 2 +-
> 3 files changed, 38 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
> index 37c3d4b..111e434 100644
> --- a/arch/x86/kernel/head_32.S
> +++ b/arch/x86/kernel/head_32.S
> @@ -309,6 +309,25 @@ ENTRY(startup_32_smp)
> subl $0x80000001, %eax
> cmpl $(0x8000ffff-0x80000001), %eax
> ja 6f
> +
> + /* Is this "GenuineIntel"? */
> + movl $0x0, %eax
> + cpuid
> + cmpl $0x756e6547, %ebx
> + jnz 5f
> + cmpl $0x49656e69, %edx
> + jnz 5f
> + cmpl $0x6c65746e, %ecx
> + jnz 5f
> +
> + /* Clear MSR_IA32_MISC_ENABLE_XD_DISABLE if set */
> + movl $MSR_IA32_MISC_ENABLE, %ecx
> + rdmsr
> + btrl $2, %edx
> + jnc 5f
> + wrmsr
> +
> +5:
> mov $0x80000001, %eax
> cpuid
> /* Execute Disable bit supported? */

Multiple problems with this code.

a) Not all Intel CPUs with extended CPUID levels have
MSR_IA32_MISC_ENABLE bit 34. Since we can't take traps here we would
have to know positively that we aren't going to trip on anything.
b) For 64 bits, this should go into verify_cpu_64.S, and since that is
32-bit code anyway, it would be best if we could merge the 32- and
64-bit code into that file; it already simply returns a value
that could be ignored on 32 bits.

-hpa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Yuhong Bao on

> a) Not all Intel CPUs with extended CPUID levels have
> MSR_IA32_MISC_ENABLE bit 34. Since we can't take traps here we would
> have to know positively that we aren't going to trip on anything.
It won't as these older Intel CPUs are always going to read this bit as zero, and even if no checking for this is include all the wrmsr would do is write the same value back, which won't cause a trap.

Yuhong Bao

_________________________________________________________________
Hotmail has tools for the New Busy. Search, chat and e-mail from your inbox.
http://www.windowslive.com/campaign/thenewbusy?ocid=PID28326::T:WLMTAGL:ON:WL:en-US:WM_HMP:042010_1--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on
Yes, I mentally flipped the sense of the bit... talked to Kees already.

"Yuhong Bao" <yuhongbao_386(a)hotmail.com> wrote:

>
>> a) Not all Intel CPUs with extended CPUID levels have
>> MSR_IA32_MISC_ENABLE bit 34. Since we can't take traps here we would
>> have to know positively that we aren't going to trip on anything.
>It won't as these older Intel CPUs are always going to read this bit as zero, and even if no checking for this is include all the wrmsr would do is write the same value back, which won't cause a trap.
>
>Yuhong Bao
>
>_________________________________________________________________
>Hotmail has tools for the New Busy. Search, chat and e-mail from your inbox.
>http://www.windowslive.com/campaign/thenewbusy?ocid=PID28326::T:WLMTAGL:ON:WL:en-US:WM_HMP:042010_1

--
Sent from my mobile phone. Please pardon any lack of formatting.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andi Kleen on
Kees Cook <kees.cook(a)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(a)linux.intel.com -- Speaking for myself only.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on
SMM is not affected; it doesn't use the kernel page tables.

"Kees Cook" <kees.cook(a)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(a)firstfloor.org> wrote:
>>
>> > Kees Cook <kees.cook(a)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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/