[PATCH] x86: Configure NX support earlier in setup_arch

[PATCH] x86: Configure NX support earlier in setup_arch

[Stuart Hayes]

A page fault can crash the kernel very early if an NX bit is set in a
page table entry, if the CPU doesn't support NX (or if NX support is
disabled in the CPU).  Move the call to x86_configure_nx() earlier
than parse_setup_data(), since that calls early_memremap().

Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
---

--- linux-3.16-rc3/arch/x86/kernel/setup.c.orig	2014-07-02 12:14:50.196308719 -0400
+++ linux-3.16-rc3/arch/x86/kernel/setup.c	2014-07-02 12:11:11.240301228 -0400
@@ -940,6 +940,17 @@ void __init setup_arch(char **cmdline_p)
 
 	iomem_resource.end = (1ULL << boot_cpu_data.x86_phys_bits) - 1;
 	setup_memory_map();
+
+	/*
+	 * x86_configure_nx() is called before parse_setup_data() to detect
+	 * whether hardware doesn't support NX (so that parse_setup_data()
+	 * can safely call early_memremap() without setting the XD bit in
+	 * the PTE, which can cause an unrecoverable page fault). It may
+	 * then be called again from within noexec_setup() during parsing
+	 * early parameters to honor the respective command line option.
+	 */
+	x86_configure_nx();
+
 	parse_setup_data();
 
 	copy_edd();
@@ -974,15 +985,6 @@ void __init setup_arch(char **cmdline_p)
 	strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
 	*cmdline_p = command_line;
 
-	/*
-	 * x86_configure_nx() is called before parse_early_param() to detect
-	 * whether hardware doesn't support NX (so that the early EHCI debug
-	 * console setup can safely call set_fixmap()). It may then be called
-	 * again from within noexec_setup() during parsing early parameters
-	 * to honor the respective command line option.
-	 */
-	x86_configure_nx();
-
 	parse_early_param();
 
 	x86_report_nx();

Re: [PATCH] x86: Configure NX support earlier in setup_arch

[Stuart Hayes]

On 7/2/2014 8:47 PM, Stuart Hayes wrote:

> A page fault can crash the kernel very early if an NX bit is set in a
> page table entry, if the CPU doesn't support NX (or if NX support is
> disabled in the CPU).  Move the call to x86_configure_nx() earlier
> than parse_setup_data(), since that calls early_memremap().
> 
> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
> ---
> 
> --- linux-3.16-rc3/arch/x86/kernel/setup.c.orig	2014-07-02 12:14:50.196308719 -0400
> +++ linux-3.16-rc3/arch/x86/kernel/setup.c	2014-07-02 12:11:11.240301228 -0400
> @@ -940,6 +940,17 @@ void __init setup_arch(char **cmdline_p)
>  
>  	iomem_resource.end = (1ULL << boot_cpu_data.x86_phys_bits) - 1;
>  	setup_memory_map();
> +
> +	/*
> +	 * x86_configure_nx() is called before parse_setup_data() to detect
> +	 * whether hardware doesn't support NX (so that parse_setup_data()
> +	 * can safely call early_memremap() without setting the XD bit in
> +	 * the PTE, which can cause an unrecoverable page fault). It may
> +	 * then be called again from within noexec_setup() during parsing
> +	 * early parameters to honor the respective command line option.
> +	 */
> +	x86_configure_nx();
> +
>  	parse_setup_data();
>  
>  	copy_edd();
> @@ -974,15 +985,6 @@ void __init setup_arch(char **cmdline_p)
>  	strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
>  	*cmdline_p = command_line;
>  
> -	/*
> -	 * x86_configure_nx() is called before parse_early_param() to detect
> -	 * whether hardware doesn't support NX (so that the early EHCI debug
> -	 * console setup can safely call set_fixmap()). It may then be called
> -	 * again from within noexec_setup() during parsing early parameters
> -	 * to honor the respective command line option.
> -	 */
> -	x86_configure_nx();
> -
>  	parse_early_param();
>  
>  	x86_report_nx();


I haven't received any responses... is there a problem with the patch?  Also CCing a couple people.

Re: [PATCH] x86: Configure NX support earlier in setup_arch

[H. Peter Anvin]

On 07/08/2014 03:34 PM, Stuart Hayes wrote:
> 
> I haven't received any responses... is there a problem with the patch?  Also CCing a couple people.
> 

I was on vacation last week and am still catching up.

It would also help if you describe the real-world scenario that made you
trip over this.

	-hpa

Re: [PATCH] x86: Configure NX support earlier in setup_arch

[Stuart Hayes]

On 7/8/2014 5:38 PM, H. Peter Anvin wrote:

> On 07/08/2014 03:34 PM, Stuart Hayes wrote:
>>
>> I haven't received any responses... is there a problem with the patch?  Also CCing a couple people.
>>
> 
> I was on vacation last week and am still catching up.
> 
> It would also help if you describe the real-world scenario that made you
> trip over this.
> 
> 	-hpa
> 


Well... I got this issue because a co-worker tripped over it.  He had NX disabled in BIOS for some reason, and found that linux wouldn't boot--it hung right after grub2.  I guess it took a while to figure out that it was the fact that NX was disabled that caused linux not to come up--and that could happen to other people.  I don't know of any real-world scenarios in which someone would actually prefer to run a recent linux kernel with NX disabled, though.

It looks like some of the other boot paths into the kernel automatically clear the XD_DISABLE bit in the MISC_ENABLE MSR in the CPU (in verify_cpu), but that doesn't happen when grub2 jumps to startup_64 in arch/x86/boot/compressed/head_64.S.  I guess instead of this patch, I could try to make a patch that turns NX back on (somewhere in startup_64), but since the kernel already supports NX being disabled, so I thought maybe just fixing that would be better.  I didn't like seeing the kernel just die without giving any indication of what the problem is.

Stuart

Re: [PATCH] x86: Configure NX support earlier in setup_arch

[Borislav Petkov]

On Wed, Jul 09, 2014 at 07:56:29PM -0500, Stuart Hayes wrote:
> Well... I got this issue because a co-worker tripped over it. He had
> NX disabled in BIOS for some reason, and found that linux wouldn't
> boot--it hung right after grub2. I guess it took a while to figure out
> that it was the fact that NX was disabled that caused linux not to
> come up--and that could happen to other people. I don't know of any
> real-world scenarios in which someone would actually prefer to run a
> recent linux kernel with NX disabled, though.
>
> It looks like some of the other boot paths into the kernel
> automatically clear the XD_DISABLE bit in the MISC_ENABLE MSR in the
> CPU (in verify_cpu), but that doesn't happen when grub2 jumps to
> startup_64 in arch/x86/boot/compressed/head_64.S. I guess instead
> of this patch, I could try to make a patch that turns NX back on
> (somewhere in startup_64), but since the kernel already supports NX
> being disabled, so I thought maybe just fixing that would be better. I
> didn't like seeing the kernel just die without giving any indication
> of what the problem is.

Well, hpa and I were talking about this briefly and this NX disabling
in the BIOS is probably for some broken legacy applications/OSes. Linux
enables NX unconditionally very early because disabling it is a very bad
idea anyway, security-wise.

So, if this is just a random trip over of a co-worker and doesn't have
any sensible use case, I'd rather leave it as is an don't fix it at all.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] x86: Configure NX support earlier in setup_arch

[H. Peter Anvin]

Oh, it is a case of Grub2 utter braindamage.  That figures.  I guess we need to invoke verify_cpu in yet another place.



On July 14, 2014 10:22:25 AM PDT, Borislav Petkov <bp@suse.de> wrote:
>On Wed, Jul 09, 2014 at 07:56:29PM -0500, Stuart Hayes wrote:
>> Well... I got this issue because a co-worker tripped over it. He had
>> NX disabled in BIOS for some reason, and found that linux wouldn't
>> boot--it hung right after grub2. I guess it took a while to figure
>out
>> that it was the fact that NX was disabled that caused linux not to
>> come up--and that could happen to other people. I don't know of any
>> real-world scenarios in which someone would actually prefer to run a
>> recent linux kernel with NX disabled, though.
>>
>> It looks like some of the other boot paths into the kernel
>> automatically clear the XD_DISABLE bit in the MISC_ENABLE MSR in the
>> CPU (in verify_cpu), but that doesn't happen when grub2 jumps to
>> startup_64 in arch/x86/boot/compressed/head_64.S. I guess instead
>> of this patch, I could try to make a patch that turns NX back on
>> (somewhere in startup_64), but since the kernel already supports NX
>> being disabled, so I thought maybe just fixing that would be better.
>I
>> didn't like seeing the kernel just die without giving any indication
>> of what the problem is.
>
>Well, hpa and I were talking about this briefly and this NX disabling
>in the BIOS is probably for some broken legacy applications/OSes. Linux
>enables NX unconditionally very early because disabling it is a very
>bad
>idea anyway, security-wise.
>
>So, if this is just a random trip over of a co-worker and doesn't have
>any sensible use case, I'd rather leave it as is an don't fix it at
>all.

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

Re: [PATCH] x86: Configure NX support earlier in setup_arch

[Borislav Petkov]

On Mon, Jul 14, 2014 at 10:28:43AM -0700, H. Peter Anvin wrote:
> Oh, it is a case of Grub2 utter braindamage. That figures. I guess we
> need to invoke verify_cpu in yet another place.

How so? The bug report speaks of disabling NX in the BIOS, I'm assuming
they're setting bit 34 in IA32_MISC_ENABLE.

How would that affect grub2?

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

RE: [PATCH] x86: Configure NX support earlier in setup_arch

[B_B_Singh]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2654 bytes --]

Hi Boris,

I feel this can be a valid scenario where user wants to disable the memory protection (NX flag disable) for his requirement, in that case he will hit this issue.
So request you to please revisit this patch.

Regards
Balaji Singh

-----Original Message-----
From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Borislav Petkov
Sent: Monday, July 14, 2014 10:52 PM
To: Stuart Hayes
Cc: H. Peter Anvin; tglx@linutronix.de; mingo@redhat.com; x86@kernel.org; linux-kernel@vger.kernel.org; matt.fleming@intel.com; bp@suse.de
Subject: Re: [PATCH] x86: Configure NX support earlier in setup_arch

On Wed, Jul 09, 2014 at 07:56:29PM -0500, Stuart Hayes wrote:
> Well... I got this issue because a co-worker tripped over it. He had 
> NX disabled in BIOS for some reason, and found that linux wouldn't 
> boot--it hung right after grub2. I guess it took a while to figure out 
> that it was the fact that NX was disabled that caused linux not to 
> come up--and that could happen to other people. I don't know of any 
> real-world scenarios in which someone would actually prefer to run a 
> recent linux kernel with NX disabled, though.
>
> It looks like some of the other boot paths into the kernel 
> automatically clear the XD_DISABLE bit in the MISC_ENABLE MSR in the 
> CPU (in verify_cpu), but that doesn't happen when grub2 jumps to
> startup_64 in arch/x86/boot/compressed/head_64.S. I guess instead of 
> this patch, I could try to make a patch that turns NX back on 
> (somewhere in startup_64), but since the kernel already supports NX 
> being disabled, so I thought maybe just fixing that would be better. I 
> didn't like seeing the kernel just die without giving any indication 
> of what the problem is.

Well, hpa and I were talking about this briefly and this NX disabling in the BIOS is probably for some broken legacy applications/OSes. Linux enables NX unconditionally very early because disabling it is a very bad idea anyway, security-wise.

So, if this is just a random trip over of a co-worker and doesn't have any sensible use case, I'd rather leave it as is an don't fix it at all.

--
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] x86: Configure NX support earlier in setup_arch

[Borislav Petkov]

On Thu, Nov 13, 2014 at 12:09:02PM +0530, B_B_Singh@DELL.com wrote:
> I feel this can be a valid scenario where user wants to disable the
> memory protection (NX flag disable) for his requirement, in that case he
> will hit this issue. So request you to please revisit this patch.

First of all, please do not top-post. Here's why:

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

Revisit this patch how? I'm not sure I understand...

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

RE: [PATCH] x86: Configure NX support earlier in setup_arch

[B_B_Singh]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1273 bytes --]


> -----Original Message-----
> From: Borislav Petkov [mailto:bp@suse.de]
> Sent: Thursday, November 13, 2014 6:01 PM
> To: Singh, B B
> Cc: stuart.w.hayes@gmail.com; hpa@zytor.com; tglx@linutronix.de;
> mingo@redhat.com; x86@kernel.org; linux-kernel@vger.kernel.org;
> matt.fleming@intel.com; Jiri Kosina
> Subject: Re: [PATCH] x86: Configure NX support earlier in setup_arch
> 
> On Thu, Nov 13, 2014 at 12:09:02PM +0530, B_B_Singh@DELL.com wrote:
> > I feel this can be a valid scenario where user wants to disable the
> > memory protection (NX flag disable) for his requirement, in that case
> > he will hit this issue. So request you to please revisit this patch.
> 
> First of all, please do not top-post. Here's why:
> 
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?
> 

Sorry Boris, will make note on this & follow the same.

> Revisit this patch how? I'm not sure I understand...

I got an answer from Thomas, will check with BIOS team & come back. 

Thanks
Balaji Singh
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] x86: Configure NX support earlier in setup_arch

[Thomas Gleixner]

On Thu, 13 Nov 2014, Borislav Petkov wrote:
> Revisit this patch how? I'm not sure I understand...

X86_64 starts with:

pteval_t __supported_pte_mask __read_mostly = ~0;

while i386 starts with:

pteval_t __supported_pte_mask __read_mostly = ~(_PAGE_NX | _PAGE_GLOBAL);

Now if the stupid BIOS disabled NX via setting bit 34 in
IA32_MISC_ENABLE, then X86_FEATURE_NX is off, but at the point where
we call x86_configure_nx() we already used the supported_pte_mask with
the NX bit set in the early remap code and accessed the mappings.

On 32bit we are safe because the early maps exclude NX at startup and
only enable it in x86_configure_nx().

That's one part of the issue. The other is that grub2 does not call in
via the trampoline, so we don't call verify_cpu. verify_cpu clears bit
34 in IA32_MISC_ENABLE depending on the cpu family/model, which is
true for 64bit machines.

So I think moving x86_configure_nx() to a point before we actually
create mappings is a sane thing to do in any case.

But of course we should also clear the stupid disable bit
unconditionally independent of the entry path for all cpus which
support it.

Thanks,

	tglx

RE: [PATCH] x86: Configure NX support earlier in setup_arch

[Thomas Gleixner]

On Thu, 13 Nov 2014, B_B_Singh@DELL.com wrote:
> I feel this can be a valid scenario where user wants to disable the
> memory protection (NX flag disable) for his requirement, in that
> case he will hit this issue.

It's not about feelings. If the user wants to disable NX he can do so
already via the kernel command line.

What that patch is useful for is to prevent an hard to debug boot
crash if the kernel happens to run on some wreckaged BIOS. But it's
not there to support and encourage braindamaged BIOS nonsense.

Thanks,

	tglx