* [PATCH] [x86] detect and report lack of NX protections @ 2009-10-19 18:42 Kees Cook 2009-10-19 23:43 ` Arjan van de Ven 0 siblings, 1 reply; 23+ messages in thread From: Kees Cook @ 2009-10-19 18:42 UTC (permalink / raw) To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel It is possible for x86_64 systems to lack the NX bit (see check_efer()) either due to the hardware lacking support or the BIOS having turned off the CPU capability, so NX status should be reported. Additionally, anyone booting NX-capable CPUs in 32bit mode without PAE will lack NX functionality, so this change provides feedback for that case as well. Signed-off-by: Kees Cook <kees.cook@canonical.com> --- arch/x86/mm/init.c | 10 ++++++++++ arch/x86/mm/setup_nx.c | 2 ++ 2 files changed, 12 insertions(+), 0 deletions(-) diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c index 73ffd55..8472293 100644 --- a/arch/x86/mm/init.c +++ b/arch/x86/mm/init.c @@ -149,6 +149,16 @@ unsigned long __init_refok init_memory_mapping(unsigned long start, set_nx(); if (nx_enabled) printk(KERN_INFO "NX (Execute Disable) protection: active\n"); + else if (cpu_has_pae) +#if defined(CONFIG_X86_64) || defined(CONFIG_X86_PAE) + /* PAE kernel, PAE CPU, without NX */ + printk(KERN_WARNING "Warning: NX (Execute Disable) protection " + "missing in CPU or disabled in BIOS!\n"); +#else + /* 32bit non-PAE kernel, PAE CPU */ + printk(KERN_WARNING "Warning: NX (Execute Disable) protection " + "cannot be enabled: non-PAE kernel!\n"); +#endif /* Enable PSE if available */ if (cpu_has_pse) diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c index 513d8ed..b039a4c 100644 --- a/arch/x86/mm/setup_nx.c +++ b/arch/x86/mm/setup_nx.c @@ -53,6 +53,8 @@ void __init set_nx(void) #else void set_nx(void) { + /* notice if _PAGE_NX was removed during check_efer() */ + nx_enabled = ((__supported_pte_mask & _PAGE_NX) == _PAGE_NX); } #endif -- 1.6.3.3 -- Kees Cook Ubuntu Security Team ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH] [x86] detect and report lack of NX protections 2009-10-19 18:42 [PATCH] [x86] detect and report lack of NX protections Kees Cook @ 2009-10-19 23:43 ` Arjan van de Ven 2009-10-20 2:04 ` [PATCH v2] " Kees Cook 0 siblings, 1 reply; 23+ messages in thread From: Arjan van de Ven @ 2009-10-19 23:43 UTC (permalink / raw) To: Kees Cook Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel On Mon, 19 Oct 2009 11:42:34 -0700 Kees Cook <kees.cook@canonical.com> wrote: > It is possible for x86_64 systems to lack the NX bit (see > check_efer()) either due to the hardware lacking support or the BIOS > having turned off the CPU capability, so NX status should be > reported. Additionally, anyone booting NX-capable CPUs in 32bit mode > without PAE will lack NX functionality, so this change provides > feedback for that case as well. > > Signed-off-by: Kees Cook <kees.cook@canonical.com> > --- > arch/x86/mm/init.c | 10 ++++++++++ > arch/x86/mm/setup_nx.c | 2 ++ > 2 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c > index 73ffd55..8472293 100644 > --- a/arch/x86/mm/init.c > +++ b/arch/x86/mm/init.c > @@ -149,6 +149,16 @@ unsigned long __init_refok > init_memory_mapping(unsigned long start, set_nx(); > if (nx_enabled) > printk(KERN_INFO "NX (Execute Disable) protection: > active\n"); > + else if (cpu_has_pae) > +#if defined(CONFIG_X86_64) || defined(CONFIG_X86_PAE) > + /* PAE kernel, PAE CPU, without NX */ > + printk(KERN_WARNING "Warning: NX (Execute Disable) > protection " > + "missing in CPU or disabled in BIOS!\n"); > +#else > + /* 32bit non-PAE kernel, PAE CPU */ > + printk(KERN_WARNING "Warning: NX (Execute Disable) > protection " > + "cannot be enabled: non-PAE kernel!\n"); > +#endif can we please not use "Warning" for something like this in the message... lets keep "Warning" for real WARN_ON() kind of events. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] [x86] detect and report lack of NX protections 2009-10-19 23:43 ` Arjan van de Ven @ 2009-10-20 2:04 ` Kees Cook 2009-10-20 2:18 ` H. Peter Anvin 0 siblings, 1 reply; 23+ messages in thread From: Kees Cook @ 2009-10-20 2:04 UTC (permalink / raw) To: Arjan van de Ven Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel It is possible for x86_64 systems to lack the NX bit (see check_efer()) either due to the hardware lacking support or the BIOS having turned off the CPU capability, so NX status should be reported. Additionally, anyone booting NX-capable CPUs in 32bit mode without PAE will lack NX functionality, so this change provides feedback for that case as well. v2: use "Alert:" instead of "Warning:" to avoid confusiong with WARN_ON() Signed-off-by: Kees Cook <kees.cook@canonical.com> --- arch/x86/mm/init.c | 10 ++++++++++ arch/x86/mm/setup_nx.c | 2 ++ 2 files changed, 12 insertions(+), 0 deletions(-) diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c index 73ffd55..8472293 100644 --- a/arch/x86/mm/init.c +++ b/arch/x86/mm/init.c @@ -149,6 +149,16 @@ unsigned long __init_refok init_memory_mapping(unsigned long start, set_nx(); if (nx_enabled) printk(KERN_INFO "NX (Execute Disable) protection: active\n"); + else if (cpu_has_pae) +#if defined(CONFIG_X86_64) || defined(CONFIG_X86_PAE) + /* PAE kernel, PAE CPU, without NX */ + printk(KERN_WARNING "Alert: NX (Execute Disable) protection " + "missing in CPU or disabled in BIOS!\n"); +#else + /* 32bit non-PAE kernel, PAE CPU */ + printk(KERN_WARNING "Alert: NX (Execute Disable) protection " + "cannot be enabled: non-PAE kernel!\n"); +#endif /* Enable PSE if available */ if (cpu_has_pse) diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c index 513d8ed..b039a4c 100644 --- a/arch/x86/mm/setup_nx.c +++ b/arch/x86/mm/setup_nx.c @@ -53,6 +53,8 @@ void __init set_nx(void) #else void set_nx(void) { + /* notice if _PAGE_NX was removed during check_efer() */ + nx_enabled = ((__supported_pte_mask & _PAGE_NX) == _PAGE_NX); } #endif -- 1.6.3.3 -- Kees Cook Ubuntu Security Team ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2] [x86] detect and report lack of NX protections 2009-10-20 2:04 ` [PATCH v2] " Kees Cook @ 2009-10-20 2:18 ` H. Peter Anvin 2009-10-20 4:44 ` Kees Cook 2009-10-20 4:55 ` [PATCH v3] " Kees Cook 0 siblings, 2 replies; 23+ messages in thread From: H. Peter Anvin @ 2009-10-20 2:18 UTC (permalink / raw) To: Kees Cook Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel On 10/20/2009 11:04 AM, Kees Cook wrote: > It is possible for x86_64 systems to lack the NX bit (see check_efer()) > either due to the hardware lacking support or the BIOS having turned > off the CPU capability, so NX status should be reported. Additionally, > anyone booting NX-capable CPUs in 32bit mode without PAE will lack NX > functionality, so this change provides feedback for that case as well. > > v2: use "Alert:" instead of "Warning:" to avoid confusiong with WARN_ON() > They're both wrong. Both imply that the user needs to take an action, which is wrong because the kernel is working as intended. If you need to use any kind of alert word, it should be something like "Notice:", and it should be KERN_NOTICE or even KERN_INFO. -hpa ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] [x86] detect and report lack of NX protections 2009-10-20 2:18 ` H. Peter Anvin @ 2009-10-20 4:44 ` Kees Cook 2009-10-20 4:55 ` [PATCH v3] " Kees Cook 1 sibling, 0 replies; 23+ messages in thread From: Kees Cook @ 2009-10-20 4:44 UTC (permalink / raw) To: H. Peter Anvin Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel Hi, On Tue, Oct 20, 2009 at 11:18:43AM +0900, H. Peter Anvin wrote: > On 10/20/2009 11:04 AM, Kees Cook wrote: > >It is possible for x86_64 systems to lack the NX bit (see check_efer()) > >either due to the hardware lacking support or the BIOS having turned > >off the CPU capability, so NX status should be reported. Additionally, > >anyone booting NX-capable CPUs in 32bit mode without PAE will lack NX > >functionality, so this change provides feedback for that case as well. > > > >v2: use "Alert:" instead of "Warning:" to avoid confusiong with WARN_ON() > > > > They're both wrong. Both imply that the user needs to take an > action, which is wrong because the kernel is working as intended. > If you need to use any kind of alert word, it should be something > like "Notice:", and it should be KERN_NOTICE or even KERN_INFO. In the case of a system where the BIOS was shipped with XD not enabled, the user needs to take an action. I'm okay with switching to Notice:, but I don't think KERN_INFO is right. I would agree, "Alert:" would seem to be a KERN_ALERT, which is above KERN_CRIT, which this is clearly not. "Notice" it is. -Kees -- Kees Cook Ubuntu Security Team ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3] [x86] detect and report lack of NX protections 2009-10-20 2:18 ` H. Peter Anvin 2009-10-20 4:44 ` Kees Cook @ 2009-10-20 4:55 ` Kees Cook 2009-11-09 22:10 ` [PATCH v4] " Kees Cook 1 sibling, 1 reply; 23+ messages in thread From: Kees Cook @ 2009-10-20 4:55 UTC (permalink / raw) To: Arjan van de Ven Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel It is possible for x86_64 systems to lack the NX bit (see check_efer()) either due to the hardware lacking support or the BIOS having turned off the CPU capability, so NX status should be reported. Additionally, anyone booting NX-capable CPUs in 32bit mode without PAE will lack NX functionality, so this change provides feedback for that case as well. v2: use "Alert:" instead of "Warning:" to avoid confusion with WARN_ON() v3: use "Notice:" instead of "Alert:" to avoid confusion with KERN_ALERT, and switch to KERN_NOTICE, in keeping with its use for "normal but significant condition" messages. Signed-off-by: Kees Cook <kees.cook@canonical.com> --- arch/x86/mm/init.c | 10 ++++++++++ arch/x86/mm/setup_nx.c | 2 ++ 2 files changed, 12 insertions(+), 0 deletions(-) diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c index 73ffd55..8472293 100644 --- a/arch/x86/mm/init.c +++ b/arch/x86/mm/init.c @@ -149,6 +149,16 @@ unsigned long __init_refok init_memory_mapping(unsigned long start, set_nx(); if (nx_enabled) printk(KERN_INFO "NX (Execute Disable) protection: active\n"); + else if (cpu_has_pae) +#if defined(CONFIG_X86_64) || defined(CONFIG_X86_PAE) + /* PAE kernel, PAE CPU, without NX */ + printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " + "missing in CPU or disabled in BIOS!\n"); +#else + /* 32bit non-PAE kernel, PAE CPU */ + printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " + "cannot be enabled: non-PAE kernel!\n"); +#endif /* Enable PSE if available */ if (cpu_has_pse) diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c index 513d8ed..b039a4c 100644 --- a/arch/x86/mm/setup_nx.c +++ b/arch/x86/mm/setup_nx.c @@ -53,6 +53,8 @@ void __init set_nx(void) #else void set_nx(void) { + /* notice if _PAGE_NX was removed during check_efer() */ + nx_enabled = ((__supported_pte_mask & _PAGE_NX) == _PAGE_NX); } #endif -- 1.6.3.3 -- Kees Cook Ubuntu Security Team ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4] [x86] detect and report lack of NX protections 2009-10-20 4:55 ` [PATCH v3] " Kees Cook @ 2009-11-09 22:10 ` Kees Cook 2009-11-09 23:16 ` H. Peter Anvin 2009-11-10 16:55 ` [PATCH v5] " Kees Cook 0 siblings, 2 replies; 23+ messages in thread From: Kees Cook @ 2009-11-09 22:10 UTC (permalink / raw) To: Arjan van de Ven Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel It is possible for x86_64 systems to lack the NX bit (see check_efer()) either due to the hardware lacking support or the BIOS having turned off the CPU capability, so NX status should be reported. Additionally, anyone booting NX-capable CPUs in 32bit mode without PAE will lack NX functionality, so this change provides feedback for that case as well. v2: use "Alert:" instead of "Warning:" to avoid confusion with WARN_ON() v3: use "Notice:" instead of "Alert:" to avoid confusion with KERN_ALERT, and switch to KERN_NOTICE, in keeping with its use for "normal but significant condition" messages. v4: check that _NX_PAGE is non-zero to avoid setting nx_enabled accidentally. Signed-off-by: Kees Cook <kees.cook@canonical.com> --- arch/x86/mm/init.c | 10 ++++++++++ arch/x86/mm/setup_nx.c | 3 +++ 2 files changed, 13 insertions(+), 0 deletions(-) diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c index 73ffd55..d98b43a 100644 --- a/arch/x86/mm/init.c +++ b/arch/x86/mm/init.c @@ -149,6 +149,16 @@ unsigned long __init_refok init_memory_mapping(unsigned long start, set_nx(); if (nx_enabled) printk(KERN_INFO "NX (Execute Disable) protection: active\n"); + else if (cpu_has_pae) +#if defined(CONFIG_X86_64) || defined(CONFIG_X86_PAE) + /* PAE kernel, PAE CPU, without NX */ + printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " + "missing in CPU or disabled in BIOS!\n"); +#else + /* 32bit non-PAE kernel, PAE CPU */ + printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " + "cannot be enabled: non-PAE kernel!\n"); +#endif /* Enable PSE if available */ if (cpu_has_pse) diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c index 513d8ed..1b93231 100644 --- a/arch/x86/mm/setup_nx.c +++ b/arch/x86/mm/setup_nx.c @@ -53,6 +53,9 @@ void __init set_nx(void) #else void set_nx(void) { + /* notice if _PAGE_NX exists and was removed during check_efer() */ + if (_PAGE_NX && ((__supported_pte_mask & _PAGE_NX) == _PAGE_NX)) + nx_enabled = 1; } #endif -- 1.6.5 -- Kees Cook Ubuntu Security Team ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v4] [x86] detect and report lack of NX protections 2009-11-09 22:10 ` [PATCH v4] " Kees Cook @ 2009-11-09 23:16 ` H. Peter Anvin 2009-11-10 15:49 ` Kees Cook 2009-11-10 16:55 ` [PATCH v5] " Kees Cook 1 sibling, 1 reply; 23+ messages in thread From: H. Peter Anvin @ 2009-11-09 23:16 UTC (permalink / raw) To: Kees Cook Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel On 11/09/2009 02:10 PM, Kees Cook wrote: > It is possible for x86_64 systems to lack the NX bit (see check_efer()) > either due to the hardware lacking support or the BIOS having turned > off the CPU capability, so NX status should be reported. Additionally, > anyone booting NX-capable CPUs in 32bit mode without PAE will lack NX > functionality, so this change provides feedback for that case as well. > > v2: use "Alert:" instead of "Warning:" to avoid confusion with WARN_ON() > v3: use "Notice:" instead of "Alert:" to avoid confusion with KERN_ALERT, > and switch to KERN_NOTICE, in keeping with its use for "normal but > significant condition" messages. > v4: check that _NX_PAGE is non-zero to avoid setting nx_enabled accidentally. > > Signed-off-by: Kees Cook <kees.cook@canonical.com> > --- > arch/x86/mm/init.c | 10 ++++++++++ > arch/x86/mm/setup_nx.c | 3 +++ > 2 files changed, 13 insertions(+), 0 deletions(-) > > diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c > index 73ffd55..d98b43a 100644 > --- a/arch/x86/mm/init.c > +++ b/arch/x86/mm/init.c > @@ -149,6 +149,16 @@ unsigned long __init_refok init_memory_mapping(unsigned long start, > set_nx(); > if (nx_enabled) > printk(KERN_INFO "NX (Execute Disable) protection: active\n"); > + else if (cpu_has_pae) > +#if defined(CONFIG_X86_64) || defined(CONFIG_X86_PAE) > + /* PAE kernel, PAE CPU, without NX */ > + printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " > + "missing in CPU or disabled in BIOS!\n"); > +#else > + /* 32bit non-PAE kernel, PAE CPU */ > + printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " > + "cannot be enabled: non-PAE kernel!\n"); > +#endif > > /* Enable PSE if available */ > if (cpu_has_pse) > diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c > index 513d8ed..1b93231 100644 > --- a/arch/x86/mm/setup_nx.c > +++ b/arch/x86/mm/setup_nx.c > @@ -53,6 +53,9 @@ void __init set_nx(void) > #else > void set_nx(void) > { > + /* notice if _PAGE_NX exists and was removed during check_efer() */ > + if (_PAGE_NX && ((__supported_pte_mask & _PAGE_NX) == _PAGE_NX)) > + nx_enabled = 1; > } > #endif > The second clause can only get executed if CONFIG_X86_PAE is unset, which in turn means _PAGE_NX == 0... so that piece of code is meaningless. It also looks to me that there is no message distinguishing the case when nx_enabled == 1 but disable_nx == 1, and instead we say NX is "active" when in fact it is disabled in the kernel. -hpa ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] [x86] detect and report lack of NX protections 2009-11-09 23:16 ` H. Peter Anvin @ 2009-11-10 15:49 ` Kees Cook 2009-11-10 16:47 ` H. Peter Anvin 0 siblings, 1 reply; 23+ messages in thread From: Kees Cook @ 2009-11-10 15:49 UTC (permalink / raw) To: H. Peter Anvin Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel On Mon, Nov 09, 2009 at 03:16:16PM -0800, H. Peter Anvin wrote: > On 11/09/2009 02:10 PM, Kees Cook wrote: > > diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c > > index 513d8ed..1b93231 100644 > > --- a/arch/x86/mm/setup_nx.c > > +++ b/arch/x86/mm/setup_nx.c > > @@ -53,6 +53,9 @@ void __init set_nx(void) > > #else > > void set_nx(void) > > { > > + /* notice if _PAGE_NX exists and was removed during check_efer() */ > > + if (_PAGE_NX && ((__supported_pte_mask & _PAGE_NX) == _PAGE_NX)) > > + nx_enabled = 1; > > } > > #endif > > > > The second clause can only get executed if CONFIG_X86_PAE is unset, > which in turn means _PAGE_NX == 0... so that piece of code is meaningless. CONFIG_X86_PAE is unset for x86_64, where _PAGE_NX is valid. (This was the main situation I was trying to address.) So that chunk runs for non-pae 32bit, and all 64bit: config X86_PAE bool "PAE (Physical Address Extension) Support" depends on X86_32 && !HIGHMEM4G > It also looks to me that there is no message distinguishing the case > when nx_enabled == 1 but disable_nx == 1, and instead we say NX is > "active" when in fact it is disabled in the kernel. That's true -- I had overlooked that part. New patch on the way... -Kees -- Kees Cook Ubuntu Security Team ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] [x86] detect and report lack of NX protections 2009-11-10 15:49 ` Kees Cook @ 2009-11-10 16:47 ` H. Peter Anvin 2009-11-10 16:57 ` Kees Cook 0 siblings, 1 reply; 23+ messages in thread From: H. Peter Anvin @ 2009-11-10 16:47 UTC (permalink / raw) To: Kees Cook Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel On 11/10/2009 07:49 AM, Kees Cook wrote: >> >> The second clause can only get executed if CONFIG_X86_PAE is unset, >> which in turn means _PAGE_NX == 0... so that piece of code is meaningless. > > CONFIG_X86_PAE is unset for x86_64, where _PAGE_NX is valid. (This was > the main situation I was trying to address.) So that chunk runs for > non-pae 32bit, and all 64bit: > In reality, X86_PAE really should have been defined for 64 bits, since 64 bits really is PAE in most aspects. Anyway, for the 64-bit case it looks like the proper place for any of this is in check_efer() just below, not in this null routine. -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] [x86] detect and report lack of NX protections 2009-11-10 16:47 ` H. Peter Anvin @ 2009-11-10 16:57 ` Kees Cook 2009-11-10 17:12 ` H. Peter Anvin 0 siblings, 1 reply; 23+ messages in thread From: Kees Cook @ 2009-11-10 16:57 UTC (permalink / raw) To: H. Peter Anvin Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel On Tue, Nov 10, 2009 at 08:47:23AM -0800, H. Peter Anvin wrote: > On 11/10/2009 07:49 AM, Kees Cook wrote: > >> > >> The second clause can only get executed if CONFIG_X86_PAE is unset, > >> which in turn means _PAGE_NX == 0... so that piece of code is meaningless. > > > > CONFIG_X86_PAE is unset for x86_64, where _PAGE_NX is valid. (This was > > the main situation I was trying to address.) So that chunk runs for > > non-pae 32bit, and all 64bit: > > > > In reality, X86_PAE really should have been defined for 64 bits, since > 64 bits really is PAE in most aspects. > > Anyway, for the 64-bit case it looks like the proper place for any of > this is in check_efer() just below, not in this null routine. 64-bit does not set nx_enabled to "1" by default anywhere. And setting the default to 1 in check_efer() seemed out of place to me. -Kees -- Kees Cook Ubuntu Security Team ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] [x86] detect and report lack of NX protections 2009-11-10 16:57 ` Kees Cook @ 2009-11-10 17:12 ` H. Peter Anvin 2009-11-10 17:46 ` Kees Cook 0 siblings, 1 reply; 23+ messages in thread From: H. Peter Anvin @ 2009-11-10 17:12 UTC (permalink / raw) To: Kees Cook Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel On 11/10/2009 08:57 AM, Kees Cook wrote: > > 64-bit does not set nx_enabled to "1" by default anywhere. And setting > the default to 1 in check_efer() seemed out of place to me. > If it doesn't set nx_enabled anywhere, you'll end up with the message + printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " + "missing in CPU or disabled in BIOS!\n"); which seems obviously wrong... no? -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] [x86] detect and report lack of NX protections 2009-11-10 17:12 ` H. Peter Anvin @ 2009-11-10 17:46 ` Kees Cook 2009-11-10 18:53 ` H. Peter Anvin 0 siblings, 1 reply; 23+ messages in thread From: Kees Cook @ 2009-11-10 17:46 UTC (permalink / raw) To: H. Peter Anvin Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel On Tue, Nov 10, 2009 at 09:12:44AM -0800, H. Peter Anvin wrote: > On 11/10/2009 08:57 AM, Kees Cook wrote: > > > > 64-bit does not set nx_enabled to "1" by default anywhere. And setting > > the default to 1 in check_efer() seemed out of place to me. > > > > If it doesn't set nx_enabled anywhere, you'll end up with the message > > + printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " > + "missing in CPU or disabled in BIOS!\n"); > > which seems obviously wrong... no? The kernel as-is does not set nx_enabled for 64-bit, so this message is skipped completely: if (nx_enabled) printk(KERN_INFO "NX (Execute Disable) protection: active\n"); The only time this printk is shown is on 32-bit with PAE (with NX). There is no "else" currently. My patch seeks to do two things: - report the "active" message on 64-bit (if not disabled in check_efer()) - report "zomg NX missing" for all cases when PAE is available to the CPU. To implement the first, the set_nx() routine for non-PAE 32-bit and all 64-bit needs to have nx_enabled set correctly (which is done in the second set_nx() that was empty in arch/x86/mm/setup_nx.c). To implement the second, the following logic is then added to the "if (nx_enabled)" case in arch/x86/mm/init.c as an "else": else if (cpu_has_pae) #if defined(CONFIG_X86_64) || defined(CONFIG_X86_PAE) /* PAE kernel, PAE CPU, without NX */ printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " "missing in CPU or disabled in BIOS!\n"); #else /* 32bit non-PAE kernel, PAE CPU */ printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " "cannot be enabled: non-PAE kernel!\n"); #endif -- Kees Cook Ubuntu Security Team ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] [x86] detect and report lack of NX protections 2009-11-10 17:46 ` Kees Cook @ 2009-11-10 18:53 ` H. Peter Anvin 2009-11-10 19:43 ` Kees Cook 0 siblings, 1 reply; 23+ messages in thread From: H. Peter Anvin @ 2009-11-10 18:53 UTC (permalink / raw) To: Kees Cook Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel On 11/10/2009 09:46 AM, Kees Cook wrote: > > The kernel as-is does not set nx_enabled for 64-bit, so this message is > skipped completely: > > if (nx_enabled) > printk(KERN_INFO "NX (Execute Disable) protection: active\n"); > > The only time this printk is shown is on 32-bit with PAE (with NX). > There is no "else" currently. > The structure you have is: if (nx_enabled) else if (cpu_has_pae) The test for cpu_has_pae is unconditional (you only #ifdef the message) -- in fact, this should cause a compile-time error on 64 bits: #undef cpu_has_pae #define cpu_has_pae ___BUG___ -hpa ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] [x86] detect and report lack of NX protections 2009-11-10 18:53 ` H. Peter Anvin @ 2009-11-10 19:43 ` Kees Cook 2009-11-10 19:59 ` H. Peter Anvin 2009-11-10 20:25 ` H. Peter Anvin 0 siblings, 2 replies; 23+ messages in thread From: Kees Cook @ 2009-11-10 19:43 UTC (permalink / raw) To: H. Peter Anvin Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel On Tue, Nov 10, 2009 at 10:53:33AM -0800, H. Peter Anvin wrote: > On 11/10/2009 09:46 AM, Kees Cook wrote: > > > > The kernel as-is does not set nx_enabled for 64-bit, so this message is > > skipped completely: > > > > if (nx_enabled) > > printk(KERN_INFO "NX (Execute Disable) protection: active\n"); > > > > The only time this printk is shown is on 32-bit with PAE (with NX). > > There is no "else" currently. > > > > The structure you have is: > > if (nx_enabled) > else if (cpu_has_pae) > > The test for cpu_has_pae is unconditional (you only #ifdef the message) > -- in fact, this should cause a compile-time error on 64 bits: > > #undef cpu_has_pae > #define cpu_has_pae ___BUG___ This is fun. CONFIG_X86_PAE isn't defined for 64-bit, and using cpu_has_pae on 64-bit is considered a bug. :) Here is the matrix of what I want to see reported about NX at boot time. How do you recommend this be implemented? kernel cpu -> | CPU has PAE | CPU lacks PAE | | | CPU has NX | CPU lacks NX | | V +-------------------+-------------------+-----------------+ 32-bit non-PAE | missing in kernel | missing in kernel | no message | +-------------------+-------------------+-----------------+ 32-bit PAE | active * | missing in CPU | no message | +-------------------+-------------------+-----------------+ 64-bit | active | missing in CPU | impossible | +-------------------+-------------------+-----------------+ The box with the "*" is the only message currently reported by the kernel. this looks ugly, but does it: #if defined(CONFIG_X86_32) else if (cpu_has_pae) #else else #endif #if defined(CONFIG_X86_64) || defined(CONFIG_X86_PAE) /* PAE kernel, PAE CPU, without NX */ printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " "missing in CPU or disabled in BIOS!\n"); #else /* 32bit non-PAE kernel, PAE CPU */ printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " "cannot be enabled: non-PAE kernel!\n"); #endif -- Kees Cook Ubuntu Security Team ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] [x86] detect and report lack of NX protections 2009-11-10 19:43 ` Kees Cook @ 2009-11-10 19:59 ` H. Peter Anvin 2009-11-10 20:55 ` Kees Cook 2009-11-10 20:25 ` H. Peter Anvin 1 sibling, 1 reply; 23+ messages in thread From: H. Peter Anvin @ 2009-11-10 19:59 UTC (permalink / raw) To: Kees Cook Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel On 11/10/2009 11:43 AM, Kees Cook wrote: > > This is fun. CONFIG_X86_PAE isn't defined for 64-bit, and using > cpu_has_pae on 64-bit is considered a bug. :) > Yeah, it's somewhat obnoxious. This stuff is a result of the 32- and 64-bit code evolving separately for too long. All of this could and should be cleaned up, but it takes a long time. Either way, you can use the explicit form: boot_cpu_has(X86_FEATURE_PAE) just fine, on any platform. However, the only case for which this can be false is for the non-PAE kernel, since the PAE kernels (32 or 64 bits) cannot boot without it. I have personally never liked the cpu_has_* shorthand macros, but they're occasionally useful for things that have to be handled specially on 64 bits. Unfortunately they have spread and people seem to think they're the only way. > Here is the matrix of what I want to see reported about NX at boot time. > How do you recommend this be implemented? > > kernel cpu -> | CPU has PAE | CPU lacks PAE | > | | CPU has NX | CPU lacks NX | | > V +-------------------+-------------------+-----------------+ > 32-bit non-PAE | missing in kernel | missing in kernel | no message | > +-------------------+-------------------+-----------------+ > 32-bit PAE | active * | missing in CPU | no message | > +-------------------+-------------------+-----------------+ > 64-bit | active | missing in CPU | impossible | > +-------------------+-------------------+-----------------+ > The box with the "*" is the only message currently reported by the kernel. The last column should actually be "no message", "impossible", "impossible". I also think "missing in kernel" is misleading in the 32-bit non-PAE, no-NX case (as it would imply that another kernel could do something), and I *really* fail to see why it is in any way different from the "CPU lacks PAE" case -- which also means no NX. "Unavailable in CPU" seems to beat everything. So the logic that makes sense would be: if (!cpu_has_nx) { /* If the CPU can't do it... */ printk(KERN_INFO "cpu: NX protection unavailable in CPU\n"); } else { #if defined(CONFIG_X86_32) && !defined(CONFIG_X86_PAE) /* Non-PAE kernel: NX unavailable */ printk(KERN_NOTICE "cpu: NX protection missing in kernel\n"); #else printk(KERN_INFO "cpu: NX protection active\n"); #endif } ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] [x86] detect and report lack of NX protections 2009-11-10 19:59 ` H. Peter Anvin @ 2009-11-10 20:55 ` Kees Cook 2009-11-10 21:22 ` H. Peter Anvin 2009-11-12 18:01 ` Yuhong Bao 0 siblings, 2 replies; 23+ messages in thread From: Kees Cook @ 2009-11-10 20:55 UTC (permalink / raw) To: H. Peter Anvin Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel On Tue, Nov 10, 2009 at 11:59:35AM -0800, H. Peter Anvin wrote: > On 11/10/2009 11:43 AM, Kees Cook wrote: > > kernel cpu -> | CPU has PAE | CPU lacks PAE | > > | | CPU has NX | CPU lacks NX | | > > V +-------------------+-------------------+-----------------+ > > 32-bit non-PAE | missing in kernel | missing in kernel | no message | > > +-------------------+-------------------+-----------------+ > > 32-bit PAE | active * | missing in CPU | no message | > > +-------------------+-------------------+-----------------+ > > 64-bit | active | missing in CPU | impossible | > > +-------------------+-------------------+-----------------+ > > The box with the "*" is the only message currently reported by the kernel. > > The last column should actually be "no message", "impossible", "impossible". Ah, yes, very true. > I also think "missing in kernel" is misleading in the 32-bit non-PAE, > no-NX case (as it would imply that another kernel could do something), Well, I think thinking that even if they turned on the flag in the BIOS, the non-PAE kernel couldn't do anything about it anyway. But, from your example, I see you went with "missing in kernel" anyway. > and I *really* fail to see why it is in any way different from the "CPU > lacks PAE" case -- which also means no NX. "Unavailable in CPU" seems > to beat everything. I was trying to be conservative and not have older hardware without PAE yelling about missing features. It seemed only worth complaining about missing NX if PAE was possible (the class of machines without PAE being larger than those without NX and with PAE). > So the logic that makes sense would be: > > if (!cpu_has_nx) { cpu_has_nx is not the same as nx_enabled (due to disable_nx). Also, why doesn't set_nx() use cpu_has_nx? It seems like it does the check manually? Should that be cleaned up? > /* If the CPU can't do it... */ > printk(KERN_INFO "cpu: NX protection unavailable in CPU\n"); > } else { > #if defined(CONFIG_X86_32) && !defined(CONFIG_X86_PAE) > /* Non-PAE kernel: NX unavailable */ > printk(KERN_NOTICE "cpu: NX protection missing in kernel\n"); > #else > printk(KERN_INFO "cpu: NX protection active\n"); > #endif > } How about this? (Along with the nx_enabled setting in set_nx() for the 64-bit and 32-bit+PAE case.) set_nx(); if (nx_enabled) { printk(KERN_INFO "NX (Execute Disable) protection: active\n"); #if defined(CONFIG_X86_32) && !defined(CONFIG_X86_PAE) } else if (cpu_has_pae) { /* 32bit non-PAE kernel, PAE CPU */ printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " "cannot be enabled: non-PAE kernel!\n"); #else } else { /* PAE kernel, PAE CPU, without NX */ printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " "missing in CPU or disabled in BIOS!\n"); #endif } -- Kees Cook Ubuntu Security Team ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] [x86] detect and report lack of NX protections 2009-11-10 20:55 ` Kees Cook @ 2009-11-10 21:22 ` H. Peter Anvin 2009-11-10 22:15 ` Kees Cook 2009-11-12 18:01 ` Yuhong Bao 1 sibling, 1 reply; 23+ messages in thread From: H. Peter Anvin @ 2009-11-10 21:22 UTC (permalink / raw) To: Kees Cook Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel On 11/10/2009 12:55 PM, Kees Cook wrote: >> > I also think "missing in kernel" is misleading in the 32-bit non-PAE, >> > no-NX case (as it would imply that another kernel could do something), > Well, I think thinking that even if they turned on the flag in the BIOS, > the non-PAE kernel couldn't do anything about it anyway. But, from your > example, I see you went with "missing in kernel" anyway. No, I didn't: in my example, the CPU checks have higher priority than the kernel feature check. >> So the logic that makes sense would be: >> >> if (!cpu_has_nx) { > > cpu_has_nx is not the same as nx_enabled (due to disable_nx). Also, why > doesn't set_nx() use cpu_has_nx? It seems like it does the check > manually? Should that be cleaned up? Yes, it should be. set_nx() and check_efer() are doing the same thing, except in different ways, and they are - IMO - *both* doing something dumb -- although check_efer() is saner. Anyway, I forgot the last case, which is NX disabled manually (disable_nx). It probably makes sense to make it the lowest priority message. if (!cpu_has_nx) { /* If the CPU can't do it... */ printk(KERN_INFO "cpu: NX protection unavailable in CPU\n"); } else { #if defined(CONFIG_X86_32) && !defined(CONFIG_X86_PAE) /* Non-PAE kernel: NX unavailable */ printk(KERN_NOTICE "cpu: NX protection not supported by kernel\n"); #else if (disable_nx) printk(KERN_INFO "cpu: NX protection disabled by kernel command line option\n"); else printk(KERN_INFO "cpu: NX protection active\n"); #endif } > How about this? (Along with the nx_enabled setting in set_nx() for the > 64-bit and 32-bit+PAE case.) No, it gives the wrong message for the manually disabled case. -hpa ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] [x86] detect and report lack of NX protections 2009-11-10 21:22 ` H. Peter Anvin @ 2009-11-10 22:15 ` Kees Cook 2009-11-10 22:25 ` H. Peter Anvin 0 siblings, 1 reply; 23+ messages in thread From: Kees Cook @ 2009-11-10 22:15 UTC (permalink / raw) To: H. Peter Anvin Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel On Tue, Nov 10, 2009 at 01:22:18PM -0800, H. Peter Anvin wrote: > Yes, it should be. set_nx() and check_efer() are doing the same thing, > except in different ways, and they are - IMO - *both* doing something > dumb -- although check_efer() is saner. BTW, it seems like set_nx() should move from init_memory_mapping to setup_arch, since init_memory_mapping can be called twice, but I don't think set_nx needs to be. Also, disable_nx is only defined in setup_nx.c when "defined(CONFIG_X86_64) || defined(CONFIG_X86_PAE)", so I re-arranged the logic to match that. How about the following clean-up and merge... --- It is possible for x86_64 systems to lack the NX bit (see check_efer()) either due to the hardware lacking support or the BIOS having turned off the CPU capability, so NX status should be reported. Additionally, anyone booting NX-capable CPUs in 32bit mode without PAE will lack NX functionality, so this change provides feedback for that case as well. Signed-off-by: Kees Cook <kees.cook@canonical.com> --- arch/x86/kernel/setup.c | 2 ++ arch/x86/mm/init.c | 4 ---- arch/x86/mm/setup_nx.c | 31 ++++++++++++++++++++----------- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index e09f0e2..72181c1 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -895,6 +895,8 @@ void __init setup_arch(char **cmdline_p) init_gbpages(); + set_nx(); + /* max_pfn_mapped is updated here */ max_low_pfn_mapped = init_memory_mapping(0, max_low_pfn<<PAGE_SHIFT); max_pfn_mapped = max_low_pfn_mapped; diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c index 73ffd55..d406c52 100644 --- a/arch/x86/mm/init.c +++ b/arch/x86/mm/init.c @@ -146,10 +146,6 @@ unsigned long __init_refok init_memory_mapping(unsigned long start, use_gbpages = direct_gbpages; #endif - set_nx(); - if (nx_enabled) - printk(KERN_INFO "NX (Execute Disable) protection: active\n"); - /* Enable PSE if available */ if (cpu_has_pse) set_in_cr4(X86_CR4_PSE); diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c index 513d8ed..269b668 100644 --- a/arch/x86/mm/setup_nx.c +++ b/arch/x86/mm/setup_nx.c @@ -33,28 +33,37 @@ static int __init noexec_setup(char *str) early_param("noexec", noexec_setup); #endif -#ifdef CONFIG_X86_PAE void __init set_nx(void) { - unsigned int v[4], l, h; - - if (cpu_has_pae && (cpuid_eax(0x80000000) > 0x80000001)) { - cpuid(0x80000001, &v[0], &v[1], &v[2], &v[3]); + if (!cpu_has_nx) { + printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " + "missing in CPU or disabled in BIOS!\n"); + } else { +#if defined(CONFIG_X86_64) || defined(CONFIG_X86_PAE) + if (disable_nx) + printk(KERN_INFO "NX (Execute Disable) protection: " + "disabled by kernel command line option\n"); + else { +# ifdef CONFIG_X86_PAE + /* 32-bit PAE */ + unsigned int l, h; - if ((v[3] & (1 << 20)) && !disable_nx) { rdmsr(MSR_EFER, l, h); l |= EFER_NX; wrmsr(MSR_EFER, l, h); nx_enabled = 1; __supported_pte_mask |= _PAGE_NX; +# endif + printk(KERN_INFO "NX (Execute Disable) protection: " + "active\n"); } - } -} #else -void set_nx(void) -{ -} + /* 32bit non-PAE kernel, NX cannot be used */ + printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " + "cannot be enabled: non-PAE kernel!\n"); #endif + } +} #ifdef CONFIG_X86_64 void __cpuinit check_efer(void) -- 1.6.5 -- Kees Cook Ubuntu Security Team ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v4] [x86] detect and report lack of NX protections 2009-11-10 22:15 ` Kees Cook @ 2009-11-10 22:25 ` H. Peter Anvin 0 siblings, 0 replies; 23+ messages in thread From: H. Peter Anvin @ 2009-11-10 22:25 UTC (permalink / raw) To: Kees Cook Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel On 11/10/2009 02:15 PM, Kees Cook wrote: > On Tue, Nov 10, 2009 at 01:22:18PM -0800, H. Peter Anvin wrote: >> Yes, it should be. set_nx() and check_efer() are doing the same thing, >> except in different ways, and they are - IMO - *both* doing something >> dumb -- although check_efer() is saner. > > BTW, it seems like set_nx() should move from init_memory_mapping to > setup_arch, since init_memory_mapping can be called twice, but I don't > think set_nx needs to be. > We already have call points in setup_arch for check_efer(), which are where this code needs to be called. As I said, check_efer() and set_nx() are the 64- and 32-bit versions of *exactly the same thing*. Furthermore, it looks to me that set_nx() is simply broken; it forces the bit set in EFER even though it is already set in head_32.S. Thus, what we *should* do is: a) remove set_nx() completely; b) enable check_efer() on 32 bits; c) rename check_efer() to something sane, like x86_configure_nx(); Optionally we should just use cpu_has_nx instead of reading EFER. -hpa ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v4] [x86] detect and report lack of NX protections 2009-11-10 20:55 ` Kees Cook 2009-11-10 21:22 ` H. Peter Anvin @ 2009-11-12 18:01 ` Yuhong Bao 1 sibling, 0 replies; 23+ messages in thread From: Yuhong Bao @ 2009-11-12 18:01 UTC (permalink / raw) To: kees.cook Cc: arjan, tglx, mingo, x86, penberg, jbeulich, vegardno, linux-kernel, hpa >> The last column should actually be "no message", "impossible", "impossible". > > Ah, yes, very true. Nope, not only can it be disabled in the BIOS, but also don't forget Intel's first D0 stepping Nocona Xeon x86-64 CP\x7fUs, which do not have NX. > (the class of machines without PAE being > larger than those without NX and with PAE).Nope also. Clue: Most post-Pentium Pro Intel CPUs and all AMD CPUs since the K7 has PAE available as a CPU feature. Yuhong Bao _________________________________________________________________ Hotmail: Powerful Free email with security by Microsoft. http://clk.atdmt.com/GBL/go/171222986/direct/01/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] [x86] detect and report lack of NX protections 2009-11-10 19:43 ` Kees Cook 2009-11-10 19:59 ` H. Peter Anvin @ 2009-11-10 20:25 ` H. Peter Anvin 1 sibling, 0 replies; 23+ messages in thread From: H. Peter Anvin @ 2009-11-10 20:25 UTC (permalink / raw) To: Kees Cook Cc: Arjan van de Ven, Thomas Gleixner, Ingo Molnar, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel The more I stare at the underlying code, the more I'm convinced that the fundamental problem is that the underlying code is insane, with multiple levels of detection for what amounts to cpu_has_nx, each effectively checking what the previous code has done. check_efer(), for example, screws with EFER, but EFER is simply set in head_64.S from CPUID (unless Xen does something insane -- but if so, Xen should clear X86_FEATURE_NX instead.) The 32-bit startup code also sets NX, but yet on 32 bits we wiggle EFER as if it had never been. This code is screaming for cleanup and unification. -hpa ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v5] [x86] detect and report lack of NX protections 2009-11-09 22:10 ` [PATCH v4] " Kees Cook 2009-11-09 23:16 ` H. Peter Anvin @ 2009-11-10 16:55 ` Kees Cook 1 sibling, 0 replies; 23+ messages in thread From: Kees Cook @ 2009-11-10 16:55 UTC (permalink / raw) To: Arjan van de Ven Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Pekka Enberg, Jan Beulich, Vegard Nossum, Yinghai Lu, Jeremy Fitzhardinge, linux-kernel It is possible for x86_64 systems to lack the NX bit (see check_efer()) either due to the hardware lacking support or the BIOS having turned off the CPU capability, so NX status should be reported. Additionally, anyone booting NX-capable CPUs in 32bit mode without PAE will lack NX functionality, so this change provides feedback for that case as well. v2: use "Alert:" instead of "Warning:" to avoid confusion with WARN_ON() v3: use "Notice:" instead of "Alert:" to avoid confusion with KERN_ALERT, and switch to KERN_NOTICE, in keeping with its use for "normal but significant condition" messages. v4: check that _NX_PAGE is non-zero to avoid setting nx_enabled accidentally. v5: check for disable_nx. Signed-off-by: Kees Cook <kees.cook@canonical.com> --- arch/x86/mm/init.c | 10 ++++++++++ arch/x86/mm/setup_nx.c | 6 +++++- 2 files changed, 15 insertions(+), 1 deletions(-) diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c index 73ffd55..d98b43a 100644 --- a/arch/x86/mm/init.c +++ b/arch/x86/mm/init.c @@ -149,6 +149,16 @@ unsigned long __init_refok init_memory_mapping(unsigned long start, set_nx(); if (nx_enabled) printk(KERN_INFO "NX (Execute Disable) protection: active\n"); + else if (cpu_has_pae) +#if defined(CONFIG_X86_64) || defined(CONFIG_X86_PAE) + /* PAE kernel, PAE CPU, without NX */ + printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " + "missing in CPU or disabled in BIOS!\n"); +#else + /* 32bit non-PAE kernel, PAE CPU */ + printk(KERN_NOTICE "Notice: NX (Execute Disable) protection " + "cannot be enabled: non-PAE kernel!\n"); +#endif /* Enable PSE if available */ if (cpu_has_pse) diff --git a/arch/x86/mm/setup_nx.c b/arch/x86/mm/setup_nx.c index 513d8ed..93d7b43 100644 --- a/arch/x86/mm/setup_nx.c +++ b/arch/x86/mm/setup_nx.c @@ -51,8 +51,12 @@ void __init set_nx(void) } } #else -void set_nx(void) +void __init set_nx(void) { + /* notice if _PAGE_NX exists and was removed during check_efer() */ + if (!disable_nx && _PAGE_NX && + ((__supported_pte_mask & _PAGE_NX) == _PAGE_NX)) + nx_enabled = 1; } #endif -- 1.6.5 -- Kees Cook Ubuntu Security Team ^ permalink raw reply related [flat|nested] 23+ messages in thread
end of thread, other threads:[~2009-11-12 18:01 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-10-19 18:42 [PATCH] [x86] detect and report lack of NX protections Kees Cook 2009-10-19 23:43 ` Arjan van de Ven 2009-10-20 2:04 ` [PATCH v2] " Kees Cook 2009-10-20 2:18 ` H. Peter Anvin 2009-10-20 4:44 ` Kees Cook 2009-10-20 4:55 ` [PATCH v3] " Kees Cook 2009-11-09 22:10 ` [PATCH v4] " Kees Cook 2009-11-09 23:16 ` H. Peter Anvin 2009-11-10 15:49 ` Kees Cook 2009-11-10 16:47 ` H. Peter Anvin 2009-11-10 16:57 ` Kees Cook 2009-11-10 17:12 ` H. Peter Anvin 2009-11-10 17:46 ` Kees Cook 2009-11-10 18:53 ` H. Peter Anvin 2009-11-10 19:43 ` Kees Cook 2009-11-10 19:59 ` H. Peter Anvin 2009-11-10 20:55 ` Kees Cook 2009-11-10 21:22 ` H. Peter Anvin 2009-11-10 22:15 ` Kees Cook 2009-11-10 22:25 ` H. Peter Anvin 2009-11-12 18:01 ` Yuhong Bao 2009-11-10 20:25 ` H. Peter Anvin 2009-11-10 16:55 ` [PATCH v5] " Kees Cook
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.