Messages in this thread | | | Date | Tue, 10 Nov 2009 13:22:18 -0800 | From | "H. Peter Anvin" <> | Subject | Re: [PATCH v4] [x86] detect and report lack of NX protections |
| |
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
| |