[PATCH v2 0/4] Enable SMEP CPU Feature

[PATCH v2 0/4] Enable SMEP CPU Feature

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@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

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

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

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

Signed-off-by: Fenghua Yu <fenghua.yu@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 },
-- 
1.7.2

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

[H. Peter Anvin]

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

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.

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

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

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

Signed-off-by: Fenghua Yu <fenghua.yu@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
-- 
1.7.2

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

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@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 <fenghua.yu@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
-- 
1.7.2

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

[Matthew Garrett]

On Mon, May 16, 2011 at 02:34:44PM -0700, Fenghua Yu wrote:
> From: Fenghua Yu <fenghua.yu@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 | mjg59@srcf.ucam.org

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

[Yu, Fenghua]

> -----Original Message-----
> From: Matthew Garrett [mailto:mjg@redhat.com]
> 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 <fenghua.yu@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?

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

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

[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

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

[Yu, Fenghua]

> -----Original Message-----
> From: H. Peter Anvin [mailto:hpa@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

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

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

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 <fenghua.yu@intel.com>
---
 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

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

[Thomas Gleixner]

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

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

[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

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

[Ingo Molnar]

* H. Peter Anvin <hpa@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

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

[Avi Kivity]

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

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

[Ingo Molnar]

* Avi Kivity <avi@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

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

[Avi Kivity]

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

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

[Ingo Molnar]

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

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

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

[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

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

[Ingo Molnar]

* Avi Kivity <avi@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?

Thanks,

	Ingo

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

[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

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

[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.

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

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

[Pekka Enberg]

On Tue, May 17, 2011 at 2:47 PM, Avi Kivity <avi@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

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

[Ingo Molnar]

* Avi Kivity <avi@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

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

[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

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

[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

-----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

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

[Ingo Molnar]

* Shan, Haitao <haitao.shan@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

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

[Ingo Molnar]

* Fenghua Yu <fenghua.yu@intel.com> wrote:

> From: Fenghua Yu <fenghua.yu@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

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

[Avi Kivity]

On 05/17/2011 10:03 AM, Ingo Molnar wrote:
> * Fenghua Yu<fenghua.yu@intel.com>  wrote:
>
> >  From: Fenghua Yu<fenghua.yu@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