Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1516840 > unrolled thread

[PATCH 0/4] x86: enable User-Mode Instruction Prevention

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4] x86: enable User-Mode Instruction Prevention Ricardo Neri <ricardo.neri-calderon@linux.intel.com> - 2016-11-08 07:20 +0100
    [PATCH 3/4] x86: Enable User-Mode Instruction Prevention Ricardo Neri <ricardo.neri-calderon@linux.intel.com> - 2016-11-08 07:20 +0100
    [PATCH 4/4] selftests/x86: Add tests for User-Mode Instruction Prevention Ricardo Neri <ricardo.neri-calderon@linux.intel.com> - 2016-11-08 07:20 +0100
    [PATCH 2/4] x86: Prepare vm86 tasks to handle User-Mode Instruction Prevention Ricardo Neri <ricardo.neri-calderon@linux.intel.com> - 2016-11-08 07:20 +0100
      Re: [PATCH 2/4] x86: Prepare vm86 tasks to handle User-Mode  Instruction Prevention Andy Lutomirski <luto@amacapital.net> - 2016-11-08 17:10 +0100
        Re: [PATCH 2/4] x86: Prepare vm86 tasks to handle User-Mode  Instruction Prevention Peter Zijlstra <peterz@infradead.org> - 2016-11-08 18:10 +0100
          Re: [PATCH 2/4] x86: Prepare vm86 tasks to handle User-Mode  Instruction Prevention Ricardo Neri <ricardo.neri-calderon@linux.intel.com> - 2016-11-09 05:30 +0100
    Re: [PATCH 0/4] x86: enable User-Mode Instruction Prevention Peter Zijlstra <peterz@infradead.org> - 2016-11-08 14:20 +0100
      Re: [PATCH 0/4] x86: enable User-Mode Instruction Prevention Andy Lutomirski <luto@amacapital.net> - 2016-11-08 16:40 +0100
        Re: [PATCH 0/4] x86: enable User-Mode Instruction Prevention Thomas Gleixner <tglx@linutronix.de> - 2016-11-08 18:00 +0100
          Re: [PATCH 0/4] x86: enable User-Mode Instruction Prevention Ricardo Neri <ricardo.neri-calderon@linux.intel.com> - 2016-11-09 05:30 +0100
        Re: [PATCH 0/4] x86: enable User-Mode Instruction Prevention Ricardo Neri <ricardo.neri-calderon@linux.intel.com> - 2016-11-09 05:40 +0100
          Re: [PATCH 0/4] x86: enable User-Mode Instruction Prevention Andy Lutomirski <luto@amacapital.net> - 2016-11-09 12:40 +0100
            Re: [PATCH 0/4] x86: enable User-Mode Instruction Prevention Ricardo Neri <ricardo.neri-calderon@linux.intel.com> - 2016-11-10 07:50 +0100
              Re: [PATCH 0/4] x86: enable User-Mode Instruction Prevention Stas Sergeev <stsp@list.ru> - 2016-11-10 10:00 +0100
                Re: [PATCH 0/4] x86: enable User-Mode Instruction Prevention Ricardo Neri <ricardo.neri-calderon@linux.intel.com> - 2016-11-11 05:20 +0100

#1516840 — [PATCH 0/4] x86: enable User-Mode Instruction Prevention

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.

There is a caveat, however. Certain applications running in virtual-8086
mode, such as DOSEMU[1] and Wine[2], want to utilize the SGDT, SIDT and
SLDT instructions for legitimate reasons. In order to keep such
applications working, UMIP must be disabled/enabled when entering/exiting
virtual-8086 mode. We also disable/enable UMIP in context switch if we
detect that there is a valid virtual-8086 state structure. However,
unconditionally disabling UMIP for virtual-8086 tasks could be exploited
by malicious applications. Hence, disabling UMIP for such kind of tasks is
allowed only if the kernel parameter 'umip=novm86' is used.

Rather than using the more modern clearcpuid=1234 format for the
kernel parameters, we use umip={no|novm86}. This is because the former does
cannot cover the three configuration states of UMIP.

The virtual-8086 mode selftests are updated to ensure that the
aforementioned instructions can be executed without issue in such mode.

Thanks and BR,
Ricardo

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Chen Yucong <slaoub@gmail.com>
Cc: Chris Metcalf <cmetcalf@mellanox.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Huang Rui <ray.huang@amd.com>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi V. Shankar <ravi.v.shankar@intel.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Shuah Khan <shuah@kernel.org>


[1]. http://www.dosemu.org/
[2]. https://wiki.winehq.org/Main_Page

Ricardo Neri (4):
  x86/cpufeature: Add User-Mode Instruction Prevention definitions
  x86: Prepare vm86 tasks to handle User-Mode Instruction Prevention
  x86: Enable User-Mode Instruction Prevention
  selftests/x86: Add tests for User-Mode Instruction Prevention

 Documentation/kernel-parameters.txt           |  5 +++
 arch/x86/Kconfig                              | 10 ++++++
 arch/x86/include/asm/cpufeatures.h            |  1 +
 arch/x86/include/asm/disabled-features.h      |  8 ++++-
 arch/x86/include/asm/vm86.h                   |  3 ++
 arch/x86/include/uapi/asm/processor-flags.h   |  2 ++
 arch/x86/kernel/cpu/common.c                  | 50 ++++++++++++++++++++++++++-
 arch/x86/kernel/process.c                     | 10 ++++++
 arch/x86/kernel/vm86_32.c                     | 20 +++++++++++
 tools/testing/selftests/x86/entry_from_vm86.c | 10 +++++-
 10 files changed, 116 insertions(+), 3 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1516841 — [PATCH 3/4] x86: Enable User-Mode Instruction Prevention

User_mode Instruction Prevention (UMIP) is enabled by setting/clearing a
bit in %cr4.

It make sense to enable UMIP at some point while booting, before user
spaces come up. Like SMAP and SMEP, is not critical to have it enabled
very early during boot. This is because UMIP is relevant only when there is
a userspace to be protected from. Given the similarities in relevance, it
makes sense to enable UMIP along with SMAP and SMEP.

Also, a __setup function is added to configure the enablement of UMIP with
kernel parameters. UMIP can be disabled completely or only for virtual-8086
tasks. We may want to disable UMIP for virtual-8086 tasks as there are
legitimate applications that utilize instructions disallowed by UMIP.
However, unconditionally disabling UMIP for virtual-8086 could be exploited
by malicious applications. Hence, we let the system owner to allow virtual-
8086 tasks to disable UMIP via a kernel parameter.

Rather than using the clearcpuid=1234 format for our kernel parameters,
this implementations relies on the format umip={no|novm86}. The intention
is to either disable UMIP completely or only for virtual-8086 tasks. The
format clearcpuid=1234 cannot take care of disabling UMIP for virtual-8086
tasks.

UMIP is enabled for all tasks by default; including virtual-8086 tasks.

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Chen Yucong <slaoub@gmail.com>
Cc: Chris Metcalf <cmetcalf@mellanox.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Huang Rui <ray.huang@amd.com>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi V. Shankar <ravi.v.shankar@intel.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
 Documentation/kernel-parameters.txt |  5 ++++
 arch/x86/Kconfig                    | 10 ++++++++
 arch/x86/kernel/cpu/common.c        | 50 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index fd5c052..95d0917 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -4201,6 +4201,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			Note that genuine overcurrent events won't be
 			reported either.
 
+	umip=		[X86] Configure User-Mode Instruction Prevention
+			no = Disable UMIP even if it is supported by processor
+			novm86 = Disable UMIP only for virtual-8086 tasks; UMIP
+				 remains active for all other tasks.
+
 	unknown_nmi_panic
 			[X86] Cause panic on unknown NMI.
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 770fb5f..dad93f9 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1718,6 +1718,16 @@ config X86_SMAP
 
 	  If unsure, say Y.
 
+config X86_INTEL_UMIP
+	def_bool y
+	depends on CPU_SUP_INTEL
+	prompt "User Mode Instruction Prevention" if EXPERT
+	---help---
+	  The User Mode Instruction Prevention (UMIP) is a security
+	  feature in newer Intel processors. If enabled, a general
+	  protection fault is issued if the instructions SGDT, SLDT,
+	  SIDT, SMSW and STR are executed in user mode.
+
 config X86_INTEL_MPX
 	prompt "Intel MPX (Memory Protection Extensions)"
 	def_bool n
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index f3e7ab2..8f0e86c 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -44,6 +44,7 @@
 #include <asm/pat.h>
 #include <asm/microcode.h>
 #include <asm/microcode_intel.h>
+#include <asm/vm86.h>
 
 #ifdef CONFIG_X86_LOCAL_APIC
 #include <asm/uv/uv.h>
@@ -306,6 +307,52 @@ static __always_inline void setup_smap(struct cpuinfo_x86 *c)
 	}
 }
 
+static __init int setup_config_umip(char *arg)
+{
+	char info[] = "x86/umip: Intel User-Mode Execution Prevention (UMIP) disabled";
+	char error[] = "x86/umip: invalid kernel parameter. Valid parameters: umip=no";
+
+	/* do not emit a message if the feature is not present */
+	if (!boot_cpu_has(X86_FEATURE_UMIP))
+		return 1;
+
+	/* do not emit a message if the feature is not enabled */
+	if (!cpu_feature_enabled(X86_FEATURE_UMIP))
+		return 1;
+
+	if (parse_option_str(arg, "no")) {
+		setup_clear_cpu_cap(X86_FEATURE_UMIP);
+		pr_info("%s\n", info);
+		return 1;
+	}
+
+	if (IS_ENABLED(CONFIG_VM86) && parse_option_str(arg, "novm86")) {
+		vm86_disable_x86_umip();
+		pr_info("%s for vm86\n", info);
+		return 1;
+	}
+
+	if (IS_ENABLED(CONFIG_VM86))
+		pr_warn("%s, umip=novm86\n", error);
+	else
+		pr_warn("%s\n", error);
+	return 0;
+}
+__setup("umip=", setup_config_umip);
+
+static __always_inline void setup_umip(struct cpuinfo_x86 *c)
+{
+	if (cpu_feature_enabled(X86_FEATURE_UMIP) &&
+	    cpu_has(c, X86_FEATURE_UMIP))
+		cr4_set_bits(X86_CR4_UMIP);
+	else
+		/*
+		 * Make sure UMIP is disabled in case it was enabled in a
+		 * previous boot (e.g., via kexec).
+		 */
+		cr4_clear_bits(X86_CR4_UMIP);
+}
+
 /*
  * Protection Keys are not available in 32-bit mode.
  */
@@ -1037,9 +1084,10 @@ static void identify_cpu(struct cpuinfo_x86 *c)
 	/* 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.
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1516844 — [PATCH 4/4] selftests/x86: Add tests for User-Mode Instruction Prevention

Certain user space programs that run on virtual-8086 mode may utilize
instructions protected by the User-Mode Instruction Prevention (UMIP)
security feature present in new Intel processors: SGDT, SIDT and SMSW. In
such a case, a general protection exception is issued if UMIP is enabled.

For the aforementioned reason, UMIP can be optionally disabled for virtual-
8086 tasks with the umip=novm86 kernel parameter. The purpose of this new
test is to verify whether the impacted instructions can be executed without
causing such #GP. If no #GP exceptions occur, we expect to exit virtual-
8086 mode from INT 0x80; provided umip=novm86 was used in the kernel
parameter.

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Chen Yucong <slaoub@gmail.com>
Cc: Chris Metcalf <cmetcalf@mellanox.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Huang Rui <ray.huang@amd.com>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi V. Shankar <ravi.v.shankar@intel.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
 tools/testing/selftests/x86/entry_from_vm86.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/x86/entry_from_vm86.c b/tools/testing/selftests/x86/entry_from_vm86.c
index d075ea0..6efa6eb 100644
--- a/tools/testing/selftests/x86/entry_from_vm86.c
+++ b/tools/testing/selftests/x86/entry_from_vm86.c
@@ -95,6 +95,11 @@ asm (
 	"int3\n\t"
 	"vmcode_int80:\n\t"
 	"int $0x80\n\t"
+	"umip:\n\t"
+	"sgdt (2052)\n\t"
+	"sidt (2052)\n\t"
+	"smsw (2052)\n\t"
+	"int $0x80\n\t"
 	".size vmcode, . - vmcode\n\t"
 	"end_vmcode:\n\t"
 	".code32\n\t"
@@ -103,7 +108,7 @@ asm (
 
 extern unsigned char vmcode[], end_vmcode[];
 extern unsigned char vmcode_bound[], vmcode_sysenter[], vmcode_syscall[],
-	vmcode_sti[], vmcode_int3[], vmcode_int80[];
+	vmcode_sti[], vmcode_int3[], vmcode_int80[], umip[];
 
 /* Returns false if the test was skipped. */
 static bool do_test(struct vm86plus_struct *v86, unsigned long eip,
@@ -218,6 +223,9 @@ int main(void)
 	v86.regs.eax = (unsigned int)-1;
 	do_test(&v86, vmcode_int80 - vmcode, VM86_INTx, 0x80, "int80");
 
+	/* UMIP -- should exit with INTx 0x80 unless UMIP was not disabled */
+	do_test(&v86, umip - vmcode, VM86_INTx, 0x80, "UMIP tests");
+
 	/* Execute a null pointer */
 	v86.regs.cs = 0;
 	v86.regs.ss = 0;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1516851 — [PATCH 2/4] x86: Prepare vm86 tasks to handle User-Mode Instruction Prevention

User-Mode Instruction Prevention (UMIP) is a security feature in new Intel
processors that causes a general protection exception if certain
instructions are executed in user mode (CPL > 0).

Unfortunately, some of the instructions that are protected by UMIP (i.e.,
SGDT, SIDT and SMSW) are used by certain applications running in virtual-
8086 mode (e.g., DOSEMU and Wine). Thus, UMIP needs to be disabled in
virtual-8086 tasks for such applications to run correctly. However,
unconditionally disabling UMIP for virtual-8086 tasks could be abused by
malicious applcations. Hence, UMIP can only be disabled for this particular
kind of tasks if requested at boot time via vm86_disable_x86_umip.

If disabling UMIP is allowed, it is done in the following two code paths:
1) entering virtual-8086 mode via a system call, and 2) task switch. When
For task-switching a new member is added to struct vm86 to keep track of
the UMIP disabling selection; set in the vm86 system call as per the the
selection made at boot time.

If supported by the CPU, UMIP is re-enabled as soon as we exit virtual-8086
mode via interrupt/exception or task switch. To determine that we switch to
a virtual-8086 mode task, we rely in the fact that virtual-8086 mode tasks
keep a copy of the value of the supervisor mode stack pointer prior to
entering in virtual-8086 mode.

Since the X86_UMIP config option is not defined yet, this code remains
dormant until such option is enabled in a subsequent patch. Such patch will
also introduce code to disable UMIP for virtual-8086 tasks via a kernel
parameter.

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Chen Yucong <slaoub@gmail.com>
Cc: Chris Metcalf <cmetcalf@mellanox.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Huang Rui <ray.huang@amd.com>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi V. Shankar <ravi.v.shankar@intel.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
 arch/x86/include/asm/vm86.h |  3 +++
 arch/x86/kernel/process.c   | 10 ++++++++++
 arch/x86/kernel/vm86_32.c   | 20 ++++++++++++++++++++
 3 files changed, 33 insertions(+)

diff --git a/arch/x86/include/asm/vm86.h b/arch/x86/include/asm/vm86.h
index 1e491f3..bd14cbc 100644
--- a/arch/x86/include/asm/vm86.h
+++ b/arch/x86/include/asm/vm86.h
@@ -40,6 +40,7 @@ struct vm86 {
 	struct revectored_struct int_revectored;
 	struct revectored_struct int21_revectored;
 	struct vm86plus_info_struct vm86plus;
+	bool disable_x86_umip;
 };
 
 #ifdef CONFIG_VM86
@@ -47,6 +48,7 @@ struct vm86 {
 void handle_vm86_fault(struct kernel_vm86_regs *, long);
 int handle_vm86_trap(struct kernel_vm86_regs *, long, int);
 void save_v86_state(struct kernel_vm86_regs *, int);
+void __init vm86_disable_x86_umip(void);
 
 struct task_struct;
 
@@ -76,6 +78,7 @@ void release_vm86_irqs(struct task_struct *);
 
 #define handle_vm86_fault(a, b)
 #define release_vm86_irqs(a)
+#define vm86_disable_x86_umip()
 
 static inline int handle_vm86_trap(struct kernel_vm86_regs *a, long b, int c)
 {
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 0888a87..32b7301 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -233,6 +233,16 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
 		 */
 		memset(tss->io_bitmap, 0xff, prev->io_bitmap_max);
 	}
+
+#if defined(CONFIG_VM86) && defined(CONFIG_X86_INTEL_UMIP)
+	if (next->vm86 && next->vm86->saved_sp0 && next->vm86->disable_x86_umip)
+		cr4_clear_bits(X86_CR4_UMIP);
+	else {
+		if (static_cpu_has(X86_FEATURE_UMIP))
+			cr4_set_bits(X86_CR4_UMIP);
+	}
+#endif
+
 	propagate_user_return_notify(prev_p, next_p);
 }
 
diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index 01f30e5..7fd22e7 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -90,6 +90,14 @@
 #define SAFE_MASK	(0xDD5)
 #define RETURN_MASK	(0xDFF)
 
+static bool disable_x86_umip;
+
+void __init vm86_disable_x86_umip(void)
+{
+	if (cpu_feature_enabled(X86_FEATURE_UMIP))
+		disable_x86_umip = true;
+}
+
 void save_v86_state(struct kernel_vm86_regs *regs, int retval)
 {
 	struct tss_struct *tss;
@@ -156,6 +164,12 @@ void save_v86_state(struct kernel_vm86_regs *regs, int retval)
 	lazy_load_gs(vm86->regs32.gs);
 
 	regs->pt.ax = retval;
+
+
+	/* Always enable UMIP if supported */
+	if (cpu_feature_enabled(X86_FEATURE_UMIP) &&
+	    static_cpu_has(X86_FEATURE_UMIP))
+		cr4_set_bits(X86_CR4_UMIP);
 }
 
 static void mark_screen_rdonly(struct mm_struct *mm)
@@ -371,6 +385,12 @@ static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus)
 	if (vm86->flags & VM86_SCREEN_BITMAP)
 		mark_screen_rdonly(tsk->mm);
 
+	if (cpu_feature_enabled(X86_FEATURE_UMIP)) {
+		vm86->disable_x86_umip = disable_x86_umip;
+		if (disable_x86_umip)
+			cr4_clear_bits(X86_CR4_UMIP);
+	}
+
 	memcpy((struct kernel_vm86_regs *)regs, &vm86regs, sizeof(vm86regs));
 	force_iret();
 	return regs->ax;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1517320 — Re: [PATCH 2/4] x86: Prepare vm86 tasks to handle User-Mode Instruction Prevention

On Mon, Nov 7, 2016 at 10:12 PM, Ricardo Neri
<ricardo.neri-calderon@linux.intel.com> wrote:
> User-Mode Instruction Prevention (UMIP) is a security feature in new Intel
> processors that causes a general protection exception if certain
> instructions are executed in user mode (CPL > 0).
>
> Unfortunately, some of the instructions that are protected by UMIP (i.e.,
> SGDT, SIDT and SMSW) are used by certain applications running in virtual-
> 8086 mode (e.g., DOSEMU and Wine). Thus, UMIP needs to be disabled in
> virtual-8086 tasks for such applications to run correctly. However,
> unconditionally disabling UMIP for virtual-8086 tasks could be abused by
> malicious applcations. Hence, UMIP can only be disabled for this particular
> kind of tasks if requested at boot time via vm86_disable_x86_umip.
>
> If disabling UMIP is allowed, it is done in the following two code paths:
> 1) entering virtual-8086 mode via a system call, and 2) task switch. When
> For task-switching a new member is added to struct vm86 to keep track of
> the UMIP disabling selection; set in the vm86 system call as per the the
> selection made at boot time.
>
> If supported by the CPU, UMIP is re-enabled as soon as we exit virtual-8086
> mode via interrupt/exception or task switch. To determine that we switch to
> a virtual-8086 mode task, we rely in the fact that virtual-8086 mode tasks
> keep a copy of the value of the supervisor mode stack pointer prior to
> entering in virtual-8086 mode.
>
> Since the X86_UMIP config option is not defined yet, this code remains
> dormant until such option is enabled in a subsequent patch. Such patch will
> also introduce code to disable UMIP for virtual-8086 tasks via a kernel
> parameter.
>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Brian Gerst <brgerst@gmail.com>
> Cc: Chen Yucong <slaoub@gmail.com>
> Cc: Chris Metcalf <cmetcalf@mellanox.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: Huang Rui <ray.huang@amd.com>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ravi V. Shankar <ravi.v.shankar@intel.com>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
>  arch/x86/include/asm/vm86.h |  3 +++
>  arch/x86/kernel/process.c   | 10 ++++++++++
>  arch/x86/kernel/vm86_32.c   | 20 ++++++++++++++++++++
>  3 files changed, 33 insertions(+)
>
> diff --git a/arch/x86/include/asm/vm86.h b/arch/x86/include/asm/vm86.h
> index 1e491f3..bd14cbc 100644
> --- a/arch/x86/include/asm/vm86.h
> +++ b/arch/x86/include/asm/vm86.h
> @@ -40,6 +40,7 @@ struct vm86 {
>         struct revectored_struct int_revectored;
>         struct revectored_struct int21_revectored;
>         struct vm86plus_info_struct vm86plus;
> +       bool disable_x86_umip;
>  };
>
>  #ifdef CONFIG_VM86
> @@ -47,6 +48,7 @@ struct vm86 {
>  void handle_vm86_fault(struct kernel_vm86_regs *, long);
>  int handle_vm86_trap(struct kernel_vm86_regs *, long, int);
>  void save_v86_state(struct kernel_vm86_regs *, int);
> +void __init vm86_disable_x86_umip(void);
>
>  struct task_struct;
>
> @@ -76,6 +78,7 @@ void release_vm86_irqs(struct task_struct *);
>
>  #define handle_vm86_fault(a, b)
>  #define release_vm86_irqs(a)
> +#define vm86_disable_x86_umip()
>
>  static inline int handle_vm86_trap(struct kernel_vm86_regs *a, long b, int c)
>  {
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 0888a87..32b7301 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -233,6 +233,16 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
>                  */
>                 memset(tss->io_bitmap, 0xff, prev->io_bitmap_max);
>         }
> +
> +#if defined(CONFIG_VM86) && defined(CONFIG_X86_INTEL_UMIP)
> +       if (next->vm86 && next->vm86->saved_sp0 && next->vm86->disable_x86_umip)
> +               cr4_clear_bits(X86_CR4_UMIP);
> +       else {
> +               if (static_cpu_has(X86_FEATURE_UMIP))
> +                       cr4_set_bits(X86_CR4_UMIP);
> +       }
> +#endif
> +

NAK.  If this code is going to exist, it needs to be deeply buried in
some unlikely if statement that already exists.  There's no good
reason to penalize all context switches to support some nonsensical
vm86 use case.

[toc] | [prev] | [next] | [standalone]

#1517395 — Re: [PATCH 2/4] x86: Prepare vm86 tasks to handle User-Mode Instruction Prevention

On Tue, Nov 08, 2016 at 08:01:39AM -0800, Andy Lutomirski wrote:
> > diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> > index 0888a87..32b7301 100644
> > --- a/arch/x86/kernel/process.c
> > +++ b/arch/x86/kernel/process.c
> > @@ -233,6 +233,16 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
> >                  */
> >                 memset(tss->io_bitmap, 0xff, prev->io_bitmap_max);
> >         }
> > +
> > +#if defined(CONFIG_VM86) && defined(CONFIG_X86_INTEL_UMIP)
> > +       if (next->vm86 && next->vm86->saved_sp0 && next->vm86->disable_x86_umip)
> > +               cr4_clear_bits(X86_CR4_UMIP);
> > +       else {
> > +               if (static_cpu_has(X86_FEATURE_UMIP))
> > +                       cr4_set_bits(X86_CR4_UMIP);
> > +       }
> > +#endif
> > +
> 
> NAK.  If this code is going to exist, it needs to be deeply buried in
> some unlikely if statement that already exists.  There's no good
> reason to penalize all context switches to support some nonsensical
> vm86 use case.

Agreed, now if instead vm86 get to emulate these instructions, this all
magically goes away..

[toc] | [prev] | [next] | [standalone]

#1517757 — Re: [PATCH 2/4] x86: Prepare vm86 tasks to handle User-Mode Instruction Prevention

On Tue, 2016-11-08 at 18:00 +0100, Peter Zijlstra wrote:
> > > +       }
> > > +#endif
> > > +
> > 
> > NAK.  If this code is going to exist, it needs to be deeply buried
> in
> > some unlikely if statement that already exists.  There's no good
> > reason to penalize all context switches to support some nonsensical
> > vm86 use case.
> 
> Agreed, now if instead vm86 get to emulate these instructions, this
> all
> magically goes away..

Yes. I agree. I will rework the series and this should not be needed.

Thanks and BR,
Ricardo

[toc] | [prev] | [next] | [standalone]

#1517157

On Mon, Nov 07, 2016 at 10:12:09PM -0800, Ricardo Neri wrote:
> There is a caveat, however. Certain applications running in virtual-8086
> mode, such as DOSEMU[1] and Wine[2], want to utilize the SGDT, SIDT and
> SLDT instructions for legitimate reasons. In order to keep such
> applications working, UMIP must be disabled/enabled when entering/exiting
> virtual-8086 mode. 

Would it not be better to emulate these instructions for them? What way
we can verify they're not malicious.

[toc] | [prev] | [next] | [standalone]

#1517293

On Tue, Nov 8, 2016 at 5:16 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Nov 07, 2016 at 10:12:09PM -0800, Ricardo Neri wrote:
>> There is a caveat, however. Certain applications running in virtual-8086
>> mode, such as DOSEMU[1] and Wine[2], want to utilize the SGDT, SIDT and
>> SLDT instructions for legitimate reasons. In order to keep such
>> applications working, UMIP must be disabled/enabled when entering/exiting
>> virtual-8086 mode.
>
> Would it not be better to emulate these instructions for them? What way
> we can verify they're not malicious.

Forget malice -- if they are really needed for some silly vm86-using
program, let's trap them and emulate them so they return dummy values.

Also, keep in mind that vm86 is already effectively gated behind a
sysctl for non-root.  I think the default should be that, if root has
enabled vm86, it should work.

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

[toc] | [prev] | [next] | [standalone]

#1517389

On Tue, 8 Nov 2016, Andy Lutomirski wrote:
> On Tue, Nov 8, 2016 at 5:16 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Mon, Nov 07, 2016 at 10:12:09PM -0800, Ricardo Neri wrote:
> >> There is a caveat, however. Certain applications running in virtual-8086
> >> mode, such as DOSEMU[1] and Wine[2], want to utilize the SGDT, SIDT and
> >> SLDT instructions for legitimate reasons. In order to keep such
> >> applications working, UMIP must be disabled/enabled when entering/exiting
> >> virtual-8086 mode.
> >
> > Would it not be better to emulate these instructions for them? What way
> > we can verify they're not malicious.
> 
> Forget malice -- if they are really needed for some silly vm86-using
> program, let's trap them and emulate them so they return dummy values.

handle_vm86_fault() already does instruction emulation, so adding the few
bits there is the right thing to do. Then we just can enable UMIP
unconditionally and be done with it.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1517758

On Tue, 2016-11-08 at 17:52 +0100, Thomas Gleixner wrote:
> On Tue, 8 Nov 2016, Andy Lutomirski wrote:
> > On Tue, Nov 8, 2016 at 5:16 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > > On Mon, Nov 07, 2016 at 10:12:09PM -0800, Ricardo Neri wrote:
> > >> There is a caveat, however. Certain applications running in virtual-8086
> > >> mode, such as DOSEMU[1] and Wine[2], want to utilize the SGDT, SIDT and
> > >> SLDT instructions for legitimate reasons. In order to keep such
> > >> applications working, UMIP must be disabled/enabled when entering/exiting
> > >> virtual-8086 mode.
> > >
> > > Would it not be better to emulate these instructions for them? What way
> > > we can verify they're not malicious.
> > 
> > Forget malice -- if they are really needed for some silly vm86-using
> > program, let's trap them and emulate them so they return dummy values.
> 
> handle_vm86_fault() already does instruction emulation, so adding the few
> bits there is the right thing to do. Then we just can enable UMIP
> unconditionally and be done with it.

Ah. I didn't think about that. It make sense to me. I will rework this
series with this approach.
> 
> Thanks,
> 
> 	tglx

[toc] | [prev] | [next] | [standalone]

#1517766

On Tue, 2016-11-08 at 07:34 -0800, Andy Lutomirski wrote:
> > Would it not be better to emulate these instructions for them? What
> way
> > we can verify they're not malicious.
> 
> Forget malice -- if they are really needed for some silly vm86-using
> program, let's trap them and emulate them so they return dummy values.
> 
> Also, keep in mind that vm86 is already effectively gated behind a
> sysctl for non-root.  I think the default should be that, if root has
> enabled vm86, it should work.

Then should I keep UMIP enabled by default and still provide an option
to disable it via a kernel parameter?

Also, a third option, umip=novm86 would "disable" UMIP in vm86 tasks.
Under the new approach (of emulating the impacted instructions), this
option, a #GP fault would still be generated but the actual values of
GDT/LDT/IDT/MSW would be passed to user space. Does this make sense?

Thanks and BR,
Ricardo

[toc] | [prev] | [next] | [standalone]

#1518058

On Tue, Nov 8, 2016 at 8:31 PM, Ricardo Neri
<ricardo.neri-calderon@linux.intel.com> wrote:
> On Tue, 2016-11-08 at 07:34 -0800, Andy Lutomirski wrote:
>> > Would it not be better to emulate these instructions for them? What
>> way
>> > we can verify they're not malicious.
>>
>> Forget malice -- if they are really needed for some silly vm86-using
>> program, let's trap them and emulate them so they return dummy values.
>>
>> Also, keep in mind that vm86 is already effectively gated behind a
>> sysctl for non-root.  I think the default should be that, if root has
>> enabled vm86, it should work.
>
> Then should I keep UMIP enabled by default and still provide an option
> to disable it via a kernel parameter?

Probably, but clearcpuid might be good enough.  There might be some
unexpected breakage.

>
> Also, a third option, umip=novm86 would "disable" UMIP in vm86 tasks.
> Under the new approach (of emulating the impacted instructions), this
> option, a #GP fault would still be generated but the actual values of
> GDT/LDT/IDT/MSW would be passed to user space. Does this make sense?

I don't think so.  As far as I know, there is no legitimate reason for
a vm86-using program to care about what these instructions spit out.
Heck, in real mode and vm86 mode, there aren't segment descriptors at
all, so the GDT is really quite useless even if it were readable.

I would suggest having all of these instructions return compile-time
constants in vm86 mode.

[toc] | [prev] | [next] | [standalone]

#1518713

On Wed, 2016-11-09 at 03:05 -0800, Andy Lutomirski wrote:
> On Tue, Nov 8, 2016 at 8:31 PM, Ricardo Neri
> <ricardo.neri-calderon@linux.intel.com> wrote:
> > On Tue, 2016-11-08 at 07:34 -0800, Andy Lutomirski wrote:
> >> > Would it not be better to emulate these instructions for them? What
> >> way
> >> > we can verify they're not malicious.
> >>
> >> Forget malice -- if they are really needed for some silly vm86-using
> >> program, let's trap them and emulate them so they return dummy values.
> >>
> >> Also, keep in mind that vm86 is already effectively gated behind a
> >> sysctl for non-root.  I think the default should be that, if root has
> >> enabled vm86, it should work.
> >
> > Then should I keep UMIP enabled by default and still provide an option
> > to disable it via a kernel parameter?
> 
> Probably, but clearcpuid might be good enough.  There might be some
> unexpected breakage.
> 
> >
> > Also, a third option, umip=novm86 would "disable" UMIP in vm86 tasks.
> > Under the new approach (of emulating the impacted instructions), this
> > option, a #GP fault would still be generated but the actual values of
> > GDT/LDT/IDT/MSW would be passed to user space. Does this make sense?
> 
> I don't think so.  As far as I know, there is no legitimate reason for
> a vm86-using program to care about what these instructions spit out.
> Heck, in real mode and vm86 mode, there aren't segment descriptors at
> all, so the GDT is really quite useless even if it were readable.

I took a closer look at the dosemu code. It appears that it does not
purposely utilize SGDT to obtain the descriptor table while in vm86. It
does use SGDT (in protected mode) to emulate certain functionality such
as the Virtual xxx Driver. In such a case, UMIP needs to be disabled.
However, this code seems to be disabled [1]. dosemu includes an i386
emulator that in some cases uses the actual instructions of the host
system. In such cases, UMIP might be needed to be disabled.

So, yes, I agree now that UMIP does not need to be disabled specifically
for vm86 tasks but via clearcpuid.

Thanks and BR,
Ricardo
[1].
https://sourceforge.net/p/dosemu/code/ci/master/tree/src/dosext/dpmi/vxd.c#l731 
> 
> I would suggest having all of these instructions return compile-time
> constants in vm86 mode.

[toc] | [prev] | [next] | [standalone]

#1518750

Hi!

I don't know the context of that discussion, so I'll only
comment on the dosemu part.

10.11.2016 09:46, Ricardo Neri пишет:
> I took a closer look at the dosemu code. It appears that it does not
> purposely utilize SGDT to obtain the descriptor table while in vm86. It
> does use SGDT (in protected mode) to emulate certain functionality such
> as the Virtual xxx Driver. In such a case, UMIP needs to be disabled.
> However, this code seems to be disabled [1].
Indeed.
The code you've found, was copied from wine, because
dosemu supports windows-3.1. But sgdt is in win32s part
that is disabled in dosemu. It is however enabled in wine, or
at least it was when I ported the VxD code from there. So you
may want to ask wine devs if they still use sgdt and vm86.
In dosemu, if we ever enable win32s support, we won't rely
on sgdt. In fact, when some prot mode program under dosemu
uses GDT selectors, in a fault handler we replace them with
LDT selectors.

>   dosemu includes an i386
> emulator that in some cases uses the actual instructions of the host
> system.
In dosemu2 code, the places you've found, now contain this:
error("SGDT not implemented\n");
If we ever support SGDT, we'll use some emulation/fake values.

So overall, dosemu is not going to willingly use sgdt in any
near future. But the programs running under vm86 or in prot mode
may do so. This is very uncommon though, especially under dosemu,
because it supports only a "polite" programs - those that work
under win95's dos prompt. No one would get sufficiently hurt if
sgdt under vm86 will somehow change from its current behaviour.

You can ask wine people for their sgdt use in win32s subsystem.

[toc] | [prev] | [next] | [standalone]

#1519554

On Thu, 2016-11-10 at 11:52 +0300, Stas Sergeev wrote:
> Hi!
> 
> I don't know the context of that discussion, so I'll only
> comment on the dosemu part.

I'm sorry! I will cc you and the linux-msdos list in my v2.
> 
> 10.11.2016 09:46, Ricardo Neri пишет:
> > I took a closer look at the dosemu code. It appears that it does not
> > purposely utilize SGDT to obtain the descriptor table while in vm86. It
> > does use SGDT (in protected mode) to emulate certain functionality such
> > as the Virtual xxx Driver. In such a case, UMIP needs to be disabled.
> > However, this code seems to be disabled [1].
> Indeed.
> The code you've found, was copied from wine, because
> dosemu supports windows-3.1. But sgdt is in win32s part
> that is disabled in dosemu. It is however enabled in wine, or
> at least it was when I ported the VxD code from there. So you
> may want to ask wine devs if they still use sgdt and vm86.
> In dosemu, if we ever enable win32s support, we won't rely
> on sgdt. In fact, when some prot mode program under dosemu
> uses GDT selectors, in a fault handler we replace them with
> LDT selectors.

Actually, the SLDT instruction is also impacted by this feature. This
feature, will cause a GP fault if the instructions SGDT, SLDT, SIDT,
SMSW or STR are executed with CPL > 0. Would this be a problem for
dosemu? The proposal now is to trap this GPU fault and give fake value
for these tables.
> 
> >   dosemu includes an i386
> > emulator that in some cases uses the actual instructions of the host
> > system.
> In dosemu2 code, the places you've found, now contain this:
> error("SGDT not implemented\n");
> If we ever support SGDT, we'll use some emulation/fake values.
> 
> So overall, dosemu is not going to willingly use sgdt in any
> near future. But the programs running under vm86 or in prot mode
> may do so. This is very uncommon though, especially under dosemu,
> because it supports only a "polite" programs - those that work
> under win95's dos prompt. No one would get sufficiently hurt if
> sgdt under vm86 will somehow change from its current behaviour.

This is good news. This means that we could go ahead and give a fake
pointer to the GDT and the other impacted tables?
> 
> You can ask wine people for their sgdt use in win32s subsystem.

Will do.

Thanks and BR,
Ricardo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web