Re: [PATCH] x86: add UMIP support

[Ricardo Neri]

Hi Jan,

On Thu, 2016-11-17 at 07:52 +0100, Ingo Molnar wrote:
> * Jan Beulich  wrote:
> 
> > This is a small aid to security, hiding in particular the kernel address
> > information otherwise available through SGDT/SIDT.
> > 
> > Signed-off-by: Jan Beulich 
> > ---
> > Main question here is whether to limit this to 64-bit (or at least
> > !CONFIG_VM86) for the time being, or to disable it while running VM86
> > mode code: Such code isn't unlikely to use SMSW (and one of SGDT/SIDT)
> > to figure out whether it's running on an i286 or i386, as the EFLAGS
> > based method recommended by Intel's SDM can't be relied upon there.

For vm86, the consensus now is to emulate these instructions by trapping
the GP fault that UMIP causes with these instructions and give the user
space fake pointers to SGDT/SIDT. In this way we hide these tables and
the vm86 application does not see the fault.

I am finishing v2 of my patches and I will send them shortly.

Thanks and BR,
Ricardo
> > ---
> >  Documentation/kernel-parameters.txt |4 
> >  arch/x86/include/asm/cpufeatures.h  |1 +
> >  arch/x86/include/uapi/asm/processor-flags.h |2 ++
> >  arch/x86/kernel/cpu/common.c|   16 +++-
> >  4 files changed, 22 insertions(+), 1 deletion(-)
> 
> Note that similar patches have been submitted a week ago:
> 
>   https://lkml.org/lkml/2016/11/8/68
> 
> Thanks,
> 
>   Ingo

Re: [PATCH] x86: add UMIP support

[Ricardo Neri]

Hi Jan,

On Thu, 2016-11-17 at 07:52 +0100, Ingo Molnar wrote:
> * Jan Beulich  wrote:
> 
> > This is a small aid to security, hiding in particular the kernel address
> > information otherwise available through SGDT/SIDT.
> > 
> > Signed-off-by: Jan Beulich 
> > ---
> > Main question here is whether to limit this to 64-bit (or at least
> > !CONFIG_VM86) for the time being, or to disable it while running VM86
> > mode code: Such code isn't unlikely to use SMSW (and one of SGDT/SIDT)
> > to figure out whether it's running on an i286 or i386, as the EFLAGS
> > based method recommended by Intel's SDM can't be relied upon there.

For vm86, the consensus now is to emulate these instructions by trapping
the GP fault that UMIP causes with these instructions and give the user
space fake pointers to SGDT/SIDT. In this way we hide these tables and
the vm86 application does not see the fault.

I am finishing v2 of my patches and I will send them shortly.

Thanks and BR,
Ricardo
> > ---
> >  Documentation/kernel-parameters.txt |4 
> >  arch/x86/include/asm/cpufeatures.h  |1 +
> >  arch/x86/include/uapi/asm/processor-flags.h |2 ++
> >  arch/x86/kernel/cpu/common.c|   16 +++-
> >  4 files changed, 22 insertions(+), 1 deletion(-)
> 
> Note that similar patches have been submitted a week ago:
> 
>   https://lkml.org/lkml/2016/11/8/68
> 
> Thanks,
> 
>   Ingo

Re: [PATCH] x86: add UMIP support

[Ingo Molnar]

* Jan Beulich  wrote:

> This is a small aid to security, hiding in particular the kernel address
> information otherwise available through SGDT/SIDT.
> 
> Signed-off-by: Jan Beulich 
> ---
> Main question here is whether to limit this to 64-bit (or at least
> !CONFIG_VM86) for the time being, or to disable it while running VM86
> mode code: Such code isn't unlikely to use SMSW (and one of SGDT/SIDT)
> to figure out whether it's running on an i286 or i386, as the EFLAGS
> based method recommended by Intel's SDM can't be relied upon there.
> ---
>  Documentation/kernel-parameters.txt |4 
>  arch/x86/include/asm/cpufeatures.h  |1 +
>  arch/x86/include/uapi/asm/processor-flags.h |2 ++
>  arch/x86/kernel/cpu/common.c|   16 +++-
>  4 files changed, 22 insertions(+), 1 deletion(-)

Note that similar patches have been submitted a week ago:

  https://lkml.org/lkml/2016/11/8/68

Thanks,

Ingo

Re: [PATCH] x86: add UMIP support

[Ingo Molnar]

* Jan Beulich  wrote:

> This is a small aid to security, hiding in particular the kernel address
> information otherwise available through SGDT/SIDT.
> 
> Signed-off-by: Jan Beulich 
> ---
> Main question here is whether to limit this to 64-bit (or at least
> !CONFIG_VM86) for the time being, or to disable it while running VM86
> mode code: Such code isn't unlikely to use SMSW (and one of SGDT/SIDT)
> to figure out whether it's running on an i286 or i386, as the EFLAGS
> based method recommended by Intel's SDM can't be relied upon there.
> ---
>  Documentation/kernel-parameters.txt |4 
>  arch/x86/include/asm/cpufeatures.h  |1 +
>  arch/x86/include/uapi/asm/processor-flags.h |2 ++
>  arch/x86/kernel/cpu/common.c|   16 +++-
>  4 files changed, 22 insertions(+), 1 deletion(-)

Note that similar patches have been submitted a week ago:

  https://lkml.org/lkml/2016/11/8/68

Thanks,

Ingo

[PATCH] x86: add UMIP support

[Jan Beulich]

This is a small aid to security, hiding in particular the kernel address
information otherwise available through SGDT/SIDT.

Signed-off-by: Jan Beulich 
---
Main question here is whether to limit this to 64-bit (or at least
!CONFIG_VM86) for the time being, or to disable it while running VM86
mode code: Such code isn't unlikely to use SMSW (and one of SGDT/SIDT)
to figure out whether it's running on an i286 or i386, as the EFLAGS
based method recommended by Intel's SDM can't be relied upon there.
---
 Documentation/kernel-parameters.txt |4 
 arch/x86/include/asm/cpufeatures.h  |1 +
 arch/x86/include/uapi/asm/processor-flags.h |2 ++
 arch/x86/kernel/cpu/common.c|   16 +++-
 4 files changed, 22 insertions(+), 1 deletion(-)

--- 4.9-rc5/Documentation/kernel-parameters.txt
+++ 4.9-rc5-x86-UMIP/Documentation/kernel-parameters.txt
@@ -2814,6 +2814,10 @@ bytes respectively. Such letter suffixes
 
notsc   [BUGS=X86-32] Disable Time Stamp Counter
 
+   noumip  [X86]
+   Disable UMIP (User Mode Instruction Prevention)
+   even if it is supported by processor.
+
nowatchdog  [KNL] Disable both lockup detectors, i.e.
 soft-lockup and NMI watchdog (hard-lockup).
 
--- 4.9-rc5/arch/x86/include/asm/cpufeatures.h
+++ 4.9-rc5-x86-UMIP/arch/x86/include/asm/cpufeatures.h
@@ -279,6 +279,7 @@
 #define X86_FEATURE_AVIC   (15*32+13) /* Virtual Interrupt Controller */
 
 /* Intel-defined CPU features, CPUID level 0x0007:0 (ecx), word 16 */
+#define X86_FEATURE_UMIP   (16*32+ 2) /* User Mode Instruction Prevention 
*/
 #define X86_FEATURE_PKU(16*32+ 3) /* Protection Keys for 
Userspace */
 #define X86_FEATURE_OSPKE  (16*32+ 4) /* OS Protection Keys Enable */
 
--- 4.9-rc5/arch/x86/include/uapi/asm/processor-flags.h
+++ 4.9-rc5-x86-UMIP/arch/x86/include/uapi/asm/processor-flags.h
@@ -104,6 +104,8 @@
 #define X86_CR4_OSFXSR _BITUL(X86_CR4_OSFXSR_BIT)
 #define X86_CR4_OSXMMEXCPT_BIT 10 /* enable unmasked SSE exceptions */
 #define X86_CR4_OSXMMEXCPT _BITUL(X86_CR4_OSXMMEXCPT_BIT)
+#define X86_CR4_UMIP_BIT   11 /* disable user mode S[GIL]DT/STR/SMSW */
+#define X86_CR4_UMIP   _BITUL(X86_CR4_UMIP_BIT)
 #define X86_CR4_VMXE_BIT   13 /* enable VMX virtualization */
 #define X86_CR4_VMXE   _BITUL(X86_CR4_VMXE_BIT)
 #define X86_CR4_SMXE_BIT   14 /* enable safer mode (TXT) */
--- 4.9-rc5/arch/x86/kernel/cpu/common.c
+++ 4.9-rc5-x86-UMIP/arch/x86/kernel/cpu/common.c
@@ -306,6 +306,19 @@ static __always_inline void setup_smap(s
}
 }
 
+static __init int setup_disable_umip(char *arg)
+{
+   setup_clear_cpu_cap(X86_FEATURE_UMIP);
+   return 1;
+}
+__setup("noumip", setup_disable_umip);
+
+static __always_inline void setup_umip(struct cpuinfo_x86 *c)
+{
+   if (cpu_has(c, X86_FEATURE_UMIP))
+   cr4_set_bits(X86_CR4_UMIP);
+}
+
 /*
  * Protection Keys are not available in 32-bit mode.
  */
@@ -1037,9 +1050,10 @@ static void identify_cpu(struct cpuinfo_
/* Disable the PN if appropriate */
squash_the_stupid_serial_number(c);
 
-   /* Set up SMEP/SMAP */
+   /* Set up SMEP/SMAP/UMIP */
setup_smep(c);
setup_smap(c);
+   setup_umip(c);
 
/*
 * The vendor-specific functions might have changed features.

[PATCH] x86: add UMIP support

[Jan Beulich]

This is a small aid to security, hiding in particular the kernel address
information otherwise available through SGDT/SIDT.

Signed-off-by: Jan Beulich 
---
Main question here is whether to limit this to 64-bit (or at least
!CONFIG_VM86) for the time being, or to disable it while running VM86
mode code: Such code isn't unlikely to use SMSW (and one of SGDT/SIDT)
to figure out whether it's running on an i286 or i386, as the EFLAGS
based method recommended by Intel's SDM can't be relied upon there.
---
 Documentation/kernel-parameters.txt |4 
 arch/x86/include/asm/cpufeatures.h  |1 +
 arch/x86/include/uapi/asm/processor-flags.h |2 ++
 arch/x86/kernel/cpu/common.c|   16 +++-
 4 files changed, 22 insertions(+), 1 deletion(-)

--- 4.9-rc5/Documentation/kernel-parameters.txt
+++ 4.9-rc5-x86-UMIP/Documentation/kernel-parameters.txt
@@ -2814,6 +2814,10 @@ bytes respectively. Such letter suffixes
 
notsc   [BUGS=X86-32] Disable Time Stamp Counter
 
+   noumip  [X86]
+   Disable UMIP (User Mode Instruction Prevention)
+   even if it is supported by processor.
+
nowatchdog  [KNL] Disable both lockup detectors, i.e.
 soft-lockup and NMI watchdog (hard-lockup).
 
--- 4.9-rc5/arch/x86/include/asm/cpufeatures.h
+++ 4.9-rc5-x86-UMIP/arch/x86/include/asm/cpufeatures.h
@@ -279,6 +279,7 @@
 #define X86_FEATURE_AVIC   (15*32+13) /* Virtual Interrupt Controller */
 
 /* Intel-defined CPU features, CPUID level 0x0007:0 (ecx), word 16 */
+#define X86_FEATURE_UMIP   (16*32+ 2) /* User Mode Instruction Prevention 
*/
 #define X86_FEATURE_PKU(16*32+ 3) /* Protection Keys for 
Userspace */
 #define X86_FEATURE_OSPKE  (16*32+ 4) /* OS Protection Keys Enable */
 
--- 4.9-rc5/arch/x86/include/uapi/asm/processor-flags.h
+++ 4.9-rc5-x86-UMIP/arch/x86/include/uapi/asm/processor-flags.h
@@ -104,6 +104,8 @@
 #define X86_CR4_OSFXSR _BITUL(X86_CR4_OSFXSR_BIT)
 #define X86_CR4_OSXMMEXCPT_BIT 10 /* enable unmasked SSE exceptions */
 #define X86_CR4_OSXMMEXCPT _BITUL(X86_CR4_OSXMMEXCPT_BIT)
+#define X86_CR4_UMIP_BIT   11 /* disable user mode S[GIL]DT/STR/SMSW */
+#define X86_CR4_UMIP   _BITUL(X86_CR4_UMIP_BIT)
 #define X86_CR4_VMXE_BIT   13 /* enable VMX virtualization */
 #define X86_CR4_VMXE   _BITUL(X86_CR4_VMXE_BIT)
 #define X86_CR4_SMXE_BIT   14 /* enable safer mode (TXT) */
--- 4.9-rc5/arch/x86/kernel/cpu/common.c
+++ 4.9-rc5-x86-UMIP/arch/x86/kernel/cpu/common.c
@@ -306,6 +306,19 @@ static __always_inline void setup_smap(s
}
 }
 
+static __init int setup_disable_umip(char *arg)
+{
+   setup_clear_cpu_cap(X86_FEATURE_UMIP);
+   return 1;
+}
+__setup("noumip", setup_disable_umip);
+
+static __always_inline void setup_umip(struct cpuinfo_x86 *c)
+{
+   if (cpu_has(c, X86_FEATURE_UMIP))
+   cr4_set_bits(X86_CR4_UMIP);
+}
+
 /*
  * Protection Keys are not available in 32-bit mode.
  */
@@ -1037,9 +1050,10 @@ static void identify_cpu(struct cpuinfo_
/* Disable the PN if appropriate */
squash_the_stupid_serial_number(c);
 
-   /* Set up SMEP/SMAP */
+   /* Set up SMEP/SMAP/UMIP */
setup_smep(c);
setup_smap(c);
+   setup_umip(c);
 
/*
 * The vendor-specific functions might have changed features.