[Patch] Disallow SMEP for PV guest

[Patch] Disallow SMEP for PV guest

[Yang, Wei Y]

This patch disallows SMEP for PV guest.

 Signed-off-by: Yang, Wei <wei.y.yang@intel.com>
 Signed-off-by: Shan, Haitao <haitao.shan@intel.com>
 Signed-off-by: Li, Xin <xin.li@intel.com>

---
 xc_cpufeature.h |    3 +++
 xc_cpuid_x86.c  |    5 +++++
 2 files changed, 8 insertions(+)

diff -r 6f72160d2d55 tools/libxc/xc_cpufeature.h
--- a/tools/libxc/xc_cpufeature.h	Wed Jun 01 19:57:32 2011 +0800
+++ b/tools/libxc/xc_cpufeature.h	Wed Jun 01 22:21:33 2011 +0800
@@ -141,4 +141,7 @@
 #define X86_FEATURE_TBM         (6*32+21) /* trailing bit manipulations */
 #define X86_FEATURE_TOPOEXT     (6*32+22) /* topology extensions CPUID leafs */
 
+/* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word 7 */
+#define X86_FEATURE_SMEP       (7*32+ 7) /* Supervisor Mode Execution Protection */
+
 #endif /* __LIBXC_CPUFEATURE_H */
diff -r 6f72160d2d55 tools/libxc/xc_cpuid_x86.c
--- a/tools/libxc/xc_cpuid_x86.c	Wed Jun 01 19:57:32 2011 +0800
+++ b/tools/libxc/xc_cpuid_x86.c	Wed Jun 01 22:21:33 2011 +0800
@@ -470,6 +470,11 @@
         xc_cpuid_config_xsave(xch, domid, xfeature_mask, input, regs);
         break;
 
+    case 7:
+        if ( input[1] == 0 )
+            clear_bit(X86_FEATURE_SMEP, regs[1]);
+        break;
+
     case 0x80000001:
         if ( !guest_64bit )
         {

Re: [Patch] Disallow SMEP for PV guest

[Konrad Rzeszutek Wilk]

On Wed, Jun 01, 2011 at 10:31:12PM +0800, Yang, Wei Y wrote:
> This patch disallows SMEP for PV guest.

What are the reasons for it? What do we gain from it?
> 
>  Signed-off-by: Yang, Wei <wei.y.yang@intel.com>
>  Signed-off-by: Shan, Haitao <haitao.shan@intel.com>
>  Signed-off-by: Li, Xin <xin.li@intel.com>
> 
> ---
>  xc_cpufeature.h |    3 +++
>  xc_cpuid_x86.c  |    5 +++++
>  2 files changed, 8 insertions(+)
> 
> diff -r 6f72160d2d55 tools/libxc/xc_cpufeature.h
> --- a/tools/libxc/xc_cpufeature.h	Wed Jun 01 19:57:32 2011 +0800
> +++ b/tools/libxc/xc_cpufeature.h	Wed Jun 01 22:21:33 2011 +0800
> @@ -141,4 +141,7 @@
>  #define X86_FEATURE_TBM         (6*32+21) /* trailing bit manipulations */
>  #define X86_FEATURE_TOPOEXT     (6*32+22) /* topology extensions CPUID leafs */
>  
> +/* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word 7 */
> +#define X86_FEATURE_SMEP       (7*32+ 7) /* Supervisor Mode Execution Protection */
> +
>  #endif /* __LIBXC_CPUFEATURE_H */
> diff -r 6f72160d2d55 tools/libxc/xc_cpuid_x86.c
> --- a/tools/libxc/xc_cpuid_x86.c	Wed Jun 01 19:57:32 2011 +0800
> +++ b/tools/libxc/xc_cpuid_x86.c	Wed Jun 01 22:21:33 2011 +0800
> @@ -470,6 +470,11 @@
>          xc_cpuid_config_xsave(xch, domid, xfeature_mask, input, regs);
>          break;
>  
> +    case 7:
> +        if ( input[1] == 0 )
> +            clear_bit(X86_FEATURE_SMEP, regs[1]);
> +        break;
> +
>      case 0x80000001:
>          if ( !guest_64bit )
>          {
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [Patch] Disallow SMEP for PV guest

[Jan Beulich]

>>> On 01.06.11 at 16:31, "Yang, Wei Y" <wei.y.yang@intel.com> wrote:
> This patch disallows SMEP for PV guest.

How about Dom0?

Jan

>  Signed-off-by: Yang, Wei <wei.y.yang@intel.com>
>  Signed-off-by: Shan, Haitao <haitao.shan@intel.com>
>  Signed-off-by: Li, Xin <xin.li@intel.com>
> 
> ---
>  xc_cpufeature.h |    3 +++
>  xc_cpuid_x86.c  |    5 +++++
>  2 files changed, 8 insertions(+)
> 
> diff -r 6f72160d2d55 tools/libxc/xc_cpufeature.h
> --- a/tools/libxc/xc_cpufeature.h	Wed Jun 01 19:57:32 2011 +0800
> +++ b/tools/libxc/xc_cpufeature.h	Wed Jun 01 22:21:33 2011 +0800
> @@ -141,4 +141,7 @@
>  #define X86_FEATURE_TBM         (6*32+21) /* trailing bit manipulations */
>  #define X86_FEATURE_TOPOEXT     (6*32+22) /* topology extensions CPUID 
> leafs */
>  
> +/* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word 7 */
> +#define X86_FEATURE_SMEP       (7*32+ 7) /* Supervisor Mode Execution 
> Protection */
> +
>  #endif /* __LIBXC_CPUFEATURE_H */
> diff -r 6f72160d2d55 tools/libxc/xc_cpuid_x86.c
> --- a/tools/libxc/xc_cpuid_x86.c	Wed Jun 01 19:57:32 2011 +0800
> +++ b/tools/libxc/xc_cpuid_x86.c	Wed Jun 01 22:21:33 2011 +0800
> @@ -470,6 +470,11 @@
>          xc_cpuid_config_xsave(xch, domid, xfeature_mask, input, regs);
>          break;
>  
> +    case 7:
> +        if ( input[1] == 0 )
> +            clear_bit(X86_FEATURE_SMEP, regs[1]);
> +        break;
> +
>      case 0x80000001:
>          if ( !guest_64bit )
>          {
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com 
> http://lists.xensource.com/xen-devel 

RE: [Patch] Disallow SMEP for PV guest

[Li, Xin]

> On Wed, Jun 01, 2011 at 10:31:12PM +0800, Yang, Wei Y wrote:
> > This patch disallows SMEP for PV guest.
> 
> What are the reasons for it? What do we gain from it?

X86_64 pv guests runs in ring3, which SMEP doesn't apply to.

Kernel supports SMEP will set it thru writing to CR4, probably we can silently ignore such writes from PV guests, but better to not let guest see it.

Thanks!
-Xin

Re: [Patch] Disallow SMEP for PV guest

[Keir Fraser]

On 01/06/2011 15:55, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com> wrote:

> On Wed, Jun 01, 2011 at 10:31:12PM +0800, Yang, Wei Y wrote:
>> This patch disallows SMEP for PV guest.
> 
> What are the reasons for it? What do we gain from it?

Good question. It looks like it would be harmless to let it through (and the
guest can see the bit in CR4 is set, and it will be getting the SMEP
experience anyhow). And this patch does not stop the flag being passed to
dom0.

 -- Keir

>> 
>>  Signed-off-by: Yang, Wei <wei.y.yang@intel.com>
>>  Signed-off-by: Shan, Haitao <haitao.shan@intel.com>
>>  Signed-off-by: Li, Xin <xin.li@intel.com>
>> 
>> ---
>>  xc_cpufeature.h |    3 +++
>>  xc_cpuid_x86.c  |    5 +++++
>>  2 files changed, 8 insertions(+)
>> 
>> diff -r 6f72160d2d55 tools/libxc/xc_cpufeature.h
>> --- a/tools/libxc/xc_cpufeature.h Wed Jun 01 19:57:32 2011 +0800
>> +++ b/tools/libxc/xc_cpufeature.h Wed Jun 01 22:21:33 2011 +0800
>> @@ -141,4 +141,7 @@
>>  #define X86_FEATURE_TBM         (6*32+21) /* trailing bit manipulations */
>>  #define X86_FEATURE_TOPOEXT     (6*32+22) /* topology extensions CPUID leafs
>> */
>>  
>> +/* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word 7 */
>> +#define X86_FEATURE_SMEP       (7*32+ 7) /* Supervisor Mode Execution
>> Protection */
>> +
>>  #endif /* __LIBXC_CPUFEATURE_H */
>> diff -r 6f72160d2d55 tools/libxc/xc_cpuid_x86.c
>> --- a/tools/libxc/xc_cpuid_x86.c Wed Jun 01 19:57:32 2011 +0800
>> +++ b/tools/libxc/xc_cpuid_x86.c Wed Jun 01 22:21:33 2011 +0800
>> @@ -470,6 +470,11 @@
>>          xc_cpuid_config_xsave(xch, domid, xfeature_mask, input, regs);
>>          break;
>>  
>> +    case 7:
>> +        if ( input[1] == 0 )
>> +            clear_bit(X86_FEATURE_SMEP, regs[1]);
>> +        break;
>> +
>>      case 0x80000001:
>>          if ( !guest_64bit )
>>          {
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [Patch] Disallow SMEP for PV guest

[Keir Fraser]

On 01/06/2011 16:17, "Li, Xin" <xin.li@intel.com> wrote:

>> On Wed, Jun 01, 2011 at 10:31:12PM +0800, Yang, Wei Y wrote:
>>> This patch disallows SMEP for PV guest.
>> 
>> What are the reasons for it? What do we gain from it?
> 
> X86_64 pv guests runs in ring3, which SMEP doesn't apply to.
> 
> Kernel supports SMEP will set it thru writing to CR4, probably we can silently
> ignore such writes from PV guests, but better to not let guest see it.

Well, maybe. But if you hide the feature from the guest in CPUID then you
should also hide it in CR4, which will involve some messing with
real_cr4_to_pv_guest_cr4() and pv_guest_cr4_to_real_cr4(), in a fairly
obvious manner. And you should hide it in dom0's CPUID too.

 -- Keir

> Thanks!
> -Xin
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

RE: [Patch] Disallow SMEP for PV guest

[Li, Xin]

> >>> This patch disallows SMEP for PV guest.
> >>
> >> What are the reasons for it? What do we gain from it?
> >
> > X86_64 pv guests runs in ring3, which SMEP doesn't apply to.
> >
> > Kernel supports SMEP will set it thru writing to CR4, probably we can silently
> > ignore such writes from PV guests, but better to not let guest see it.
> 
> Well, maybe. But if you hide the feature from the guest in CPUID then you
> should also hide it in CR4, which will involve some messing with
> real_cr4_to_pv_guest_cr4() and pv_guest_cr4_to_real_cr4(), in a fairly
> obvious manner. And you should hide it in dom0's CPUID too.

People are very interested in this feature :).

As it can't apply to ring 3, x86_64 pv guest kernel accessing user code won't
trigger instruction fetch page fault.  thus it makes no sense to use it here.

Definitely we should hide it from dom0 kernel.  The change should be in Xen or pvops dom0?

thanks!
-Xin

Re: [Patch] Disallow SMEP for PV guest

[Konrad Rzeszutek Wilk]

On Thu, Jun 02, 2011 at 12:01:33AM +0800, Li, Xin wrote:
> > >>> This patch disallows SMEP for PV guest.
> > >>
> > >> What are the reasons for it? What do we gain from it?
> > >
> > > X86_64 pv guests runs in ring3, which SMEP doesn't apply to.
> > >
> > > Kernel supports SMEP will set it thru writing to CR4, probably we can silently
> > > ignore such writes from PV guests, but better to not let guest see it.
> > 
> > Well, maybe. But if you hide the feature from the guest in CPUID then you
> > should also hide it in CR4, which will involve some messing with
> > real_cr4_to_pv_guest_cr4() and pv_guest_cr4_to_real_cr4(), in a fairly
> > obvious manner. And you should hide it in dom0's CPUID too.
> 
> People are very interested in this feature :).

Hmm, can you give more details on what SMEP tries to do? The very interested
sounds like I should be aware of this but .. ah here it is:

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.

> 
> As it can't apply to ring 3, x86_64 pv guest kernel accessing user code won't
> trigger instruction fetch page fault.  thus it makes no sense to use it here.
> 
> Definitely we should hide it from dom0 kernel.  The change should be in Xen or pvops dom0?

Ugh, if have a patch against the paravirt kernel that would only cover the 3.1 kernel.
So you could still run with the SMEP enabled with the older kernels. Sounds like
a candidate for Xen hypervisor?

Re: [Patch] Disallow SMEP for PV guest

[Keir Fraser]

On 01/06/2011 18:27, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com> wrote:

>> As it can't apply to ring 3, x86_64 pv guest kernel accessing user code won't
>> trigger instruction fetch page fault.  thus it makes no sense to use it here.
>> 
>> Definitely we should hide it from dom0 kernel.  The change should be in Xen
>> or pvops dom0?
> 
> Ugh, if have a patch against the paravirt kernel that would only cover the 3.1
> kernel.
> So you could still run with the SMEP enabled with the older kernels. Sounds
> like
> a candidate for Xen hypervisor?

Definitely, it's a one liner to traps.c:pv_cpuid(). Given that the domU
patching is already done by the hypervisor (in libxc) obviously it should be
done by the hypervisor for dom0 also.

And the feature should be hidden in CR4, by the hypervisor also.

 -- Keir