Re: [PATCH v4] [x86] detect and report lack of NX protections

[Kees Cook <kees.cook@canonical.com>]

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