[PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Kosuke Tatsukawa]

The kernel panics early in boot on a x86_64 server if the eXecute
Disable (XD) bit is set to disabled in the uEFI firmware.  The message
in the kernel log buffer looks like below.
------------------------------------------------------------------------
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.4.0-rc3 #1
[    0.000000]  0000000000000000 261c6fa13723be1b ffffffff819b7e40 ffffffff8131f320
[    0.000000]  ffffffffffffffff ffffffff819b7f30 ffffffff81b261b0 000000000000001c
[    0.000000]  ffffffff81d77a1c 0000000000000010 00000000be35a000 ffffffffff200000
[    0.000000] Call Trace:
[    0.000000]  [<ffffffff8131f320>] dump_stack+0x44/0x64
[    0.000000]  [<ffffffff81b261b0>] early_idt_handler_common+0x90/0xd0
[    0.000000]  [<ffffffff81b2f1c5>] ? setup_arch+0x1f1/0xce0
[    0.000000]  [<ffffffff81b2f1c5>] ? setup_arch+0x1f1/0xce0
[    0.000000]  [<ffffffff81b26120>] ? early_idt_handler_array+0x120/0x120
[    0.000000]  [<ffffffff81b26d81>] start_kernel+0xe6/0x4f0
[    0.000000]  [<ffffffff81b26120>] ? early_idt_handler_array+0x120/0x120
[    0.000000]  [<ffffffff81b26120>] ? early_idt_handler_array+0x120/0x120
[    0.000000]  [<ffffffff81b265ee>] x86_64_start_reservations+0x2a/0x2c
[    0.000000]  [<ffffffff81b2673c>] x86_64_start_kernel+0x14c/0x16f
[    0.000000] RIP 0x80000000be359163
------------------------------------------------------------------------

The panic occurs because __early_set_fixmap() called from
parse_setup_data() unconditionally sets the PTE with FIXMAP_PAGE_NORMAL,
which contains _PAGE_NX and causes an exception.

This patch modifies __early_set_fixmap() to set _PAGE_NX only when the
hardware supports it.  It also moves the call to x86_configure_nx()
earlier in setup_arch() before __early_set_fixmap() is first called.

The above problem occurs after __early_set_fixmap() is called from
parse_setup_data().  However, since setup_olpc_ofw_pgd() can also call
__early_set_fixmap(), the patch moves the call to x86_configure_nx()
before that.

Signed-off-by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
---
 arch/x86/kernel/setup.c |   18 +++++++++---------
 arch/x86/mm/ioremap.c   |    3 +++
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 29db25f..c8b2cdb 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -894,6 +894,15 @@ void __init setup_arch(char **cmdline_p)
 	early_cpu_init();
 	early_ioremap_init();
 
+	/*
+	 * x86_configure_nx() is called to detect whether hardware doesn't
+	 * support NX.  It has to be called before __early_set_fixmap() is
+	 * called from setup_olpc_ofw_pgd and parse_setup_data.  It may
+	 * then be called again from within noexec_setup() during parsing
+	 * early parameters to honor the respective command line option.
+	 */
+	x86_configure_nx();
+
 	setup_olpc_ofw_pgd();
 
 	ROOT_DEV = old_decode_dev(boot_params.hdr.root_dev);
@@ -971,15 +980,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();
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index b9c78f3..9036c8e 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -493,6 +493,9 @@ void __init __early_set_fixmap(enum fixed_addresses idx,
 	}
 	pte = early_ioremap_pte(addr);
 
+	if (!(__supported_pte_mask & _PAGE_NX))
+		pgprot_val(flags) &= ~_PAGE_NX;
+
 	if (pgprot_val(flags))
 		set_pte(pte, pfn_pte(phys >> PAGE_SHIFT, flags));
 	else

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Matt Fleming]

On Thu, 03 Dec, at 11:58:33PM, Kosuke Tatsukawa wrote:
> The kernel panics early in boot on a x86_64 server if the eXecute
> Disable (XD) bit is set to disabled in the uEFI firmware.  The message
> in the kernel log buffer looks like below.
> ------------------------------------------------------------------------
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.4.0-rc3 #1
> [    0.000000]  0000000000000000 261c6fa13723be1b ffffffff819b7e40 ffffffff8131f320
> [    0.000000]  ffffffffffffffff ffffffff819b7f30 ffffffff81b261b0 000000000000001c
> [    0.000000]  ffffffff81d77a1c 0000000000000010 00000000be35a000 ffffffffff200000
> [    0.000000] Call Trace:
> [    0.000000]  [<ffffffff8131f320>] dump_stack+0x44/0x64
> [    0.000000]  [<ffffffff81b261b0>] early_idt_handler_common+0x90/0xd0
> [    0.000000]  [<ffffffff81b2f1c5>] ? setup_arch+0x1f1/0xce0
> [    0.000000]  [<ffffffff81b2f1c5>] ? setup_arch+0x1f1/0xce0
> [    0.000000]  [<ffffffff81b26120>] ? early_idt_handler_array+0x120/0x120
> [    0.000000]  [<ffffffff81b26d81>] start_kernel+0xe6/0x4f0
> [    0.000000]  [<ffffffff81b26120>] ? early_idt_handler_array+0x120/0x120
> [    0.000000]  [<ffffffff81b26120>] ? early_idt_handler_array+0x120/0x120
> [    0.000000]  [<ffffffff81b265ee>] x86_64_start_reservations+0x2a/0x2c
> [    0.000000]  [<ffffffff81b2673c>] x86_64_start_kernel+0x14c/0x16f
> [    0.000000] RIP 0x80000000be359163
> ------------------------------------------------------------------------
> 
> The panic occurs because __early_set_fixmap() called from
> parse_setup_data() unconditionally sets the PTE with FIXMAP_PAGE_NORMAL,
> which contains _PAGE_NX and causes an exception.
> 
> This patch modifies __early_set_fixmap() to set _PAGE_NX only when the
> hardware supports it.  It also moves the call to x86_configure_nx()
> earlier in setup_arch() before __early_set_fixmap() is first called.
> 
> The above problem occurs after __early_set_fixmap() is called from
> parse_setup_data().  However, since setup_olpc_ofw_pgd() can also call
> __early_set_fixmap(), the patch moves the call to x86_configure_nx()
> before that.
> 
> Signed-off-by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
> ---
>  arch/x86/kernel/setup.c |   18 +++++++++---------
>  arch/x86/mm/ioremap.c   |    3 +++
>  2 files changed, 12 insertions(+), 9 deletions(-)

Could you try booting with the commit 04633df0c43d ("x86/cpu: Call
verify_cpu() after having entered long mode too") instead? It's part
of v4.4-rc1.

Allowing NX to be disabled should be avoided.

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Matt Fleming]

On Thu, 03 Dec, at 11:58:33PM, Kosuke Tatsukawa wrote:
> The kernel panics early in boot on a x86_64 server if the eXecute
> Disable (XD) bit is set to disabled in the uEFI firmware.  The message
> in the kernel log buffer looks like below.
> ------------------------------------------------------------------------
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.4.0-rc3 #1
> [    0.000000]  0000000000000000 261c6fa13723be1b ffffffff819b7e40 ffffffff8131f320
> [    0.000000]  ffffffffffffffff ffffffff819b7f30 ffffffff81b261b0 000000000000001c
> [    0.000000]  ffffffff81d77a1c 0000000000000010 00000000be35a000 ffffffffff200000
> [    0.000000] Call Trace:
> [    0.000000]  [<ffffffff8131f320>] dump_stack+0x44/0x64
> [    0.000000]  [<ffffffff81b261b0>] early_idt_handler_common+0x90/0xd0
> [    0.000000]  [<ffffffff81b2f1c5>] ? setup_arch+0x1f1/0xce0
> [    0.000000]  [<ffffffff81b2f1c5>] ? setup_arch+0x1f1/0xce0
> [    0.000000]  [<ffffffff81b26120>] ? early_idt_handler_array+0x120/0x120
> [    0.000000]  [<ffffffff81b26d81>] start_kernel+0xe6/0x4f0
> [    0.000000]  [<ffffffff81b26120>] ? early_idt_handler_array+0x120/0x120
> [    0.000000]  [<ffffffff81b26120>] ? early_idt_handler_array+0x120/0x120
> [    0.000000]  [<ffffffff81b265ee>] x86_64_start_reservations+0x2a/0x2c
> [    0.000000]  [<ffffffff81b2673c>] x86_64_start_kernel+0x14c/0x16f
> [    0.000000] RIP 0x80000000be359163
> ------------------------------------------------------------------------
> 
> The panic occurs because __early_set_fixmap() called from
> parse_setup_data() unconditionally sets the PTE with FIXMAP_PAGE_NORMAL,
> which contains _PAGE_NX and causes an exception.
> 
> This patch modifies __early_set_fixmap() to set _PAGE_NX only when the
> hardware supports it.  It also moves the call to x86_configure_nx()
> earlier in setup_arch() before __early_set_fixmap() is first called.
> 
> The above problem occurs after __early_set_fixmap() is called from
> parse_setup_data().  However, since setup_olpc_ofw_pgd() can also call
> __early_set_fixmap(), the patch moves the call to x86_configure_nx()
> before that.
> 
> Signed-off-by: Kosuke Tatsukawa <tatsu-zZGIbrA41Td8UrSeD/g0lQ@public.gmane.org>
> ---
>  arch/x86/kernel/setup.c |   18 +++++++++---------
>  arch/x86/mm/ioremap.c   |    3 +++
>  2 files changed, 12 insertions(+), 9 deletions(-)

Could you try booting with the commit 04633df0c43d ("x86/cpu: Call
verify_cpu() after having entered long mode too") instead? It's part
of v4.4-rc1.

Allowing NX to be disabled should be avoided.

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Kosuke Tatsukawa]

Matt Fleming wrote:
> On Thu, 03 Dec, at 11:58:33PM, Kosuke Tatsukawa wrote:
>> The kernel panics early in boot on a x86_64 server if the eXecute
>> Disable (XD) bit is set to disabled in the uEFI firmware.  The message
>> in the kernel log buffer looks like below.
>> ------------------------------------------------------------------------
>> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.4.0-rc3 #1
>> [    0.000000]  0000000000000000 261c6fa13723be1b ffffffff819b7e40 ffffffff8131f320
>> [    0.000000]  ffffffffffffffff ffffffff819b7f30 ffffffff81b261b0 000000000000001c
>> [    0.000000]  ffffffff81d77a1c 0000000000000010 00000000be35a000 ffffffffff200000
>> [    0.000000] Call Trace:
>> [    0.000000]  [<ffffffff8131f320>] dump_stack+0x44/0x64
>> [    0.000000]  [<ffffffff81b261b0>] early_idt_handler_common+0x90/0xd0
>> [    0.000000]  [<ffffffff81b2f1c5>] ? setup_arch+0x1f1/0xce0
>> [    0.000000]  [<ffffffff81b2f1c5>] ? setup_arch+0x1f1/0xce0
>> [    0.000000]  [<ffffffff81b26120>] ? early_idt_handler_array+0x120/0x120
>> [    0.000000]  [<ffffffff81b26d81>] start_kernel+0xe6/0x4f0
>> [    0.000000]  [<ffffffff81b26120>] ? early_idt_handler_array+0x120/0x120
>> [    0.000000]  [<ffffffff81b26120>] ? early_idt_handler_array+0x120/0x120
>> [    0.000000]  [<ffffffff81b265ee>] x86_64_start_reservations+0x2a/0x2c
>> [    0.000000]  [<ffffffff81b2673c>] x86_64_start_kernel+0x14c/0x16f
>> [    0.000000] RIP 0x80000000be359163
>> ------------------------------------------------------------------------
>> 
>> The panic occurs because __early_set_fixmap() called from
>> parse_setup_data() unconditionally sets the PTE with FIXMAP_PAGE_NORMAL,
>> which contains _PAGE_NX and causes an exception.
>> 
>> This patch modifies __early_set_fixmap() to set _PAGE_NX only when the
>> hardware supports it.  It also moves the call to x86_configure_nx()
>> earlier in setup_arch() before __early_set_fixmap() is first called.
>> 
>> The above problem occurs after __early_set_fixmap() is called from
>> parse_setup_data().  However, since setup_olpc_ofw_pgd() can also call
>> __early_set_fixmap(), the patch moves the call to x86_configure_nx()
>> before that.
>> 
>> Signed-off-by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
>> ---
>>  arch/x86/kernel/setup.c |   18 +++++++++---------
>>  arch/x86/mm/ioremap.c   |    3 +++
>>  2 files changed, 12 insertions(+), 9 deletions(-)
> 
> Could you try booting with the commit 04633df0c43d ("x86/cpu: Call
> verify_cpu() after having entered long mode too") instead? It's part
> of v4.4-rc1.
> 
> Allowing NX to be disabled should be avoided.

Thank you pointing that out.

linux-4.4-rc3 booted without a problem on a real server even with XD
turned off by the firmware.  I didn't notice this before because I was
using an older version of the kernel on the real server, and doing
investigation on a KVM guest.

The "noexec=off" kernel parameter still seems to come up with EFI
runtime service disabled though.  Do you think this should be left alone
as an disadvantage for using a bad option?
---
Kosuke TATSUKAWA  | 3rd IT Platform Department
                  | IT Platform Division, NEC Corporation
                  | tatsu@ab.jp.nec.com

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Kosuke Tatsukawa]

Matt Fleming wrote:
> On Thu, 03 Dec, at 11:58:33PM, Kosuke Tatsukawa wrote:
>> The kernel panics early in boot on a x86_64 server if the eXecute
>> Disable (XD) bit is set to disabled in the uEFI firmware.  The message
>> in the kernel log buffer looks like below.
>> ------------------------------------------------------------------------
>> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.4.0-rc3 #1
>> [    0.000000]  0000000000000000 261c6fa13723be1b ffffffff819b7e40 ffffffff8131f320
>> [    0.000000]  ffffffffffffffff ffffffff819b7f30 ffffffff81b261b0 000000000000001c
>> [    0.000000]  ffffffff81d77a1c 0000000000000010 00000000be35a000 ffffffffff200000
>> [    0.000000] Call Trace:
>> [    0.000000]  [<ffffffff8131f320>] dump_stack+0x44/0x64
>> [    0.000000]  [<ffffffff81b261b0>] early_idt_handler_common+0x90/0xd0
>> [    0.000000]  [<ffffffff81b2f1c5>] ? setup_arch+0x1f1/0xce0
>> [    0.000000]  [<ffffffff81b2f1c5>] ? setup_arch+0x1f1/0xce0
>> [    0.000000]  [<ffffffff81b26120>] ? early_idt_handler_array+0x120/0x120
>> [    0.000000]  [<ffffffff81b26d81>] start_kernel+0xe6/0x4f0
>> [    0.000000]  [<ffffffff81b26120>] ? early_idt_handler_array+0x120/0x120
>> [    0.000000]  [<ffffffff81b26120>] ? early_idt_handler_array+0x120/0x120
>> [    0.000000]  [<ffffffff81b265ee>] x86_64_start_reservations+0x2a/0x2c
>> [    0.000000]  [<ffffffff81b2673c>] x86_64_start_kernel+0x14c/0x16f
>> [    0.000000] RIP 0x80000000be359163
>> ------------------------------------------------------------------------
>> 
>> The panic occurs because __early_set_fixmap() called from
>> parse_setup_data() unconditionally sets the PTE with FIXMAP_PAGE_NORMAL,
>> which contains _PAGE_NX and causes an exception.
>> 
>> This patch modifies __early_set_fixmap() to set _PAGE_NX only when the
>> hardware supports it.  It also moves the call to x86_configure_nx()
>> earlier in setup_arch() before __early_set_fixmap() is first called.
>> 
>> The above problem occurs after __early_set_fixmap() is called from
>> parse_setup_data().  However, since setup_olpc_ofw_pgd() can also call
>> __early_set_fixmap(), the patch moves the call to x86_configure_nx()
>> before that.
>> 
>> Signed-off-by: Kosuke Tatsukawa <tatsu-zZGIbrA41Td8UrSeD/g0lQ@public.gmane.org>
>> ---
>>  arch/x86/kernel/setup.c |   18 +++++++++---------
>>  arch/x86/mm/ioremap.c   |    3 +++
>>  2 files changed, 12 insertions(+), 9 deletions(-)
> 
> Could you try booting with the commit 04633df0c43d ("x86/cpu: Call
> verify_cpu() after having entered long mode too") instead? It's part
> of v4.4-rc1.
> 
> Allowing NX to be disabled should be avoided.

Thank you pointing that out.

linux-4.4-rc3 booted without a problem on a real server even with XD
turned off by the firmware.  I didn't notice this before because I was
using an older version of the kernel on the real server, and doing
investigation on a KVM guest.

The "noexec=off" kernel parameter still seems to come up with EFI
runtime service disabled though.  Do you think this should be left alone
as an disadvantage for using a bad option?
---
Kosuke TATSUKAWA  | 3rd IT Platform Department
                  | IT Platform Division, NEC Corporation
                  | tatsu-zZGIbrA41Td8UrSeD/g0lQ@public.gmane.org

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Matt Fleming]

On Mon, 07 Dec, at 11:10:43PM, Kosuke Tatsukawa wrote:
> 
> Thank you pointing that out.
> 
> linux-4.4-rc3 booted without a problem on a real server even with XD
> turned off by the firmware.  I didn't notice this before because I was
> using an older version of the kernel on the real server, and doing
> investigation on a KVM guest.
> 
> The "noexec=off" kernel parameter still seems to come up with EFI
> runtime service disabled though.  Do you think this should be left alone
> as an disadvantage for using a bad option?

Good question.

I couldn't find any other examples of code that returns an error if
PAGE_NX isn't supported either by the hardware or via the noexec
command line option. Things like set_memory_x() and set_memory_nx()
return success without actually doing anything.

While I'm all for using hardware security features wherever possible,
such as force-enabling it in commit 04633df0c43d ("x86/cpu: Call
verify_cpu() after having entered long mode too"), if the user has
explicitly turned it off on the kernel command line, we should still
try and provide EFI services.

Borislav, what do you think about stripping PAGE_NX from 'page_flags'
in kernel_map_pages_in_pgd() if NX isn't supported, rather than
returning EINVAL? At least that way EFI runtime services would still
work.

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Matt Fleming]

On Mon, 07 Dec, at 11:10:43PM, Kosuke Tatsukawa wrote:
> 
> Thank you pointing that out.
> 
> linux-4.4-rc3 booted without a problem on a real server even with XD
> turned off by the firmware.  I didn't notice this before because I was
> using an older version of the kernel on the real server, and doing
> investigation on a KVM guest.
> 
> The "noexec=off" kernel parameter still seems to come up with EFI
> runtime service disabled though.  Do you think this should be left alone
> as an disadvantage for using a bad option?

Good question.

I couldn't find any other examples of code that returns an error if
PAGE_NX isn't supported either by the hardware or via the noexec
command line option. Things like set_memory_x() and set_memory_nx()
return success without actually doing anything.

While I'm all for using hardware security features wherever possible,
such as force-enabling it in commit 04633df0c43d ("x86/cpu: Call
verify_cpu() after having entered long mode too"), if the user has
explicitly turned it off on the kernel command line, we should still
try and provide EFI services.

Borislav, what do you think about stripping PAGE_NX from 'page_flags'
in kernel_map_pages_in_pgd() if NX isn't supported, rather than
returning EINVAL? At least that way EFI runtime services would still
work.

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Borislav Petkov]

On Tue, Dec 08, 2015 at 12:25:57PM +0000, Matt Fleming wrote:
> On Mon, 07 Dec, at 11:10:43PM, Kosuke Tatsukawa wrote:
> > 
> > Thank you pointing that out.
> > 
> > linux-4.4-rc3 booted without a problem on a real server even with XD
> > turned off by the firmware.  I didn't notice this before because I was

The aforementioned patch reenables NX.

> Borislav, what do you think about stripping PAGE_NX from 'page_flags'
> in kernel_map_pages_in_pgd() if NX isn't supported, rather than
> returning EINVAL? At least that way EFI runtime services would still
> work.

I guess we can - I mean, I don't see what can go wrong more when
allowing the kernel to execute even NX UEFI regions. Maybe easier
generation of "gadgets" in the ROP sense ...

On a related node, I'm very sceptical of the existence of this "noexec"
chicken bit, if you ask me. It is a really bad idea, security-wise, to
disable NX. Is there even a valid use case to disable NX?

Because if not, I'd vote for removing that chicken bit or at least
taining the kernel with

	add_taint(TAINT_USER_MORON, ... );

Kees, has this NX disabling practice come up in the past, per chance... ?

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Borislav Petkov]

On Tue, Dec 08, 2015 at 12:25:57PM +0000, Matt Fleming wrote:
> On Mon, 07 Dec, at 11:10:43PM, Kosuke Tatsukawa wrote:
> > 
> > Thank you pointing that out.
> > 
> > linux-4.4-rc3 booted without a problem on a real server even with XD
> > turned off by the firmware.  I didn't notice this before because I was

The aforementioned patch reenables NX.

> Borislav, what do you think about stripping PAGE_NX from 'page_flags'
> in kernel_map_pages_in_pgd() if NX isn't supported, rather than
> returning EINVAL? At least that way EFI runtime services would still
> work.

I guess we can - I mean, I don't see what can go wrong more when
allowing the kernel to execute even NX UEFI regions. Maybe easier
generation of "gadgets" in the ROP sense ...

On a related node, I'm very sceptical of the existence of this "noexec"
chicken bit, if you ask me. It is a really bad idea, security-wise, to
disable NX. Is there even a valid use case to disable NX?

Because if not, I'd vote for removing that chicken bit or at least
taining the kernel with

	add_taint(TAINT_USER_MORON, ... );

Kees, has this NX disabling practice come up in the past, per chance... ?

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Kees Cook]

On Tue, Dec 8, 2015 at 6:19 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Tue, Dec 08, 2015 at 12:25:57PM +0000, Matt Fleming wrote:
>> On Mon, 07 Dec, at 11:10:43PM, Kosuke Tatsukawa wrote:
>> >
>> > Thank you pointing that out.
>> >
>> > linux-4.4-rc3 booted without a problem on a real server even with XD
>> > turned off by the firmware.  I didn't notice this before because I was
>
> The aforementioned patch reenables NX.
>
>> Borislav, what do you think about stripping PAGE_NX from 'page_flags'
>> in kernel_map_pages_in_pgd() if NX isn't supported, rather than
>> returning EINVAL? At least that way EFI runtime services would still
>> work.
>
> I guess we can - I mean, I don't see what can go wrong more when
> allowing the kernel to execute even NX UEFI regions. Maybe easier
> generation of "gadgets" in the ROP sense ...
>
> On a related node, I'm very sceptical of the existence of this "noexec"
> chicken bit, if you ask me. It is a really bad idea, security-wise, to
> disable NX. Is there even a valid use case to disable NX?
>
> Because if not, I'd vote for removing that chicken bit or at least
> taining the kernel with
>
>         add_taint(TAINT_USER_MORON, ... );

If we add this for not-nx, I would like to add it for not-rodata too.

> Kees, has this NX disabling practice come up in the past, per chance... ?

I've never seen anyone actually use it. I was asked to include it out
of fear of some kind of rogue imagined CPU configuration that mixed NX
and non-NX capable CPUs in a single machine where the forced NX
re-enablement code would cause problems. As you might imagine, I'm not
aware of this case ever being an issue. ;)

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Kees Cook]

On Tue, Dec 8, 2015 at 6:19 AM, Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org> wrote:
> On Tue, Dec 08, 2015 at 12:25:57PM +0000, Matt Fleming wrote:
>> On Mon, 07 Dec, at 11:10:43PM, Kosuke Tatsukawa wrote:
>> >
>> > Thank you pointing that out.
>> >
>> > linux-4.4-rc3 booted without a problem on a real server even with XD
>> > turned off by the firmware.  I didn't notice this before because I was
>
> The aforementioned patch reenables NX.
>
>> Borislav, what do you think about stripping PAGE_NX from 'page_flags'
>> in kernel_map_pages_in_pgd() if NX isn't supported, rather than
>> returning EINVAL? At least that way EFI runtime services would still
>> work.
>
> I guess we can - I mean, I don't see what can go wrong more when
> allowing the kernel to execute even NX UEFI regions. Maybe easier
> generation of "gadgets" in the ROP sense ...
>
> On a related node, I'm very sceptical of the existence of this "noexec"
> chicken bit, if you ask me. It is a really bad idea, security-wise, to
> disable NX. Is there even a valid use case to disable NX?
>
> Because if not, I'd vote for removing that chicken bit or at least
> taining the kernel with
>
>         add_taint(TAINT_USER_MORON, ... );

If we add this for not-nx, I would like to add it for not-rodata too.

> Kees, has this NX disabling practice come up in the past, per chance... ?

I've never seen anyone actually use it. I was asked to include it out
of fear of some kind of rogue imagined CPU configuration that mixed NX
and non-NX capable CPUs in a single machine where the forced NX
re-enablement code would cause problems. As you might imagine, I'm not
aware of this case ever being an issue. ;)

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[H. Peter Anvin]

On December 8, 2015 12:30:06 PM PST, Kees Cook <keescook@chromium.org> wrote:
>On Tue, Dec 8, 2015 at 6:19 AM, Borislav Petkov <bp@alien8.de> wrote:
>> On Tue, Dec 08, 2015 at 12:25:57PM +0000, Matt Fleming wrote:
>>> On Mon, 07 Dec, at 11:10:43PM, Kosuke Tatsukawa wrote:
>>> >
>>> > Thank you pointing that out.
>>> >
>>> > linux-4.4-rc3 booted without a problem on a real server even with
>XD
>>> > turned off by the firmware.  I didn't notice this before because I
>was
>>
>> The aforementioned patch reenables NX.
>>
>>> Borislav, what do you think about stripping PAGE_NX from
>'page_flags'
>>> in kernel_map_pages_in_pgd() if NX isn't supported, rather than
>>> returning EINVAL? At least that way EFI runtime services would still
>>> work.
>>
>> I guess we can - I mean, I don't see what can go wrong more when
>> allowing the kernel to execute even NX UEFI regions. Maybe easier
>> generation of "gadgets" in the ROP sense ...
>>
>> On a related node, I'm very sceptical of the existence of this
>"noexec"
>> chicken bit, if you ask me. It is a really bad idea, security-wise,
>to
>> disable NX. Is there even a valid use case to disable NX?
>>
>> Because if not, I'd vote for removing that chicken bit or at least
>> taining the kernel with
>>
>>         add_taint(TAINT_USER_MORON, ... );
>
>If we add this for not-nx, I would like to add it for not-rodata too.
>
>> Kees, has this NX disabling practice come up in the past, per
>chance... ?
>
>I've never seen anyone actually use it. I was asked to include it out
>of fear of some kind of rogue imagined CPU configuration that mixed NX
>and non-NX capable CPUs in a single machine where the forced NX
>re-enablement code would cause problems. As you might imagine, I'm not
>aware of this case ever being an issue. ;)
>
>-Kees

Actually I think of it much more as a debug option - being able to mimic NX-unaware hardware and to track down problems in the field.
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[H. Peter Anvin]

On December 8, 2015 12:30:06 PM PST, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>On Tue, Dec 8, 2015 at 6:19 AM, Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org> wrote:
>> On Tue, Dec 08, 2015 at 12:25:57PM +0000, Matt Fleming wrote:
>>> On Mon, 07 Dec, at 11:10:43PM, Kosuke Tatsukawa wrote:
>>> >
>>> > Thank you pointing that out.
>>> >
>>> > linux-4.4-rc3 booted without a problem on a real server even with
>XD
>>> > turned off by the firmware.  I didn't notice this before because I
>was
>>
>> The aforementioned patch reenables NX.
>>
>>> Borislav, what do you think about stripping PAGE_NX from
>'page_flags'
>>> in kernel_map_pages_in_pgd() if NX isn't supported, rather than
>>> returning EINVAL? At least that way EFI runtime services would still
>>> work.
>>
>> I guess we can - I mean, I don't see what can go wrong more when
>> allowing the kernel to execute even NX UEFI regions. Maybe easier
>> generation of "gadgets" in the ROP sense ...
>>
>> On a related node, I'm very sceptical of the existence of this
>"noexec"
>> chicken bit, if you ask me. It is a really bad idea, security-wise,
>to
>> disable NX. Is there even a valid use case to disable NX?
>>
>> Because if not, I'd vote for removing that chicken bit or at least
>> taining the kernel with
>>
>>         add_taint(TAINT_USER_MORON, ... );
>
>If we add this for not-nx, I would like to add it for not-rodata too.
>
>> Kees, has this NX disabling practice come up in the past, per
>chance... ?
>
>I've never seen anyone actually use it. I was asked to include it out
>of fear of some kind of rogue imagined CPU configuration that mixed NX
>and non-NX capable CPUs in a single machine where the forced NX
>re-enablement code would cause problems. As you might imagine, I'm not
>aware of this case ever being an issue. ;)
>
>-Kees

Actually I think of it much more as a debug option - being able to mimic NX-unaware hardware and to track down problems in the field.
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Borislav Petkov]

On Tue, Dec 08, 2015 at 12:39:14PM -0800, H. Peter Anvin wrote:
> Actually I think of it much more as a debug option - being able to
> mimic NX-unaware hardware and to track down problems in the field.

Considering it can be dangerous when exposed to the user, should we hide
it behind a "Kernel Hacking" .config option which is default-off?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Borislav Petkov]

On Tue, Dec 08, 2015 at 12:30:06PM -0800, Kees Cook wrote:
> If we add this for not-nx, I would like to add it for not-rodata too.

The W+X thing?

I was under the impression we want to fix all those so that the ptdump
check doesn't fire anymore.

> I've never seen anyone actually use it. I was asked to include it out
> of fear of some kind of rogue imagined CPU configuration that mixed NX
> and non-NX capable CPUs in a single machine where the forced NX
> re-enablement code would cause problems. As you might imagine, I'm not
> aware of this case ever being an issue. ;)

Right. :)

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Borislav Petkov]

On Tue, Dec 08, 2015 at 12:30:06PM -0800, Kees Cook wrote:
> If we add this for not-nx, I would like to add it for not-rodata too.

The W+X thing?

I was under the impression we want to fix all those so that the ptdump
check doesn't fire anymore.

> I've never seen anyone actually use it. I was asked to include it out
> of fear of some kind of rogue imagined CPU configuration that mixed NX
> and non-NX capable CPUs in a single machine where the forced NX
> re-enablement code would cause problems. As you might imagine, I'm not
> aware of this case ever being an issue. ;)

Right. :)

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Andy Lutomirski]

On Tue, Dec 8, 2015 at 12:39 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On December 8, 2015 12:30:06 PM PST, Kees Cook <keescook@chromium.org> wrote:
>>On Tue, Dec 8, 2015 at 6:19 AM, Borislav Petkov <bp@alien8.de> wrote:
>>> On Tue, Dec 08, 2015 at 12:25:57PM +0000, Matt Fleming wrote:
>>>> On Mon, 07 Dec, at 11:10:43PM, Kosuke Tatsukawa wrote:
>>>> >
>>>> > Thank you pointing that out.
>>>> >
>>>> > linux-4.4-rc3 booted without a problem on a real server even with
>>XD
>>>> > turned off by the firmware.  I didn't notice this before because I
>>was
>>>
>>> The aforementioned patch reenables NX.
>>>
>>>> Borislav, what do you think about stripping PAGE_NX from
>>'page_flags'
>>>> in kernel_map_pages_in_pgd() if NX isn't supported, rather than
>>>> returning EINVAL? At least that way EFI runtime services would still
>>>> work.
>>>
>>> I guess we can - I mean, I don't see what can go wrong more when
>>> allowing the kernel to execute even NX UEFI regions. Maybe easier
>>> generation of "gadgets" in the ROP sense ...
>>>
>>> On a related node, I'm very sceptical of the existence of this
>>"noexec"
>>> chicken bit, if you ask me. It is a really bad idea, security-wise,
>>to
>>> disable NX. Is there even a valid use case to disable NX?
>>>
>>> Because if not, I'd vote for removing that chicken bit or at least
>>> taining the kernel with
>>>
>>>         add_taint(TAINT_USER_MORON, ... );
>>
>>If we add this for not-nx, I would like to add it for not-rodata too.
>>
>>> Kees, has this NX disabling practice come up in the past, per
>>chance... ?
>>
>>I've never seen anyone actually use it. I was asked to include it out
>>of fear of some kind of rogue imagined CPU configuration that mixed NX
>>and non-NX capable CPUs in a single machine where the forced NX
>>re-enablement code would cause problems. As you might imagine, I'm not
>>aware of this case ever being an issue. ;)
>>
>>-Kees
>
> Actually I think of it much more as a debug option - being able to mimic NX-unaware hardware and to track down problems in the field.

Does that really work?  We don't respect noexec when setting up EFER.

in any case, we should get plenty of coverage.  Non-PAE kernels are
effectively running on non NX-supporting hardware no matter what.

--Andy

Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware

[Andy Lutomirski]

On Tue, Dec 8, 2015 at 12:39 PM, H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org> wrote:
> On December 8, 2015 12:30:06 PM PST, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>>On Tue, Dec 8, 2015 at 6:19 AM, Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org> wrote:
>>> On Tue, Dec 08, 2015 at 12:25:57PM +0000, Matt Fleming wrote:
>>>> On Mon, 07 Dec, at 11:10:43PM, Kosuke Tatsukawa wrote:
>>>> >
>>>> > Thank you pointing that out.
>>>> >
>>>> > linux-4.4-rc3 booted without a problem on a real server even with
>>XD
>>>> > turned off by the firmware.  I didn't notice this before because I
>>was
>>>
>>> The aforementioned patch reenables NX.
>>>
>>>> Borislav, what do you think about stripping PAGE_NX from
>>'page_flags'
>>>> in kernel_map_pages_in_pgd() if NX isn't supported, rather than
>>>> returning EINVAL? At least that way EFI runtime services would still
>>>> work.
>>>
>>> I guess we can - I mean, I don't see what can go wrong more when
>>> allowing the kernel to execute even NX UEFI regions. Maybe easier
>>> generation of "gadgets" in the ROP sense ...
>>>
>>> On a related node, I'm very sceptical of the existence of this
>>"noexec"
>>> chicken bit, if you ask me. It is a really bad idea, security-wise,
>>to
>>> disable NX. Is there even a valid use case to disable NX?
>>>
>>> Because if not, I'd vote for removing that chicken bit or at least
>>> taining the kernel with
>>>
>>>         add_taint(TAINT_USER_MORON, ... );
>>
>>If we add this for not-nx, I would like to add it for not-rodata too.
>>
>>> Kees, has this NX disabling practice come up in the past, per
>>chance... ?
>>
>>I've never seen anyone actually use it. I was asked to include it out
>>of fear of some kind of rogue imagined CPU configuration that mixed NX
>>and non-NX capable CPUs in a single machine where the forced NX
>>re-enablement code would cause problems. As you might imagine, I'm not
>>aware of this case ever being an issue. ;)
>>
>>-Kees
>
> Actually I think of it much more as a debug option - being able to mimic NX-unaware hardware and to track down problems in the field.

Does that really work?  We don't respect noexec when setting up EFER.

in any case, we should get plenty of coverage.  Non-PAE kernels are
effectively running on non NX-supporting hardware no matter what.

--Andy