2011-05-16 21:47:25

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v2 0/4] Enable SMEP CPU Feature

From: Fenghua Yu <[email protected]>

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


2011-05-16 21:47:56

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v2 1/4] x86, cpu: Add CPU flags for SMEP

From: Fenghua Yu <[email protected]>

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

Signed-off-by: Fenghua Yu <[email protected]>
---
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 },
--
1.7.2

2011-05-16 21:47:55

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v2 2/4] x86, cpu: Add SMEP CPU feature in CR4

From: Fenghua Yu <[email protected]>

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

Signed-off-by: Fenghua Yu <[email protected]>
---
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
--
1.7.2

2011-05-16 21:47:28

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v2 3/4] x86, head_32/64.S: Enable SMEP

From: Fenghua Yu <[email protected]>

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 <[email protected]>
---
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
--
1.7.2

2011-05-16 21:47:26

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v2 4/4] x86/kernel/common.c: Disable SMEP by kernel option nosmep

From: Fenghua Yu <[email protected]>

SMEP is enabled unconditionally on all CPUs that support it, the nosmep boot
option would turn it off shortly afterwards.

Signed-off-by: Fenghua Yu <[email protected]>
---
Documentation/kernel-parameters.txt | 4 ++++
arch/x86/kernel/cpu/common.c | 22 ++++++++++++++++++++++
2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index cc85a92..76c67e5 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 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..cd0762a 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -254,6 +254,27 @@ static inline void squash_the_stupid_serial_number(struct cpuinfo_x86 *c)
}
#endif

+static int disable_smep __initdata;
+
+static __init int setup_nosmep(char *arg)
+{
+ disable_smep = 1;
+ return 1;
+}
+__setup("nosmep", setup_nosmep);
+
+/*
+ * If SMEP is supported by the processor, SMEP has been enabled in CR4 earlier.
+ * But if kernel option "nosmep" is given, we disable SMEP here.
+ */
+static __init void config_smep(struct cpuinfo_x86 *c)
+{
+ if (cpu_has(c, X86_FEATURE_SMEP) && unlikely(disable_smep)) {
+ setup_clear_cpu_cap(X86_FEATURE_SMEP);
+ clear_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
@@ -737,6 +758,7 @@ static void __cpuinit generic_identify(struct cpuinfo_x86 *c)
get_cpu_vendor(c);

get_cpu_cap(c);
+ config_smep(c);

if (c->cpuid_level >= 0x00000001) {
c->initial_apicid = (cpuid_ebx(1) >> 24) & 0xFF;
--
1.7.2

2011-05-16 21:53:11

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable SMEP CPU Feature

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

2011-05-16 22:02:15

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] x86/kernel/common.c: Disable SMEP by kernel option nosmep

On Mon, 16 May 2011, Fenghua Yu wrote:
> +/*
> + * If SMEP is supported by the processor, SMEP has been enabled in CR4 earlier.
> + * But if kernel option "nosmep" is given, we disable SMEP here.
> + */
> +static __init void config_smep(struct cpuinfo_x86 *c)
> +{
> + if (cpu_has(c, X86_FEATURE_SMEP) && unlikely(disable_smep)) {

That unlikely() is completely pointless. This is init code and inside
of a if() already. Where is the point ?

> + setup_clear_cpu_cap(X86_FEATURE_SMEP);
> + clear_in_cr4(X86_CR4_SMEP);
> + }
> +}

Thanks,

tglx

2011-05-17 01:57:40

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] x86, cpu: Add CPU flags for SMEP

On 05/16/2011 02:34 PM, Fenghua Yu wrote:
> From: Fenghua Yu <[email protected]>
>
> Add support for newly documented SMEP (Supervisor Mode Execution Protection) CPU
> feature flags.
>
> Signed-off-by: Fenghua Yu <[email protected]>
> ---
> 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 },

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.

2011-05-17 02:10:46

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] x86, head_32/64.S: Enable SMEP

On Mon, May 16, 2011 at 02:34:44PM -0700, Fenghua Yu wrote:
> From: Fenghua Yu <[email protected]>
>
> 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 | [email protected]

2011-05-17 07:03:36

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable SMEP CPU Feature


* Fenghua Yu <[email protected]> wrote:

> From: Fenghua Yu <[email protected]>
>
> 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

2011-05-17 07:05:46

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable SMEP CPU Feature


* H. Peter Anvin <[email protected]> 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

2011-05-17 09:16:46

by Avi Kivity

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable SMEP CPU Feature

On 05/17/2011 10:05 AM, Ingo Molnar wrote:
> * H. Peter Anvin<[email protected]> 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

2011-05-17 09:17:34

by Avi Kivity

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable SMEP CPU Feature

On 05/17/2011 10:03 AM, Ingo Molnar wrote:
> * Fenghua Yu<[email protected]> wrote:
>
> > From: Fenghua Yu<[email protected]>
> >
> > 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

2011-05-17 09:29:31

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable SMEP CPU Feature


* Avi Kivity <[email protected]> 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

2011-05-17 09:32:53

by Avi Kivity

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable SMEP CPU Feature

On 05/17/2011 12:29 PM, Ingo Molnar wrote:
> * Avi Kivity<[email protected]> 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

2011-05-17 10:47:24

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable SMEP CPU Feature


* Avi Kivity <[email protected]> wrote:

> On 05/17/2011 12:29 PM, Ingo Molnar wrote:
> >* Avi Kivity<[email protected]> 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

2011-05-17 11:35:16

by Avi Kivity

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable SMEP CPU Feature

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

2011-05-17 11:39:48

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable SMEP CPU Feature


* Avi Kivity <[email protected]> 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?

Thanks,

Ingo

2011-05-17 11:45:08

by Avi Kivity

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable SMEP CPU Feature

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

2011-05-17 11:48:11

by Avi Kivity

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable SMEP CPU Feature

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.

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

2011-05-17 11:49:22

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable SMEP CPU Feature

On Tue, May 17, 2011 at 2:47 PM, Avi Kivity <[email protected]> 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

2011-05-17 11:51:05

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable SMEP CPU Feature


* Avi Kivity <[email protected]> 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

2011-05-17 23:10:53

by Fenghua Yu

[permalink] [raw]
Subject: RE: [PATCH v2 3/4] x86, head_32/64.S: Enable SMEP

> -----Original Message-----
> From: Matthew Garrett [mailto:[email protected]]
> Sent: Monday, May 16, 2011 7:10 PM
> To: Yu, Fenghua
> Cc: Ingo Molnar; Thomas Gleixner; H Peter Anvin; 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 Mon, May 16, 2011 at 02:34:44PM -0700, Fenghua Yu wrote:
> > From: Fenghua Yu <[email protected]>
> >
> > 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?

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

2011-05-17 23:14:05

by H. Peter Anvin

[permalink] [raw]
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

2011-05-18 02:59:23

by Fenghua Yu

[permalink] [raw]
Subject: RE: [PATCH v2 3/4] x86, head_32/64.S: Enable SMEP

> -----Original Message-----
> From: H. Peter Anvin [mailto:[email protected]]
> 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

2011-05-19 06:11:23

by Tian, Kevin

[permalink] [raw]
Subject: RE: [PATCH v2 0/4] Enable SMEP CPU Feature

> 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

2011-05-19 06:42:37

by Shan, Haitao

[permalink] [raw]
Subject: RE: [PATCH v2 0/4] Enable SMEP CPU Feature

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

-----Original Message-----
From: Tian, Kevin
Sent: Thursday, May 19, 2011 2:11 PM
To: Avi Kivity; Ingo Molnar
Cc: H. Peter Anvin; Yu, Fenghua; Thomas Gleixner; Mallick, Asit K; Linus Torvalds; Arjan van de Ven; Andrew Morton; Andi Kleen; linux-kernel; Pekka Enberg; Shan, Haitao
Subject: RE: [PATCH v2 0/4] Enable SMEP CPU Feature

> 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

2011-05-20 08:21:42

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable SMEP CPU Feature


* Shan, Haitao <[email protected]> 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