Re: [PATCH 1/4] x86/cpufeature: Add User-Mode Instruction Prevention definitions

2016-11-11 Thread Ricardo Neri 
On Fri, 2016-11-11 at 11:22 +0100, Borislav Petkov wrote:
> On Thu, Nov 10, 2016 at 08:08:07PM -0800, Ricardo Neri wrote:
> > UMIP is enabled by setting a bit in CR4. If that bit is not supposed
> > to be set, that could cause a #GP fault.
> 
> Yeah, you do check CPUID first, AFAICT, so you should be ok...

Right. I missed this detail. Yes, there should not be a problem.
> 
> -- 
> Regards/Gruss,
> Boris.
> 
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 
> 21284 (AG Nürnberg)


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] x86/cpufeature: Add User-Mode Instruction Prevention definitions

2016-11-11 Thread Dave Hansen 
On 11/10/2016 08:08 PM, Ricardo Neri wrote:
> Thanks for the suggestions. Perhaps I can include these metrics in my
> V2. On th other hand, Dave Hansen gave a good argument on potential
> conflicts when, of instance running on an AMD CPU. UMIP is enabled by
> setting a bit in CR4. If that bit is not supposed to be set, that could
> cause a #GP fault.

I just meant that some folks probably appreciate being able to build out
all the Intel-specific features.  Not that it causes a functional problem.


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] x86/cpufeature: Add User-Mode Instruction Prevention definitions

2016-11-11 Thread Borislav Petkov 
On Thu, Nov 10, 2016 at 08:08:07PM -0800, Ricardo Neri wrote:
> UMIP is enabled by setting a bit in CR4. If that bit is not supposed
> to be set, that could cause a #GP fault.

Yeah, you do check CPUID first, AFAICT, so you should be ok...

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] x86/cpufeature: Add User-Mode Instruction Prevention definitions

2016-11-10 Thread Ricardo Neri 
On Thu, 2016-11-10 at 09:58 +0100, Borislav Petkov wrote:
> On Wed, Nov 09, 2016 at 07:24:43PM -0800, Ricardo Neri wrote:
> > I intended this feature to be configurable at build time in case someone
> > wants to build a kernel without it; similar to other features such as
> > SMAP. Is this not needed? Should Linux be built with this feature always
> > enabled?
> > 
> > This feature could always be disabled via a kernel parameter, though;
> > even if Linux is built with it.
> 
> It probably is a good idea to have it build-time configurable for the
> embedded folks. But you can do a before and after build and look at
> the vmlinux size and see how much it has grown. If it is only a couple
> of KBs I guess we can drop the config option even but I know there are
> people who still care about KBs too...

Thanks for the suggestions. Perhaps I can include these metrics in my
V2. On th other hand, Dave Hansen gave a good argument on potential
conflicts when, of instance running on an AMD CPU. UMIP is enabled by
setting a bit in CR4. If that bit is not supposed to be set, that could
cause a #GP fault.

Thanks and BR,
Ricardo
> 
> -- 
> Regards/Gruss,
> Boris.
> 
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 
> 21284 (AG Nürnberg)


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] x86/cpufeature: Add User-Mode Instruction Prevention definitions

2016-11-10 Thread Dave Hansen 
On 11/09/2016 07:24 PM, Ricardo Neri wrote:
> On Wed, 2016-11-09 at 03:02 -0800, Andy Lutomirski wrote:
...
>> > What I mean is: why does this need a config option at all?
> I intended this feature to be configurable at build time in case someone
> wants to build a kernel without it; similar to other features such as
> SMAP. Is this not needed? Should Linux be built with this feature always
> enabled?

I think marking these features with their own CONFIG's is a really good
idea.  It helps the tinification effort.  It's also nice for folks that
might want to turn all the Intel features off because they're running on
AMD or something.

We don't necessarily need prompts for *everything*, but I can't imagine
just slapping the code in without #ifdefs of any kind.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] x86/cpufeature: Add User-Mode Instruction Prevention definitions

2016-11-10 Thread Borislav Petkov 
On Wed, Nov 09, 2016 at 07:24:43PM -0800, Ricardo Neri wrote:
> I intended this feature to be configurable at build time in case someone
> wants to build a kernel without it; similar to other features such as
> SMAP. Is this not needed? Should Linux be built with this feature always
> enabled?
> 
> This feature could always be disabled via a kernel parameter, though;
> even if Linux is built with it.

It probably is a good idea to have it build-time configurable for the
embedded folks. But you can do a before and after build and look at
the vmlinux size and see how much it has grown. If it is only a couple
of KBs I guess we can drop the config option even but I know there are
people who still care about KBs too...

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] x86/cpufeature: Add User-Mode Instruction Prevention definitions

2016-11-09 Thread Ricardo Neri 
On Wed, 2016-11-09 at 03:02 -0800, Andy Lutomirski wrote:
> On Tue, Nov 8, 2016 at 8:25 PM, Ricardo Neri
>  wrote:
> > On Tue, 2016-11-08 at 07:32 -0800, Andy Lutomirski wrote:
> >> > diff --git a/arch/x86/include/asm/disabled-features.h
> >> b/arch/x86/include/asm/disabled-features.h
> >> > index 85599ad..4707445 100644
> >> > --- a/arch/x86/include/asm/disabled-features.h
> >> > +++ b/arch/x86/include/asm/disabled-features.h
> >> > @@ -16,6 +16,12 @@
> >> >  # define DISABLE_MPX   (1<<(X86_FEATURE_MPX & 31))
> >> >  #endif
> >> >
> >> > +#ifdef CONFIG_X86_INTEL_UMIP
> >>
> >> ^
> >>
> >> What's this?
> >>
> >> Let's try to do this with a minimum of configuration.
> >
> > My intention here is put in this file all the #if build configurations
> > so that I don't have to put them other files by using functions such as
> > cpu_feature_enable. Isn't this the intention of this file?
> 
> What I mean is: why does this need a config option at all?

I intended this feature to be configurable at build time in case someone
wants to build a kernel without it; similar to other features such as
SMAP. Is this not needed? Should Linux be built with this feature always
enabled?

This feature could always be disabled via a kernel parameter, though;
even if Linux is built with it.

Thanks and BR,
Ricardo
> 
> --Andy


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] x86/cpufeature: Add User-Mode Instruction Prevention definitions

2016-11-09 Thread Andy Lutomirski 
On Tue, Nov 8, 2016 at 8:25 PM, Ricardo Neri
 wrote:
> On Tue, 2016-11-08 at 07:32 -0800, Andy Lutomirski wrote:
>> > diff --git a/arch/x86/include/asm/disabled-features.h
>> b/arch/x86/include/asm/disabled-features.h
>> > index 85599ad..4707445 100644
>> > --- a/arch/x86/include/asm/disabled-features.h
>> > +++ b/arch/x86/include/asm/disabled-features.h
>> > @@ -16,6 +16,12 @@
>> >  # define DISABLE_MPX   (1<<(X86_FEATURE_MPX & 31))
>> >  #endif
>> >
>> > +#ifdef CONFIG_X86_INTEL_UMIP
>>
>> ^
>>
>> What's this?
>>
>> Let's try to do this with a minimum of configuration.
>
> My intention here is put in this file all the #if build configurations
> so that I don't have to put them other files by using functions such as
> cpu_feature_enable. Isn't this the intention of this file?

What I mean is: why does this need a config option at all?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] x86/cpufeature: Add User-Mode Instruction Prevention definitions

2016-11-08 Thread Ricardo Neri 
On Tue, 2016-11-08 at 07:32 -0800, Andy Lutomirski wrote:
> > diff --git a/arch/x86/include/asm/disabled-features.h
> b/arch/x86/include/asm/disabled-features.h
> > index 85599ad..4707445 100644
> > --- a/arch/x86/include/asm/disabled-features.h
> > +++ b/arch/x86/include/asm/disabled-features.h
> > @@ -16,6 +16,12 @@
> >  # define DISABLE_MPX   (1<<(X86_FEATURE_MPX & 31))
> >  #endif
> >
> > +#ifdef CONFIG_X86_INTEL_UMIP
> 
> ^
> 
> What's this?
> 
> Let's try to do this with a minimum of configuration.

My intention here is put in this file all the #if build configurations
so that I don't have to put them other files by using functions such as
cpu_feature_enable. Isn't this the intention of this file?

Thanks and BR,
Ricardo


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] x86/cpufeature: Add User-Mode Instruction Prevention definitions

2016-11-08 Thread Andy Lutomirski 
On Mon, Nov 7, 2016 at 10:12 PM, Ricardo Neri
 wrote:
> User-Mode Instruction Prevention (UMIP) is a security feature present in
> new Intel Processors. If enabled, it prevents the execution of certain
> instructions if the Current Privilege Level (CPL) is greater than 0. If
> these instructions were executed while in CPL > 0, user space applications
> could have access to system-wide settings such as the global and local
> descriptor tables, the task register and the interrupt descriptor table.
>
> These are the instructions covered by UMIP:
> * SGDT - Store Global Descriptor Table
> * SIDT - Store Interrupt Descriptor Table
> * SLDT - Store Local Descriptor Table
> * SMSW - Store Machine Status Word
> * STR - Store Task Register
>
> If any of these instructions is executed with CPL > 0, a general protection
> exception is issued when UMIP is enbled.
>
> Cc: Andy Lutomirski 
> Cc: Andrew Morton 
> Cc: Borislav Petkov 
> Cc: Brian Gerst 
> Cc: Chen Yucong 
> Cc: Chris Metcalf 
> Cc: Dave Hansen 
> Cc: Fenghua Yu 
> Cc: Huang Rui 
> Cc: Jiri Slaby 
> Cc: Jonathan Corbet 
> Cc: Michael S. Tsirkin 
> Cc: Paul Gortmaker 
> Cc: Peter Zijlstra 
> Cc: Ravi V. Shankar 
> Cc: Shuah Khan 
> Cc: Vlastimil Babka 
> Signed-off-by: Ricardo Neri 
> ---
>  arch/x86/include/asm/cpufeatures.h  | 1 +
>  arch/x86/include/asm/disabled-features.h| 8 +++-
>  arch/x86/include/uapi/asm/processor-flags.h | 2 ++
>  3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/cpufeatures.h 
> b/arch/x86/include/asm/cpufeatures.h
> index 5f0931b..81ef3bbe 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -282,6 +282,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 
> Protection */
>  #define X86_FEATURE_PKU(16*32+ 3) /* Protection Keys for 
> Userspace */
>  #define X86_FEATURE_OSPKE  (16*32+ 4) /* OS Protection Keys Enable */
>
> diff --git a/arch/x86/include/asm/disabled-features.h 
> b/arch/x86/include/asm/disabled-features.h
> index 85599ad..4707445 100644
> --- a/arch/x86/include/asm/disabled-features.h
> +++ b/arch/x86/include/asm/disabled-features.h
> @@ -16,6 +16,12 @@
>  # define DISABLE_MPX   (1<<(X86_FEATURE_MPX & 31))
>  #endif
>
> +#ifdef CONFIG_X86_INTEL_UMIP

^

What's this?

Let's try to do this with a minimum of configuration.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] x86/cpufeature: Add User-Mode Instruction Prevention definitions

2016-11-07 Thread Ricardo Neri 
User-Mode Instruction Prevention (UMIP) is a security feature present in
new Intel Processors. If enabled, it prevents the execution of certain
instructions if the Current Privilege Level (CPL) is greater than 0. If
these instructions were executed while in CPL > 0, user space applications
could have access to system-wide settings such as the global and local
descriptor tables, the task register and the interrupt descriptor table.

These are the instructions covered by UMIP:
* SGDT - Store Global Descriptor Table
* SIDT - Store Interrupt Descriptor Table
* SLDT - Store Local Descriptor Table
* SMSW - Store Machine Status Word
* STR - Store Task Register

If any of these instructions is executed with CPL > 0, a general protection
exception is issued when UMIP is enbled.

Cc: Andy Lutomirski 
Cc: Andrew Morton 
Cc: Borislav Petkov 
Cc: Brian Gerst 
Cc: Chen Yucong 
Cc: Chris Metcalf 
Cc: Dave Hansen 
Cc: Fenghua Yu 
Cc: Huang Rui 
Cc: Jiri Slaby 
Cc: Jonathan Corbet 
Cc: Michael S. Tsirkin 
Cc: Paul Gortmaker 
Cc: Peter Zijlstra 
Cc: Ravi V. Shankar 
Cc: Shuah Khan 
Cc: Vlastimil Babka 
Signed-off-by: Ricardo Neri 
---
 arch/x86/include/asm/cpufeatures.h  | 1 +
 arch/x86/include/asm/disabled-features.h| 8 +++-
 arch/x86/include/uapi/asm/processor-flags.h | 2 ++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeatures.h 
b/arch/x86/include/asm/cpufeatures.h
index 5f0931b..81ef3bbe 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -282,6 +282,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 Protection 
*/
 #define X86_FEATURE_PKU(16*32+ 3) /* Protection Keys for 
Userspace */
 #define X86_FEATURE_OSPKE  (16*32+ 4) /* OS Protection Keys Enable */
 
diff --git a/arch/x86/include/asm/disabled-features.h 
b/arch/x86/include/asm/disabled-features.h
index 85599ad..4707445 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -16,6 +16,12 @@
 # define DISABLE_MPX   (1<<(X86_FEATURE_MPX & 31))
 #endif
 
+#ifdef CONFIG_X86_INTEL_UMIP
+# define DISABLE_UMIP  0
+#else
+# define DISABLE_UMIP  (1<<(X86_FEATURE_UMIP & 31))
+#endif
+
 #ifdef CONFIG_X86_64
 # define DISABLE_VME   (1<<(X86_FEATURE_VME & 31))
 # define DISABLE_K6_MTRR   (1<<(X86_FEATURE_K6_MTRR & 31))
@@ -55,7 +61,7 @@
 #define DISABLED_MASK130
 #define DISABLED_MASK140
 #define DISABLED_MASK150
-#define DISABLED_MASK16(DISABLE_PKU|DISABLE_OSPKE)
+#define DISABLED_MASK16(DISABLE_PKU|DISABLE_OSPKE|DISABLE_UMIP)
 #define DISABLED_MASK170
 #define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 18)
 
diff --git a/arch/x86/include/uapi/asm/processor-flags.h 
b/arch/x86/include/uapi/asm/processor-flags.h
index 567de50..d2c2af8 100644
--- a/arch/x86/include/uapi/asm/processor-flags.h
+++ b/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 /* enable UMIP support */
+#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) */
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html