[PATCH 3/3] x86, cpu: Enable/disable SMEP

[Fenghua Yu]

From: Fenghua Yu <fengh...@intel.com>

Enable/disable newly documented SMEP (Supervisor Mode Execution Protection) CPU
feature in kernel. CR4.SMEP (bit 20) is 0 at power-on. If the feature is
supported by CPU (X86_FEATURE_SMEP), enable SMEP by setting CR4.SMEP. New kernel
option nosmep disables the feature even if the feature is supported by CPU.

Signed-off-by: Fenghua Yu <fengh...@intel.com>
---
Documentation/kernel-parameters.txt | 4 ++++
arch/x86/kernel/cpu/common.c | 21 +++++++++++++++++++++
2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index cc85a92..56fb8c1 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1664,6 +1664,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
noexec=on: enable non-executable mappings (default)
noexec=off: disable non-executable mappings

+ nosmep [X86]
+ Disable SMEP (Supervisor Mode Execution Protection)
+ even if it is supported by processor.
+
noexec32 [X86-64]
This affects only 32-bit executables.
noexec32=on: enable non-executable mappings (default)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index e2ced00..f06b2d5 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -254,6 +254,25 @@ static inline void squash_the_stupid_serial_number(struct cpuinfo_x86 *c)
}
#endif

+static int disable_smep;
+static __init int setup_disable_smep(char *arg)
+{
+ disable_smep = 1;
+ return 1;
+}
+__setup("nosmep", setup_disable_smep);
+
+static __init void setup_smep(struct cpuinfo_x86 *c)
+{
+ if (cpu_has(c, X86_FEATURE_SMEP)) {
+ if (unlikely(disable_smep)) {
+ setup_clear_cpu_cap(X86_FEATURE_SMEP);
+ clear_in_cr4(X86_CR4_SMEP);
+ } else
+ set_in_cr4(X86_CR4_SMEP);
+ }
+}
+
/*
* Some CPU features depend on higher CPUID levels, which may not always
* be available due to CPUID level capping or broken virtualization
@@ -867,6 +886,8 @@ static void __cpuinit identify_cpu(struct cpuinfo_x86 *c)
/* Init Machine Check Exception if available. */
mcheck_cpu_init(c);

+ setup_smep(c);
+
select_idle_routine(c);

#ifdef CONFIG_NUMA
--
1.7.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Fenghua Yu]

From: Fenghua Yu <fengh...@intel.com>

Add support for newly documented SMEP (Supervisor Mode Execution Protection) CPU
feature flags.

Signed-off-by: Fenghua Yu <fengh...@intel.com>
---

arch/x86/include/asm/cpufeature.h | 1 +
arch/x86/kernel/cpu/scattered.c | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 50c0d30..e773f13 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -174,6 +174,7 @@
#define X86_FEATURE_PLN (7*32+ 5) /* Intel Power Limit Notification */
#define X86_FEATURE_PTS (7*32+ 6) /* Intel Package Thermal Status */
#define X86_FEATURE_DTS (7*32+ 7) /* Digital Thermal Sensor */
+#define X86_FEATURE_SMEP (7*32+ 8) /* Supervisor Mode Execution Protection*/

/* Virtualization flags: Linux defined, word 8 */
#define X86_FEATURE_TPR_SHADOW (8*32+ 0) /* Intel TPR Shadow */
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index c7f64e6..2de3aea 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -38,6 +38,7 @@ void __cpuinit init_scattered_cpuid_features(struct cpuinfo_x86 *c)
{ X86_FEATURE_PTS, CR_EAX, 6, 0x00000006, 0 },
{ X86_FEATURE_APERFMPERF, CR_ECX, 0, 0x00000006, 0 },
{ X86_FEATURE_EPB, CR_ECX, 3, 0x00000006, 0 },
+ { X86_FEATURE_SMEP, CR_EBX, 7, 0x00000007, 0 },
{ X86_FEATURE_XSAVEOPT, CR_EAX, 0, 0x0000000d, 1 },
{ X86_FEATURE_CPB, CR_EDX, 9, 0x80000007, 0 },
{ X86_FEATURE_NPT, CR_EDX, 0, 0x8000000a, 0 },

[Fenghua Yu]

From: Fenghua Yu <fengh...@intel.com>

Intel new CPU supports SMEP (Supervisor Mode Execution Protection). SMEP
prevents kernel from executing code in application. Updated Intel SDM describes
this CPU feature. The document will be published soon.

Fenghua Yu (3):
x86, cpu: Add CPU flags for SMEP
x86, cpu: Add SMEP CPU feature in CR4
x86, cpu: Enable/disable SMEP

Documentation/kernel-parameters.txt | 4 ++++
arch/x86/include/asm/cpufeature.h | 1 +
arch/x86/include/asm/processor-flags.h | 1 +
arch/x86/kernel/cpu/common.c | 21 +++++++++++++++++++++
arch/x86/kernel/cpu/scattered.c | 1 +
5 files changed, 28 insertions(+), 0 deletions(-)

[Fenghua Yu]

From: Fenghua Yu <fengh...@intel.com>

Add support for newly documented SMEP (Supervisor Mode Execution Protection)

CPU feature in CR4.

Signed-off-by: Fenghua Yu <fengh...@intel.com>
---

arch/x86/include/asm/processor-flags.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/processor-flags.h b/arch/x86/include/asm/processor-flags.h
index a898a2b..59ab4df 100644
--- a/arch/x86/include/asm/processor-flags.h
+++ b/arch/x86/include/asm/processor-flags.h
@@ -60,6 +60,7 @@
#define X86_CR4_OSXMMEXCPT 0x00000400 /* enable unmasked SSE exceptions */
#define X86_CR4_VMXE 0x00002000 /* enable VMX virtualization */
#define X86_CR4_OSXSAVE 0x00040000 /* enable xsave and xrestore */
+#define X86_CR4_SMEP 0x00100000 /* enable SMEP support */

/*
* x86-64 Task Priority Register, CR8

[Andi Kleen]

"Fenghua Yu" <fengh...@intel.com> writes:

> +static int disable_smep;

If you add a __initdata here the whole thing will completely disappear
after boot.

> @@ -867,6 +886,8 @@ static void __cpuinit identify_cpu(struct cpuinfo_x86 *c)
> /* Init Machine Check Exception if available. */
> mcheck_cpu_init(c);
>
> + setup_smep(c);

My feeling is you're setting this too late. In theory there could be
early overflows. Better use a early_param and set it as early
as possible, directly after the cpuid flags are set up in early
initialization.

-Andi
--
a...@linux.intel.com -- Speaking for myself only

[Fenghua Yu]

From: Fenghua Yu <fengh...@intel.com>

Enable/disable newly documented SMEP (Supervisor Mode Execution Protection) CPU
feature in kernel. CR4.SMEP (bit 20) is 0 at power-on. If the feature is
supported by CPU (X86_FEATURE_SMEP), enable SMEP by setting CR4.SMEP. New kernel
option nosmep disables the feature even if the feature is supported by CPU.

Signed-off-by: Fenghua Yu <fengh...@intel.com>
---
Documentation/kernel-parameters.txt | 4 ++++
arch/x86/kernel/cpu/common.c | 23 +++++++++++++++++++++++
2 files changed, 27 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index cc85a92..56fb8c1 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1664,6 +1664,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
noexec=on: enable non-executable mappings (default)
noexec=off: disable non-executable mappings

+ nosmep [X86]
+ Disable SMEP (Supervisor Mode Execution Protection)
+ even if it is supported by processor.
+
noexec32 [X86-64]
This affects only 32-bit executables.
noexec32=on: enable non-executable mappings (default)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c

index e2ced00..5f3cdd4 100644

--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -254,6 +254,25 @@ static inline void squash_the_stupid_serial_number(struct cpuinfo_x86 *c)
}
#endif

+static int disable_smep __initdata;

+static __init int setup_disable_smep(char *arg)
+{
+ disable_smep = 1;
+ return 1;
+}
+__setup("nosmep", setup_disable_smep);
+
+static __init void setup_smep(struct cpuinfo_x86 *c)
+{
+ if (cpu_has(c, X86_FEATURE_SMEP)) {
+ if (unlikely(disable_smep)) {
+ setup_clear_cpu_cap(X86_FEATURE_SMEP);
+ clear_in_cr4(X86_CR4_SMEP);
+ } else
+ set_in_cr4(X86_CR4_SMEP);
+ }
+}
+
/*
* Some CPU features depend on higher CPUID levels, which may not always
* be available due to CPUID level capping or broken virtualization

@@ -661,6 +680,8 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)

get_cpu_cap(c);

+ setup_smep(c);
+
if (this_cpu->c_early_init)
this_cpu->c_early_init(c);

@@ -738,6 +759,8 @@ static void __cpuinit generic_identify(struct cpuinfo_x86 *c)

get_cpu_cap(c);

+ setup_smep(c);
+
if (c->cpuid_level >= 0x00000001) {
c->initial_apicid = (cpuid_ebx(1) >> 24) & 0xFF;
#ifdef CONFIG_X86_32
--
1.7.2

[H. Peter Anvin]

On 05/11/2011 03:36 PM, Andi Kleen wrote:
> "Fenghua Yu"<fengh...@intel.com> writes:
>
>> +static int disable_smep;
>
> If you add a __initdata here the whole thing will completely disappear
> after boot.
>
>> @@ -867,6 +886,8 @@ static void __cpuinit identify_cpu(struct cpuinfo_x86 *c)
>> /* Init Machine Check Exception if available. */
>> mcheck_cpu_init(c);
>>
>> + setup_smep(c);
>
> My feeling is you're setting this too late. In theory there could be
> early overflows. Better use a early_param and set it as early
> as possible, directly after the cpuid flags are set up in early
> initialization.
>
> -Andi

SMEP only matters if we can into userspace code, so I *think* the above
should be okay. However, the most logical place would be to enable it
in the same place(s) where we enable NX.

-hpa

[Ingo Molnar]

* Fenghua Yu <fengh...@intel.com> wrote:

> From: Fenghua Yu <fengh...@intel.com>
>
> Enable/disable newly documented SMEP (Supervisor Mode Execution Protection) CPU
> feature in kernel. CR4.SMEP (bit 20) is 0 at power-on. If the feature is
> supported by CPU (X86_FEATURE_SMEP), enable SMEP by setting CR4.SMEP. New kernel
> option nosmep disables the feature even if the feature is supported by CPU.

Please add a clearer explanation to the changelog and the code as well,
something like:

SMEP prevents the CPU in kernel-mode to jump to an executable page that does
not have the kernel/system flag set in the pte. This prevents the kernel
from executing user-space code accidentally or maliciously, so it for
example prevents kernel exploits from jumping to specially prepared
user-mode shell code.

> Signed-off-by: Fenghua Yu <fengh...@intel.com>
> ---
> Documentation/kernel-parameters.txt | 4 ++++
> arch/x86/kernel/cpu/common.c | 21 +++++++++++++++++++++
> 2 files changed, 25 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index cc85a92..56fb8c1 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1664,6 +1664,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> noexec=on: enable non-executable mappings (default)
> noexec=off: disable non-executable mappings
>
> + nosmep [X86]
> + Disable SMEP (Supervisor Mode Execution Protection)
> + even if it is supported by processor.

Typo: s/by processor/by the processor

> +
> noexec32 [X86-64]
> This affects only 32-bit executables.
> noexec32=on: enable non-executable mappings (default)
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index e2ced00..f06b2d5 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -254,6 +254,25 @@ static inline void squash_the_stupid_serial_number(struct cpuinfo_x86 *c)
> }
> #endif
>
> +static int disable_smep;
> +static __init int setup_disable_smep(char *arg)

Nit: please put a newline between variables and the next function.

Also, since setup_smep() is __init this could be __initdata.

> +{
> + disable_smep = 1;
> + return 1;
> +}
> +__setup("nosmep", setup_disable_smep);

Naming nit: s/setup_disable_smep/setup_nosmep

> +static __init void setup_smep(struct cpuinfo_x86 *c)
> +{
> + if (cpu_has(c, X86_FEATURE_SMEP)) {
> + if (unlikely(disable_smep)) {
> + setup_clear_cpu_cap(X86_FEATURE_SMEP);
> + clear_in_cr4(X86_CR4_SMEP);
> + } else
> + set_in_cr4(X86_CR4_SMEP);

Nit: Please use symmetric curly braces.

> + }
> +}
> +
> /*
> * Some CPU features depend on higher CPUID levels, which may not always
> * be available due to CPUID level capping or broken virtualization
> @@ -867,6 +886,8 @@ static void __cpuinit identify_cpu(struct cpuinfo_x86 *c)
> /* Init Machine Check Exception if available. */
> mcheck_cpu_init(c);
>
> + setup_smep(c);
> +
> select_idle_routine(c);

The other option would be to enable SMEP in arch/x86/kernel/head_32/64.S where
we twiddle the cr4 anyway and enable PAE and PGE, and where we do a cpuid to
check whether the CPU supports NX.

This means we enable SMEP unconditionally on all CPUs that support it, the
nosmep boot option would turn it off shortly afterwards.

Also, is there some KVM impact of this CPU feature? If the hypervisor can pass
this to the guest kernel as well then we want to add support there as well, and
probably want to turn it on by default. (unless it breaks something in a bad
way)

Thanks,

Ingo

[Fenghua Yu]

From: Fenghua Yu <fengh...@intel.com>

Intel new CPU supports SMEP (Supervisor Mode Execution Protection). SMEP
prevents kernel from executing code in application. Updated Intel SDM describes
this CPU feature. The document will be published soon.

Note: This patch set doesn't enable the SMEP feature in KVM. If it's needed,
another patch will be pushed for enabling the feature in KVM.

Fenghua Yu (4):

x86, cpu: Add CPU flags for SMEP
x86, cpu: Add SMEP CPU feature in CR4

x86, head_32/64.S: Enable SMEP
x86/kernel/common.c: Disable SMEP by kernel option nosmep

Documentation/kernel-parameters.txt | 4 ++++
arch/x86/include/asm/cpufeature.h | 1 +
arch/x86/include/asm/processor-flags.h | 1 +

arch/x86/kernel/cpu/common.c | 22 ++++++++++++++++++++++
arch/x86/kernel/cpu/scattered.c | 1 +
arch/x86/kernel/head_32.S | 17 +++++++++++++----
arch/x86/kernel/head_64.S | 13 +++++++++++--
7 files changed, 53 insertions(+), 6 deletions(-)

--
1.7.2

[Fenghua Yu]

From: Fenghua Yu <fengh...@intel.com>

Enable newly documented SMEP (Supervisor Mode Execution Protection) CPU
feature in kernel.

SMEP prevents the CPU in kernel-mode to jump to an executable page that does

not have the kernel/system flag set in the pte. This prevents the kernel
from executing user-space code accidentally or maliciously, so it for example
prevents kernel exploits from jumping to specially prepared user-mode shell

code. The violation will cause page fault #PF and will have error code
identical to XD violation.

CR4.SMEP (bit 20) is 0 at power-on. If the feature is supported by CPU
(X86_FEATURE_SMEP), enable SMEP by setting CR4.SMEP. New kernel
option nosmep disables the feature even if the feature is supported by CPU.

Signed-off-by: Fenghua Yu <fengh...@intel.com>
---

arch/x86/kernel/head_32.S | 17 +++++++++++++----
arch/x86/kernel/head_64.S | 13 +++++++++++--

2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index ce0be7c..5325c02 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -308,11 +308,20 @@ default_entry:
movl cr4_bits,%edx
andl %edx,%edx
jz 6f
- movl %cr4,%eax # Turn on paging options (PSE,PAE,..)
- orl %edx,%eax
- movl %eax,%cr4
+ movl %cr4,%edi # Turn on paging options (PSE,PAE,..)
+ orl %edx,%edi

- testb $X86_CR4_PAE, %al # check if PAE is enabled
+ /* Check if SMEP is supported by the processor */
+ movl $0x7, %eax
+ movl $0, %ecx
+ cpuid
+ btl $7, %ebx
+ jnc 1f
+ /* Enable SMEP */
+ orl $(X86_CR4_SMEP), %edi
+1: movl %edi, %cr4
+
+ test $X86_CR4_PAE, %di # check if PAE is enabled
jz 6f

/* Check if extended functions are implemented */
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index e11e394..220ec5f 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -161,8 +161,17 @@ ENTRY(secondary_startup_64)
*/

/* Enable PAE mode and PGE */
- movl $(X86_CR4_PAE | X86_CR4_PGE), %eax
- movq %rax, %cr4
+ movl $(X86_CR4_PAE | X86_CR4_PGE), %edi
+
+ /* Check if SMEP is supported by the processor */
+ movl $0x7, %eax
+ movl $0, %ecx
+ cpuid
+ btl $7, %ebx
+ jnc 1f
+ /* Enable PAE mode, PGE, and SMEP */
+ movl $(X86_CR4_PAE | X86_CR4_PGE | X86_CR4_SMEP), %edi
+1: movq %rdi, %cr4

/* Setup early boot stage 4 level pagetables. */
movq $(init_level4_pgt - __START_KERNEL_map), %rax

[Fenghua Yu]

From: Fenghua Yu <fengh...@intel.com>

Add support for newly documented SMEP (Supervisor Mode Execution Protection) CPU
feature flags.

Signed-off-by: Fenghua Yu <fengh...@intel.com>
---

[Fenghua Yu]

From: Fenghua Yu <fengh...@intel.com>

Add support for newly documented SMEP (Supervisor Mode Execution Protection)

CPU feature in CR4.

Signed-off-by: Fenghua Yu <fengh...@intel.com>
---

arch/x86/include/asm/processor-flags.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/processor-flags.h b/arch/x86/include/asm/processor-flags.h
index a898a2b..59ab4df 100644
--- a/arch/x86/include/asm/processor-flags.h
+++ b/arch/x86/include/asm/processor-flags.h
@@ -60,6 +60,7 @@
#define X86_CR4_OSXMMEXCPT 0x00000400 /* enable unmasked SSE exceptions */
#define X86_CR4_VMXE 0x00002000 /* enable VMX virtualization */
#define X86_CR4_OSXSAVE 0x00040000 /* enable xsave and xrestore */
+#define X86_CR4_SMEP 0x00100000 /* enable SMEP support */

/*
* x86-64 Task Priority Register, CR8

[H. Peter Anvin]

On 05/16/2011 02:34 PM, Fenghua Yu wrote:
>
> Note: This patch set doesn't enable the SMEP feature in KVM. If it's needed,
> another patch will be pushed for enabling the feature in KVM.
>

Hi Avi,

Could you comment on if this needs to be a gating factor?

-hpa

[H. Peter Anvin]

On 05/16/2011 02:34 PM, Fenghua Yu wrote:

The level 7 flags are not considered "scattered"; they have their own
word; please use it.

-hpa

--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.

[Matthew Garrett]

On Mon, May 16, 2011 at 02:34:44PM -0700, Fenghua Yu wrote:
> From: Fenghua Yu <fengh...@intel.com>
>
> Enable newly documented SMEP (Supervisor Mode Execution Protection) CPU
> feature in kernel.
>
> SMEP prevents the CPU in kernel-mode to jump to an executable page that does
> not have the kernel/system flag set in the pte. This prevents the kernel
> from executing user-space code accidentally or maliciously, so it for example
> prevents kernel exploits from jumping to specially prepared user-mode shell
> code. The violation will cause page fault #PF and will have error code
> identical to XD violation.

Are EFI runtime service pages currently set up appropriately?

--
Matthew Garrett | mj...@srcf.ucam.org

[Ingo Molnar]

* Fenghua Yu <fengh...@intel.com> wrote:

> From: Fenghua Yu <fengh...@intel.com>
>
> Intel new CPU supports SMEP (Supervisor Mode Execution Protection). SMEP
> prevents kernel from executing code in application. Updated Intel SDM describes
> this CPU feature. The document will be published soon.
>
> Note: This patch set doesn't enable the SMEP feature in KVM. If it's needed,
> another patch will be pushed for enabling the feature in KVM.

We can do it separately from native kernel support, but i'm sure Avi would
agree that SMEP support in KVM would be nice! (as long as it's configurable as
well, there might be guest OSs that break if SMEP is enabled, right?)

Thanks,

Ingo

[Ingo Molnar]

* H. Peter Anvin <h...@zytor.com> wrote:

> On 05/16/2011 02:34 PM, Fenghua Yu wrote:
> >
> > Note: This patch set doesn't enable the SMEP feature in KVM. If it's needed,
> > another patch will be pushed for enabling the feature in KVM.
> >
>
> Hi Avi,
>
> Could you comment on if this needs to be a gating factor?

I think KVM would benefit from the native kernel playing guinea pig whether
SMEP is really, truly 100% trouble-free to enable by default (for Linux) ;-)

Some programmable configurability seems necessary on the KVM side, as KVM has
no control over how sane the guest kernel is.

Thanks,

Ingo

[Avi Kivity]

On 05/17/2011 10:05 AM, Ingo Molnar wrote:
> * H. Peter Anvin<h...@zytor.com> wrote:
>
> > On 05/16/2011 02:34 PM, Fenghua Yu wrote:
> > >
> > > Note: This patch set doesn't enable the SMEP feature in KVM. If it's needed,
> > > another patch will be pushed for enabling the feature in KVM.
> > >
> >
> > Hi Avi,
> >
> > Could you comment on if this needs to be a gating factor?

It should certainly not be a gating factor. Note that smep will be
disabled when switching to the guest, so there are no compatibility issues.

> I think KVM would benefit from the native kernel playing guinea pig whether
> SMEP is really, truly 100% trouble-free to enable by default (for Linux) ;-)
>
> Some programmable configurability seems necessary on the KVM side, as KVM has
> no control over how sane the guest kernel is.

We should simply expose the cpuid bit and cr4.smep. If the guest kernel
feels it is up to it, it can enable smep itself.

--
error compiling committee.c: too many arguments to function

[Avi Kivity]

On 05/17/2011 10:03 AM, Ingo Molnar wrote:
> * Fenghua Yu<fengh...@intel.com> wrote:
>
> > From: Fenghua Yu<fengh...@intel.com>
> >
> > Intel new CPU supports SMEP (Supervisor Mode Execution Protection). SMEP
> > prevents kernel from executing code in application. Updated Intel SDM describes
> > this CPU feature. The document will be published soon.
> >
> > Note: This patch set doesn't enable the SMEP feature in KVM. If it's needed,
> > another patch will be pushed for enabling the feature in KVM.
>
> We can do it separately from native kernel support, but i'm sure Avi would
> agree that SMEP support in KVM would be nice!

Definitely.

> (as long as it's configurable as
> well, there might be guest OSs that break if SMEP is enabled, right?)

As mentioned earlier, the simple thing is to expose smep and let the
guest enable it itself.

--
error compiling committee.c: too many arguments to function

--

[Ingo Molnar]

* Avi Kivity <a...@redhat.com> wrote:

> > Some programmable configurability seems necessary on the KVM side, as KVM
> > has no control over how sane the guest kernel is.
>
> We should simply expose the cpuid bit and cr4.smep. If the guest kernel
> feels it is up to it, it can enable smep itself.

Well, given that there's lots of legacy installations around it would be a neat
KVM feature if it was possible to enable SMEP even if the guest kernel does not
enable it. As an additional (optional) layer of security.

For example legacy Linux guests will work just fine, even if they do not enable
SMEP themselves.

Thanks,

Ingo

[Avi Kivity]

On 05/17/2011 12:29 PM, Ingo Molnar wrote:
> * Avi Kivity<a...@redhat.com> wrote:
>
> > > Some programmable configurability seems necessary on the KVM side, as KVM
> > > has no control over how sane the guest kernel is.
> >
> > We should simply expose the cpuid bit and cr4.smep. If the guest kernel
> > feels it is up to it, it can enable smep itself.
>
> Well, given that there's lots of legacy installations around it would be a neat
> KVM feature if it was possible to enable SMEP even if the guest kernel does not
> enable it. As an additional (optional) layer of security.
>
> For example legacy Linux guests will work just fine, even if they do not enable
> SMEP themselves.

It's certainly possible (set CR4.SMEP transparently and hide it from the
guest). But there's no way to tell if it doesn't break something
wierd. The host might not even know if the guest is Linux or something
else.

We could support it as a non-default feature, but that reduces its utility.

--
error compiling committee.c: too many arguments to function

--

[Ingo Molnar]

* Avi Kivity <a...@redhat.com> wrote:

> On 05/17/2011 12:29 PM, Ingo Molnar wrote:
> >* Avi Kivity<a...@redhat.com> wrote:
> >
> >> > Some programmable configurability seems necessary on the KVM side, as KVM
> >> > has no control over how sane the guest kernel is.
> >>
> >> We should simply expose the cpuid bit and cr4.smep. If the guest kernel
> >> feels it is up to it, it can enable smep itself.
> >
> > Well, given that there's lots of legacy installations around it would be a
> > neat KVM feature if it was possible to enable SMEP even if the guest kernel
> > does not enable it. As an additional (optional) layer of security.
> >
> > For example legacy Linux guests will work just fine, even if they do not
> > enable SMEP themselves.
>
> It's certainly possible (set CR4.SMEP transparently and hide it from the
> guest). But there's no way to tell if it doesn't break something wierd. The
> host might not even know if the guest is Linux or something else.
>
> We could support it as a non-default feature, but that reduces its utility.

It would be a nice touch for tools/kvm/: we would use KVM_GET_SREGS and
KVM_GET_SREGS to twiddle CR4.SMEP, even without the guest explicitly doing it.

A quick glance suggests that it could be done straight away in
tools/kvm/kvm-cpu.c::kvm_cpu__setup_sregs() during vcpu setup, and hopefully
that cr4 value survives boot and ends up in the guest kernel's mmu_cr4_features
mask shadow register.

Thanks,

Ingo

[Ingo Molnar]

* Avi Kivity <a...@redhat.com> wrote:

> On 05/17/2011 01:46 PM, Ingo Molnar wrote:
> >>
> >> We could support it as a non-default feature, but that reduces its utility.
> >
> >It would be a nice touch for tools/kvm/: we would use KVM_GET_SREGS and
> >KVM_GET_SREGS to twiddle CR4.SMEP, even without the guest explicitly doing it.
> >
> >A quick glance suggests that it could be done straight away in
> >tools/kvm/kvm-cpu.c::kvm_cpu__setup_sregs() during vcpu setup, and hopefully
> >that cr4 value survives boot and ends up in the guest kernel's mmu_cr4_features
> >mask shadow register.
>

> Depends if the guest uses a read-modify-write pattern or not. We could do it
> transparently in kvm.ko, since the real cr4 need not corresponds to the guest
> notion (for example, we often set cr0.wp or cr0.ts even though the guest
> wants them clear).

Oh, being transparent is a nice touch when it comes to security measures
(catching attackers who think there's no SMEP and such) - but that would need
KVM support and a new ioctl to configure it, right?

[Avi Kivity]

On 05/17/2011 01:46 PM, Ingo Molnar wrote:
> >

> > We could support it as a non-default feature, but that reduces its utility.
>
> It would be a nice touch for tools/kvm/: we would use KVM_GET_SREGS and
> KVM_GET_SREGS to twiddle CR4.SMEP, even without the guest explicitly doing it.
>
> A quick glance suggests that it could be done straight away in
> tools/kvm/kvm-cpu.c::kvm_cpu__setup_sregs() during vcpu setup, and hopefully
> that cr4 value survives boot and ends up in the guest kernel's mmu_cr4_features
> mask shadow register.

Depends if the guest uses a read-modify-write pattern or not. We could

do it transparently in kvm.ko, since the real cr4 need not corresponds
to the guest notion (for example, we often set cr0.wp or cr0.ts even
though the guest wants them clear).

--

error compiling committee.c: too many arguments to function

--

[Avi Kivity]

On 05/17/2011 02:38 PM, Ingo Molnar wrote:
> >
> > Depends if the guest uses a read-modify-write pattern or not. We could do it
> > transparently in kvm.ko, since the real cr4 need not corresponds to the guest
> > notion (for example, we often set cr0.wp or cr0.ts even though the guest
> > wants them clear).
>
> Oh, being transparent is a nice touch when it comes to security measures
> (catching attackers who think there's no SMEP and such) - but that would need
> KVM support and a new ioctl to configure it, right?

Yes.

--
error compiling committee.c: too many arguments to function

--

[Avi Kivity]

On 05/17/2011 02:44 PM, Avi Kivity wrote:
> On 05/17/2011 02:38 PM, Ingo Molnar wrote:
>> >
>> > Depends if the guest uses a read-modify-write pattern or not. We
>> could do it
>> > transparently in kvm.ko, since the real cr4 need not corresponds
>> to the guest
>> > notion (for example, we often set cr0.wp or cr0.ts even though the
>> guest
>> > wants them clear).
>>
>> Oh, being transparent is a nice touch when it comes to security measures
>> (catching attackers who think there's no SMEP and such) - but that
>> would need
>> KVM support and a new ioctl to configure it, right?
>
> Yes.
>

btw, KVM support is required anyway, you can't set random bits in cr4
(from either the guest or host userspace) - kvm needs to understand them.

[Pekka Enberg]

On Tue, May 17, 2011 at 2:47 PM, Avi Kivity <a...@redhat.com> wrote:
> On 05/17/2011 02:44 PM, Avi Kivity wrote:
>>
>> On 05/17/2011 02:38 PM, Ingo Molnar wrote:
>>>
>>> >
>>> >  Depends if the guest uses a read-modify-write pattern or not.  We
>>> > could do it
>>> >  transparently in kvm.ko, since the real cr4 need not corresponds to
>>> > the guest
>>> >  notion (for example, we often set cr0.wp or cr0.ts even though the
>>> > guest
>>> >  wants them clear).
>>>
>>> Oh, being transparent is a nice touch when it comes to security measures
>>> (catching attackers who think there's no SMEP and such) - but that would
>>> need
>>> KVM support and a new ioctl to configure it, right?
>>
>> Yes.
>>
>
> btw, KVM support is required anyway, you can't set random bits in cr4 (from
> either the guest or host userspace) - kvm needs to understand them.

Yeah, I was wondering how the CR4 hack would actually enable
something. :-) Please CC me if you do add such an ioctl() and we'll
make the native KVM tool use it by default.

Pekka

[Ingo Molnar]

* Avi Kivity <a...@redhat.com> wrote:

> On 05/17/2011 02:44 PM, Avi Kivity wrote:
> >On 05/17/2011 02:38 PM, Ingo Molnar wrote:
> >>>
> >>> Depends if the guest uses a read-modify-write pattern or not.
> >>We could do it
> >>> transparently in kvm.ko, since the real cr4 need not
> >>corresponds to the guest
> >>> notion (for example, we often set cr0.wp or cr0.ts even
> >>though the guest
> >>> wants them clear).
> >>
> >>Oh, being transparent is a nice touch when it comes to security measures
> >>(catching attackers who think there's no SMEP and such) - but
> >>that would need
> >>KVM support and a new ioctl to configure it, right?
> >
> >Yes.
> >
>
> btw, KVM support is required anyway, you can't set random bits in
> cr4 (from either the guest or host userspace) - kvm needs to
> understand them.

Sure, that is the whole premise of this discussion.

I meant to say:

"but that would need KVM ABI support via a new ioctl to configure it, right?"

Thanks,

Ingo

[Yu, Fenghua]

They are not set up yet. efi init is called after this.

But at this time there is no user space code yet. So there is no SMEP violation chance until later when any user space page table is setup.

Thanks.

-Fenghua

[H. Peter Anvin]

On 05/17/2011 04:08 PM, Yu, Fenghua wrote:
>>
>> Are EFI runtime service pages currently set up appropriately?
>
> They are not set up yet. efi init is called after this.
>
> But at this time there is no user space code yet. So there is no SMEP violation chance until later when any user space page table is setup.
>

Are EFI runtime pages set up with U=0? I would argue it is a bug if
they aren't, but we want to make sure that there isn't such a bug.

-hpa

[Yu, Fenghua]

> -----Original Message-----
> From: H. Peter Anvin [mailto:h...@zytor.com]
> Sent: Tuesday, May 17, 2011 4:13 PM
> To: Yu, Fenghua
> Cc: Matthew Garrett; Ingo Molnar; Thomas Gleixner; Mallick, Asit K;
> Linus Torvalds; Avi Kivity; Arjan van de Ven; Andrew Morton; Andi
> Kleen; linux-kernel
> Subject: Re: [PATCH v2 3/4] x86, head_32/64.S: Enable SMEP
>

> On 05/17/2011 04:08 PM, Yu, Fenghua wrote:
> >>
> >> Are EFI runtime service pages currently set up appropriately?
> >
> > They are not set up yet. efi init is called after this.
> >
> > But at this time there is no user space code yet. So there is no SMEP
> violation chance until later when any user space page table is setup.
> >
>
> Are EFI runtime pages set up with U=0? I would argue it is a bug if
> they aren't, but we want to make sure that there isn't such a bug.
>
> -hpa

The runtime services are ioremapped. So they should be U=0.

Thanks.

-Fenghua

[tip-bot for Fenghua Yu]

Commit-ID: de5397ad5b9ad22e2401c4dacdf1bb3b19c05679
Gitweb: http://git.kernel.org/tip/de5397ad5b9ad22e2401c4dacdf1bb3b19c05679
Author: Fenghua Yu <fengh...@intel.com>
AuthorDate: Wed, 11 May 2011 16:51:05 -0700
Committer: H. Peter Anvin <h...@linux.intel.com>
CommitDate: Tue, 17 May 2011 21:22:00 -0700

x86, cpu: Enable/disable Supervisor Mode Execution Protection

Enable/disable newly documented SMEP (Supervisor Mode Execution Protection) CPU
feature in kernel. CR4.SMEP (bit 20) is 0 at power-on. If the feature is

supported by CPU (X86_FEATURE_SMEP), enable SMEP by setting CR4.SMEP. New kernel
option nosmep disables the feature even if the feature is supported by CPU.

[ hpa: moved the call to setup_smep() until after the vendor-specific
initialization; that ensures that CPUID features are unmasked. We
will still run it before we have userspace (never mind uncontrolled
userspace). ]

Signed-off-by: Fenghua Yu <fengh...@intel.com>
LKML-Reference: <1305157865-31727-1-git...@intel.com>
Signed-off-by: H. Peter Anvin <h...@linux.intel.com>
---
Documentation/kernel-parameters.txt | 4 ++++
arch/x86/kernel/cpu/common.c | 23 +++++++++++++++++++++++
2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index cc85a92..56fb8c1 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1664,6 +1664,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
noexec=on: enable non-executable mappings (default)
noexec=off: disable non-executable mappings

+ nosmep [X86]
+ Disable SMEP (Supervisor Mode Execution Protection)
+ even if it is supported by processor.

+
noexec32 [X86-64]
This affects only 32-bit executables.
noexec32=on: enable non-executable mappings (default)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c

index 173f3a3..cbc70a2 100644

--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -254,6 +254,25 @@ static inline void squash_the_stupid_serial_number(struct cpuinfo_x86 *c)
}
#endif

+static int disable_smep __initdata;

+static __init int setup_disable_smep(char *arg)

+{
+ disable_smep = 1;
+ return 1;
+}
+__setup("nosmep", setup_disable_smep);

+

+static __init void setup_smep(struct cpuinfo_x86 *c)
+{
+ if (cpu_has(c, X86_FEATURE_SMEP)) {
+ if (unlikely(disable_smep)) {
+ setup_clear_cpu_cap(X86_FEATURE_SMEP);
+ clear_in_cr4(X86_CR4_SMEP);
+ } else
+ set_in_cr4(X86_CR4_SMEP);

+ }
+}
+
/*
* Some CPU features depend on higher CPUID levels, which may not always
* be available due to CPUID level capping or broken virtualization

@@ -667,6 +686,8 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
c->cpu_index = 0;
#endif
filter_cpuid_features(c, false);
+
+ setup_smep(c);
}

void __init early_cpu_init(void)
@@ -752,6 +773,8 @@ static void __cpuinit generic_identify(struct cpuinfo_x86 *c)
#endif
}

+ setup_smep(c);
+
get_model_name(c); /* Default name */

detect_nopl(c);

[Tian, Kevin]

> From: Avi Kivity
> Sent: Tuesday, May 17, 2011 7:34 PM

>
> On 05/17/2011 01:46 PM, Ingo Molnar wrote:
> > >
> > > We could support it as a non-default feature, but that reduces its utility.
> >
> > It would be a nice touch for tools/kvm/: we would use KVM_GET_SREGS
> > and KVM_GET_SREGS to twiddle CR4.SMEP, even without the guest explicitly
> doing it.
> >
> > A quick glance suggests that it could be done straight away in
> > tools/kvm/kvm-cpu.c::kvm_cpu__setup_sregs() during vcpu setup, and
> > hopefully that cr4 value survives boot and ends up in the guest
> > kernel's mmu_cr4_features mask shadow register.
>
> Depends if the guest uses a read-modify-write pattern or not. We could do it
> transparently in kvm.ko, since the real cr4 need not corresponds to the guest
> notion (for example, we often set cr0.wp or cr0.ts even though the guest wants
> them clear).
>

We have a patch in hand now to enable SMEP for kvm guest, which is under
test and cleanup and will be sent out soon. Basically it contains the basic
idea by exposing CR4.smep and cpuid leaf. Above idea is nice and we'll try
that in a following work.

Thanks
Kevin

[Shan, Haitao]

After this basic enabling work, we could work out a patch to address Ingo's idea, which will mandatorily turn on SMEP for guests.

Shan Haitao

[Ingo Molnar]

* Shan, Haitao <haita...@intel.com> wrote:

> After this basic enabling work, we could work out a patch to address Ingo's
> idea, which will mandatorily turn on SMEP for guests.

FYI, basic host arch/x86/ kernel SMEP support is now upstream (sent it to Linus
yesterday) and will be released as part of v2.6.40.

Thanks,

Ingo

[Yinghai Lu]

should be __cpuinit for this function.

otherwise got:

WARNING: arch/x86/built-in.o(.cpuinit.text+0x1b10): Section mismatch in reference from the function identify_cpu() to the function .init.text:setup_smep()
The function __cpuinit identify_cpu() references
a function __init setup_smep().
If setup_smep is only used by identify_cpu then
annotate setup_smep with a matching annotation.

> +
> /*
> * Some CPU features depend on higher CPUID levels, which may not always
> * be available due to CPUID level capping or broken virtualization
> @@ -667,6 +686,8 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
> c->cpu_index = 0;
> #endif
> filter_cpuid_features(c, false);
> +
> + setup_smep(c);
> }
>
> void __init early_cpu_init(void)
> @@ -752,6 +773,8 @@ static void __cpuinit generic_identify(struct cpuinfo_x86 *c)
> #endif
> }
>
> + setup_smep(c);
> +
> get_model_name(c); /* Default name */
>
> detect_nopl(c);
> --

> To unsubscribe from this list: send the line "unsubscribe linux-tip-commits" in

[Ingo Molnar]

* Yinghai Lu <yin...@kernel.org> wrote:

> > +static __init void setup_smep(struct cpuinfo_x86 *c)
> > +{
> > + if (cpu_has(c, X86_FEATURE_SMEP)) {
> > + if (unlikely(disable_smep)) {
> > + setup_clear_cpu_cap(X86_FEATURE_SMEP);
> > + clear_in_cr4(X86_CR4_SMEP);
> > + } else
> > + set_in_cr4(X86_CR4_SMEP);
> > + }
> > +}
>
> should be __cpuinit for this function.
>
> otherwise got:
>
> WARNING: arch/x86/built-in.o(.cpuinit.text+0x1b10): Section mismatch in reference from the function identify_cpu() to the function .init.text:setup_smep()
> The function __cpuinit identify_cpu() references
> a function __init setup_smep().
> If setup_smep is only used by identify_cpu then
> annotate setup_smep with a matching annotation.

Yeah, indeed, and it might in fact crash with certain .config's. Mind sending a
tested patch?

Thanks,

Ingo