Re: [v2 6/7] x86/traps: Fixup general protection faults caused by UMIP

2016-12-27 Thread Ricardo Neri 
On Fri, 2016-12-23 at 18:11 -0800, Andy Lutomirski wrote:
> On Fri, Dec 23, 2016 at 5:37 PM, Ricardo Neri
>  wrote:
> > If the User-Mode Instruction Prevention CPU feature is available and
> > enabled, a general protection fault will be issued if the instructions
> > sgdt, sldt, sidt, str or smsw are executed from user-mode context
> > (CPL > 0). If the fault was caused by any of the instructions protected
> > by UMIP, fixup_umip_exceptino will emulate dummy results for these
> > instructions.
> >
> > Cc: Andy Lutomirski 
> > Cc: Andrew Morton 
> > Cc: H. Peter Anvin 
> > 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 
> > Cc: Tony Luck 
> > Cc: Paolo Bonzini 
> > Cc: Liang Z. Li 
> > Cc: Alexandre Julliard 
> > Cc: Stas Sergeev 
> > Cc: x...@kernel.org
> > Cc: linux-msdos@vger.kernel.org
> > Signed-off-by: Ricardo Neri 
> > ---
> >  arch/x86/kernel/traps.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> > index bf0c6d0..5044fb3 100644
> > --- a/arch/x86/kernel/traps.c
> > +++ b/arch/x86/kernel/traps.c
> > @@ -64,6 +64,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #ifdef CONFIG_X86_64
> >  #include 
> > @@ -491,6 +492,9 @@ do_general_protection(struct pt_regs *regs, long 
> > error_code)
> > RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
> > cond_local_irq_enable(regs);
> >
> > +   if (user_mode(regs) && !fixup_umip_exception(regs))
> > +   return;
> > +
> 
> I would do fixup_umip_exception(regs) == 0 to make it more obvious
> what's going on.

Sure. I will make this change.
> 
> Also, since you're allowing this in v8086 mode, I think this should
> have an explicit test in
> tools/testing/selftests/x86/entry_from_vm86.c.  I *think* it will work
> fine, but the pt_regs handling in vm86 mode is quite scary and has
> been rewritten recently.

I will include a test for this.

Thanks and BR,
Ricardo


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

Re: [v2 6/7] x86/traps: Fixup general protection faults caused by UMIP

2016-12-23 Thread kbuild test robot 
Hi Ricardo,

[auto build test ERROR on tip/auto-latest]
[also build test ERROR on next-20161223]
[cannot apply to tip/x86/core v4.9]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Ricardo-Neri/x86-enable-User-Mode-Instruction-Prevention/20161224-094338
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   In file included from arch/x86/tools/test_get_len.c:27:0:
>> arch/x86/include/asm/insn.h:26:23: fatal error: linux/bug.h: No such file or 
>> directory
#include 
  ^
   compilation terminated.
--
   In file included from tools/include/linux/compiler.h:55:0,
from arch/x86/include/asm/insn.h:25,
from arch/x86/tools/insn_sanity.c:34:
>> tools/include/linux/types.h:28:18: error: conflicting types for 'u64'
typedef uint64_t u64;
 ^~~
   In file included from /usr/include/asm-generic/types.h:6:0,
from /usr/include/x86_64-linux-gnu/asm/types.h:4,
from tools/include/linux/types.h:9,
from tools/include/linux/compiler.h:55,
from arch/x86/include/asm/insn.h:25,
from arch/x86/tools/insn_sanity.c:34:
   include/asm-generic/int-ll64.h:25:28: note: previous declaration of 'u64' 
was here
typedef unsigned long long u64;
   ^~~
   In file included from tools/include/linux/compiler.h:55:0,
from arch/x86/include/asm/insn.h:25,
from arch/x86/tools/insn_sanity.c:34:
>> tools/include/linux/types.h:29:17: error: conflicting types for 's64'
typedef int64_t s64;
^~~
   In file included from /usr/include/asm-generic/types.h:6:0,
from /usr/include/x86_64-linux-gnu/asm/types.h:4,
from tools/include/linux/types.h:9,
from tools/include/linux/compiler.h:55,
from arch/x86/include/asm/insn.h:25,
from arch/x86/tools/insn_sanity.c:34:
   include/asm-generic/int-ll64.h:24:26: note: previous declaration of 's64' 
was here
typedef signed long long s64;
 ^~~
   In file included from tools/include/linux/types.h:4:0,
from tools/include/linux/compiler.h:55,
from arch/x86/include/asm/insn.h:25,
from arch/x86/tools/insn_sanity.c:34:
>> include/linux/stddef.h:10:2: error: expected identifier before numeric 
>> constant
 false = 0,
 ^
   In file included from arch/x86/include/asm/ptrace.h:5:0,
from arch/x86/include/asm/insn.h:28,
from arch/x86/tools/insn_sanity.c:34:
>> arch/x86/include/asm/page_types.h:61:15: error: unknown type name 
>> 'phys_addr_t'
static inline phys_addr_t get_max_mapped(void)
  ^~~
   arch/x86/include/asm/page_types.h: In function 'get_max_mapped':
>> arch/x86/include/asm/page_types.h:63:10: error: 'phys_addr_t' undeclared 
>> (first use in this function)
 return (phys_addr_t)max_pfn_mapped << PAGE_SHIFT;
 ^~~
   arch/x86/include/asm/page_types.h:63:10: note: each undeclared identifier is 
reported only once for each function it appears in
>> arch/x86/include/asm/page_types.h:63:22: error: expected ';' before 
>> 'max_pfn_mapped'
 return (phys_addr_t)max_pfn_mapped << PAGE_SHIFT;
 ^~
   In file included from arch/x86/include/asm/insn.h:28:0,
from arch/x86/tools/insn_sanity.c:34:
   arch/x86/include/asm/ptrace.h: At top level:
>> arch/x86/include/asm/ptrace.h:33:8: error: redefinition of 'struct pt_regs'
struct pt_regs {
   ^~~
   In file included from arch/x86/include/asm/insn.h:26:0,
from arch/x86/tools/insn_sanity.c:34:
   include/linux/bug.h:13:8: note: originally defined here
struct pt_regs;
   ^~~

vim +33 arch/x86/include/asm/ptrace.h

92bc2056 include/asm-x86/ptrace.h  Harvey Harrison 2008-02-08  27   
unsigned long sp;
92bc2056 include/asm-x86/ptrace.h  Harvey Harrison 2008-02-08  28   
unsigned long ss;
65ea5b03 include/asm-x86/ptrace.h  H. Peter Anvin  2008-01-30  29  };
8fc37f2c include/asm-x86/ptrace.h  Thomas Gleixner 2007-10-23  30  
8fc37f2c include/asm-x86/ptrace.h  Thomas Gleixner 2007-10-23  31  #else /* 
__i386__ */
8fc37f2c include/asm-x86/ptrace.h  Thomas Gleixner 2007-10-23  32  
65ea5b03 include/asm-x86/ptrace.h  H. Peter Anvin  2008-01-30 @33  struct 
pt_regs {
e90e147c arch/x86/include/asm/ptrace.h Denys Vlasenko  2015-02-26  34  /*
e90e147c arch/x86/include/asm/ptrace.h Denys Vlasenko  2015-02-26  35   * C ABI 
says these

Re: [v2 6/7] x86/traps: Fixup general protection faults caused by UMIP

2016-12-23 Thread Andy Lutomirski 
On Fri, Dec 23, 2016 at 5:37 PM, Ricardo Neri
 wrote:
> If the User-Mode Instruction Prevention CPU feature is available and
> enabled, a general protection fault will be issued if the instructions
> sgdt, sldt, sidt, str or smsw are executed from user-mode context
> (CPL > 0). If the fault was caused by any of the instructions protected
> by UMIP, fixup_umip_exceptino will emulate dummy results for these
> instructions.
>
> Cc: Andy Lutomirski 
> Cc: Andrew Morton 
> Cc: H. Peter Anvin 
> 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 
> Cc: Tony Luck 
> Cc: Paolo Bonzini 
> Cc: Liang Z. Li 
> Cc: Alexandre Julliard 
> Cc: Stas Sergeev 
> Cc: x...@kernel.org
> Cc: linux-msdos@vger.kernel.org
> Signed-off-by: Ricardo Neri 
> ---
>  arch/x86/kernel/traps.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index bf0c6d0..5044fb3 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -64,6 +64,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #ifdef CONFIG_X86_64
>  #include 
> @@ -491,6 +492,9 @@ do_general_protection(struct pt_regs *regs, long 
> error_code)
> RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
> cond_local_irq_enable(regs);
>
> +   if (user_mode(regs) && !fixup_umip_exception(regs))
> +   return;
> +

I would do fixup_umip_exception(regs) == 0 to make it more obvious
what's going on.

Also, since you're allowing this in v8086 mode, I think this should
have an explicit test in
tools/testing/selftests/x86/entry_from_vm86.c.  I *think* it will work
fine, but the pt_regs handling in vm86 mode is quite scary and has
been rewritten recently.
--
To unsubscribe from this list: send the line "unsubscribe linux-msdos" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[v2 6/7] x86/traps: Fixup general protection faults caused by UMIP

2016-12-23 Thread Ricardo Neri 
If the User-Mode Instruction Prevention CPU feature is available and
enabled, a general protection fault will be issued if the instructions
sgdt, sldt, sidt, str or smsw are executed from user-mode context
(CPL > 0). If the fault was caused by any of the instructions protected
by UMIP, fixup_umip_exceptino will emulate dummy results for these
instructions.

Cc: Andy Lutomirski 
Cc: Andrew Morton 
Cc: H. Peter Anvin 
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 
Cc: Tony Luck 
Cc: Paolo Bonzini 
Cc: Liang Z. Li 
Cc: Alexandre Julliard 
Cc: Stas Sergeev 
Cc: x...@kernel.org
Cc: linux-msdos@vger.kernel.org
Signed-off-by: Ricardo Neri 
---
 arch/x86/kernel/traps.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index bf0c6d0..5044fb3 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -64,6 +64,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifdef CONFIG_X86_64
 #include 
@@ -491,6 +492,9 @@ do_general_protection(struct pt_regs *regs, long error_code)
RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
cond_local_irq_enable(regs);
 
+   if (user_mode(regs) && !fixup_umip_exception(regs))
+   return;
+
if (v8086_mode(regs)) {
local_irq_enable();
handle_vm86_fault((struct kernel_vm86_regs *) regs, error_code);
-- 
2.9.3

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