[PATCH v4 0/3] Xen iopl fixes

[PATCH v4 0/3] Xen iopl fixes

[Andy Lutomirski]

Hi all-

For those who are seeing this for the first time: any 64-bit Xen PV
domain with IO port access privileges (in practice, this means dom0
AFAIK) and any user programs that use iopl(3) (various old X
drivers, presumably) is probably vulnerable to privilege escalations
by unprivileged programs running in the same PV domain.

There's a long public description of the issue here:

http://xenbits.xen.org/xsa/advisory-171.html

Changes from v3:
 - Add Jan's R-b.
 - No longer embargoed

Changes from v2: Pretend v2 never happened...

Changes from v1: Use xen/hypervisor.h instead of xen-ops.h (Jan)

Andy Lutomirski (3):
  selftests/x86: Add a iopl test
  x86/iopl/64: Properly context-switch IOPL on Xen PV
  x86/iopl: Fix iopl capability check on Xen PV

 arch/x86/include/asm/xen/hypervisor.h |   2 +
 arch/x86/kernel/ioport.c              |  12 ++-
 arch/x86/kernel/process_64.c          |  12 +++
 arch/x86/xen/enlighten.c              |   2 +-
 tools/testing/selftests/x86/Makefile  |   2 +-
 tools/testing/selftests/x86/iopl.c    | 135 ++++++++++++++++++++++++++++++++++
 6 files changed, 160 insertions(+), 5 deletions(-)
 create mode 100644 tools/testing/selftests/x86/iopl.c

-- 
2.5.0

[PATCH v4 1/3] selftests/x86: Add a iopl test

[Andy Lutomirski]

This exercises two cases that are known to be buggy on Xen PV right
now.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 tools/testing/selftests/x86/Makefile |   2 +-
 tools/testing/selftests/x86/iopl.c   | 135 +++++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/x86/iopl.c

diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index d0c473f65850..506e5dea179e 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -4,7 +4,7 @@ include ../lib.mk
 
 .PHONY: all all_32 all_64 warn_32bit_failure clean
 
-TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt ptrace_syscall
+TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt ptrace_syscall iopl
 TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault sigreturn test_syscall_vdso unwind_vdso \
 			test_FCMOV test_FCOMI test_FISTTP \
 			ldt_gdt \
diff --git a/tools/testing/selftests/x86/iopl.c b/tools/testing/selftests/x86/iopl.c
new file mode 100644
index 000000000000..c496ca97bc18
--- /dev/null
+++ b/tools/testing/selftests/x86/iopl.c
@@ -0,0 +1,135 @@
+/*
+ * iopl.c - Test case for a Linux on Xen 64-bit bug
+ * Copyright (c) 2015 Andrew Lutomirski
+ */
+
+#define _GNU_SOURCE
+#include <err.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <signal.h>
+#include <setjmp.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <stdbool.h>
+#include <sched.h>
+#include <sys/io.h>
+
+static int nerrs = 0;
+
+static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
+		       int flags)
+{
+	struct sigaction sa;
+	memset(&sa, 0, sizeof(sa));
+	sa.sa_sigaction = handler;
+	sa.sa_flags = SA_SIGINFO | flags;
+	sigemptyset(&sa.sa_mask);
+	if (sigaction(sig, &sa, 0))
+		err(1, "sigaction");
+
+}
+
+static jmp_buf jmpbuf;
+
+static void sigsegv(int sig, siginfo_t *si, void *ctx_void)
+{
+	siglongjmp(jmpbuf, 1);
+}
+
+int main(void)
+{
+	cpu_set_t cpuset;
+	CPU_ZERO(&cpuset);
+	CPU_SET(0, &cpuset);
+	if (sched_setaffinity(0, sizeof(cpuset), &cpuset) != 0)
+		err(1, "sched_setaffinity to CPU 0");
+
+	/* Probe for iopl support.  Note that iopl(0) works even as nonroot. */
+	if (iopl(3) != 0) {
+		printf("[OK]\tiopl(3) failed (%d) -- try running as root\n",
+		       errno);
+		return 0;
+	}
+
+	/* Restore our original state prior to starting the test. */
+	if (iopl(0) != 0)
+		err(1, "iopl(0)");
+
+	pid_t child = fork();
+	if (child == -1)
+		err(1, "fork");
+
+	if (child == 0) {
+		printf("\tchild: set IOPL to 3\n");
+		if (iopl(3) != 0)
+			err(1, "iopl");
+
+		printf("[RUN]\tchild: write to 0x80\n");
+		asm volatile ("outb %%al, $0x80" : : "a" (0));
+
+		return 0;
+	} else {
+		int status;
+		if (waitpid(child, &status, 0) != child ||
+		    !WIFEXITED(status)) {
+			printf("[FAIL]\tChild died\n");
+			nerrs++;
+		} else if (WEXITSTATUS(status) != 0) {
+			printf("[FAIL]\tChild failed\n");
+			nerrs++;
+		} else {
+			printf("[OK]\tChild succeeded\n");
+		}
+	}
+
+	printf("[RUN]\tparent: write to 0x80 (should fail)\n");
+
+	sethandler(SIGSEGV, sigsegv, 0);
+	if (sigsetjmp(jmpbuf, 1) != 0) {
+		printf("[OK]\twrite was denied\n");
+	} else {
+		asm volatile ("outb %%al, $0x80" : : "a" (0));
+		printf("[FAIL]\twrite was allowed\n");
+		nerrs++;
+	}
+
+	/* Test the capability checks. */
+	printf("\tiopl(3)\n");
+	if (iopl(3) != 0)
+		err(1, "iopl(3)");
+
+	printf("\tDrop privileges\n");
+	if (setresuid(1, 1, 1) != 0) {
+		printf("[WARN]\tDropping privileges failed\n");
+		goto done;
+	}
+
+	printf("[RUN]\tiopl(3) unprivileged but with IOPL==3\n");
+	if (iopl(3) != 0) {
+		printf("[FAIL]\tiopl(3) should work if iopl is already 3 even if unprivileged\n");
+		nerrs++;
+	}
+
+	printf("[RUN]\tiopl(0) unprivileged\n");
+	if (iopl(0) != 0) {
+		printf("[FAIL]\tiopl(0) should work if iopl is already 3 even if unprivileged\n");
+		nerrs++;
+	}
+
+	printf("[RUN]\tiopl(3) unprivileged\n");
+	if (iopl(3) == 0) {
+		printf("[FAIL]\tiopl(3) should fail if when unprivileged if iopl==0\n");
+		nerrs++;
+	} else {
+		printf("[OK]\tFailed as expected\n");
+	}
+
+done:
+	return nerrs ? 1 : 0;
+}
+
-- 
2.5.0

[PATCH v4 2/3] x86/iopl/64: Properly context-switch IOPL on Xen PV

[Andy Lutomirski]

On Xen PV, regs->flags doesn't reliably reflect IOPL and the
exit-to-userspace code doesn't change IOPL.  We need to context
switch it manually.

I'm doing this without going through paravirt because this is
specific to Xen PV.  After the dust settles, we can merge this with
the 32-bit code, tidy up the iopl syscall implementation, and remove
the set_iopl pvop entirely.

Fixes XSA-171.

Cc: stable@vger.kernel.org
Reviewewd-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/xen/hypervisor.h |  2 ++
 arch/x86/kernel/process_64.c          | 12 ++++++++++++
 arch/x86/xen/enlighten.c              |  2 +-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
index 8b2d4bea9962..39171b3646bb 100644
--- a/arch/x86/include/asm/xen/hypervisor.h
+++ b/arch/x86/include/asm/xen/hypervisor.h
@@ -62,4 +62,6 @@ void xen_arch_register_cpu(int num);
 void xen_arch_unregister_cpu(int num);
 #endif
 
+extern void xen_set_iopl_mask(unsigned mask);
+
 #endif /* _ASM_X86_XEN_HYPERVISOR_H */
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index b9d99e0f82c4..9f751876066f 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -48,6 +48,7 @@
 #include <asm/syscalls.h>
 #include <asm/debugreg.h>
 #include <asm/switch_to.h>
+#include <asm/xen/hypervisor.h>
 
 asmlinkage extern void ret_from_fork(void);
 
@@ -411,6 +412,17 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
 		     task_thread_info(prev_p)->flags & _TIF_WORK_CTXSW_PREV))
 		__switch_to_xtra(prev_p, next_p, tss);
 
+#ifdef CONFIG_XEN
+	/*
+	 * On Xen PV, IOPL bits in pt_regs->flags have no effect, and
+	 * current_pt_regs()->flags may not match the current task's
+	 * intended IOPL.  We need to switch it manually.
+	 */
+	if (unlikely(static_cpu_has(X86_FEATURE_XENPV) &&
+		     prev->iopl != next->iopl))
+		xen_set_iopl_mask(next->iopl);
+#endif
+
 	if (static_cpu_has_bug(X86_BUG_SYSRET_SS_ATTRS)) {
 		/*
 		 * AMD CPUs have a misfeature: SYSRET sets the SS selector but
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index d09e4c9d7cc5..e3679db17545 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -961,7 +961,7 @@ static void xen_load_sp0(struct tss_struct *tss,
 	tss->x86_tss.sp0 = thread->sp0;
 }
 
-static void xen_set_iopl_mask(unsigned mask)
+void xen_set_iopl_mask(unsigned mask)
 {
 	struct physdev_set_iopl set_iopl;
 
-- 
2.5.0

[PATCH v4 3/3] x86/iopl: Fix iopl capability check on Xen PV

[Andy Lutomirski]

iopl(3) is supposed to work if iopl is already 3, even if
unprivileged.  This didn't work right on Xen PV.  Fix it.

Cc: stable@vger.kernel.org
Reviewewd-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/kernel/ioport.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index 37dae792dbbe..589b3193f102 100644
--- a/arch/x86/kernel/ioport.c
+++ b/arch/x86/kernel/ioport.c
@@ -96,9 +96,14 @@ asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int turn_on)
 SYSCALL_DEFINE1(iopl, unsigned int, level)
 {
 	struct pt_regs *regs = current_pt_regs();
-	unsigned int old = (regs->flags >> 12) & 3;
 	struct thread_struct *t = &current->thread;
 
+	/*
+	 * Careful: the IOPL bits in regs->flags are undefined under Xen PV
+	 * and changing them has no effect.
+	 */
+	unsigned int old = t->iopl >> X86_EFLAGS_IOPL_BIT;
+
 	if (level > 3)
 		return -EINVAL;
 	/* Trying to gain more privileges? */
@@ -106,8 +111,9 @@ SYSCALL_DEFINE1(iopl, unsigned int, level)
 		if (!capable(CAP_SYS_RAWIO))
 			return -EPERM;
 	}
-	regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) | (level << 12);
-	t->iopl = level << 12;
+	regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) |
+		(level << X86_EFLAGS_IOPL_BIT);
+	t->iopl = level << X86_EFLAGS_IOPL_BIT;
 	set_iopl_mask(t->iopl);
 
 	return 0;
-- 
2.5.0

[tip:x86/urgent] selftests/x86: Add an iopl test

[tip-bot for Andy Lutomirski]

Commit-ID:  b08983015cdddca7e41c95f5054e2a8fb222a264
Gitweb:     http://git.kernel.org/tip/b08983015cdddca7e41c95f5054e2a8fb222a264
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Wed, 16 Mar 2016 14:14:20 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 17 Mar 2016 09:49:26 +0100

selftests/x86: Add an iopl test

This exercises two cases that are known to be buggy on Xen PV right
now.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/61afe904c95c92abb29cd075b51e10e7feb0f774.1458162709.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/testing/selftests/x86/Makefile |   2 +-
 tools/testing/selftests/x86/iopl.c   | 135 +++++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index d5ce7d7..b47ebd1 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -5,7 +5,7 @@ include ../lib.mk
 .PHONY: all all_32 all_64 warn_32bit_failure clean
 
 TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt ptrace_syscall \
-			check_initial_reg_state sigreturn ldt_gdt
+			check_initial_reg_state sigreturn ldt_gdt iopl
 TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault test_syscall_vdso unwind_vdso \
 			test_FCMOV test_FCOMI test_FISTTP \
 			vdso_restorer
diff --git a/tools/testing/selftests/x86/iopl.c b/tools/testing/selftests/x86/iopl.c
new file mode 100644
index 0000000..c496ca9
--- /dev/null
+++ b/tools/testing/selftests/x86/iopl.c
@@ -0,0 +1,135 @@
+/*
+ * iopl.c - Test case for a Linux on Xen 64-bit bug
+ * Copyright (c) 2015 Andrew Lutomirski
+ */
+
+#define _GNU_SOURCE
+#include <err.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <signal.h>
+#include <setjmp.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <stdbool.h>
+#include <sched.h>
+#include <sys/io.h>
+
+static int nerrs = 0;
+
+static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
+		       int flags)
+{
+	struct sigaction sa;
+	memset(&sa, 0, sizeof(sa));
+	sa.sa_sigaction = handler;
+	sa.sa_flags = SA_SIGINFO | flags;
+	sigemptyset(&sa.sa_mask);
+	if (sigaction(sig, &sa, 0))
+		err(1, "sigaction");
+
+}
+
+static jmp_buf jmpbuf;
+
+static void sigsegv(int sig, siginfo_t *si, void *ctx_void)
+{
+	siglongjmp(jmpbuf, 1);
+}
+
+int main(void)
+{
+	cpu_set_t cpuset;
+	CPU_ZERO(&cpuset);
+	CPU_SET(0, &cpuset);
+	if (sched_setaffinity(0, sizeof(cpuset), &cpuset) != 0)
+		err(1, "sched_setaffinity to CPU 0");
+
+	/* Probe for iopl support.  Note that iopl(0) works even as nonroot. */
+	if (iopl(3) != 0) {
+		printf("[OK]\tiopl(3) failed (%d) -- try running as root\n",
+		       errno);
+		return 0;
+	}
+
+	/* Restore our original state prior to starting the test. */
+	if (iopl(0) != 0)
+		err(1, "iopl(0)");
+
+	pid_t child = fork();
+	if (child == -1)
+		err(1, "fork");
+
+	if (child == 0) {
+		printf("\tchild: set IOPL to 3\n");
+		if (iopl(3) != 0)
+			err(1, "iopl");
+
+		printf("[RUN]\tchild: write to 0x80\n");
+		asm volatile ("outb %%al, $0x80" : : "a" (0));
+
+		return 0;
+	} else {
+		int status;
+		if (waitpid(child, &status, 0) != child ||
+		    !WIFEXITED(status)) {
+			printf("[FAIL]\tChild died\n");
+			nerrs++;
+		} else if (WEXITSTATUS(status) != 0) {
+			printf("[FAIL]\tChild failed\n");
+			nerrs++;
+		} else {
+			printf("[OK]\tChild succeeded\n");
+		}
+	}
+
+	printf("[RUN]\tparent: write to 0x80 (should fail)\n");
+
+	sethandler(SIGSEGV, sigsegv, 0);
+	if (sigsetjmp(jmpbuf, 1) != 0) {
+		printf("[OK]\twrite was denied\n");
+	} else {
+		asm volatile ("outb %%al, $0x80" : : "a" (0));
+		printf("[FAIL]\twrite was allowed\n");
+		nerrs++;
+	}
+
+	/* Test the capability checks. */
+	printf("\tiopl(3)\n");
+	if (iopl(3) != 0)
+		err(1, "iopl(3)");
+
+	printf("\tDrop privileges\n");
+	if (setresuid(1, 1, 1) != 0) {
+		printf("[WARN]\tDropping privileges failed\n");
+		goto done;
+	}
+
+	printf("[RUN]\tiopl(3) unprivileged but with IOPL==3\n");
+	if (iopl(3) != 0) {
+		printf("[FAIL]\tiopl(3) should work if iopl is already 3 even if unprivileged\n");
+		nerrs++;
+	}
+
+	printf("[RUN]\tiopl(0) unprivileged\n");
+	if (iopl(0) != 0) {
+		printf("[FAIL]\tiopl(0) should work if iopl is already 3 even if unprivileged\n");
+		nerrs++;
+	}
+
+	printf("[RUN]\tiopl(3) unprivileged\n");
+	if (iopl(3) == 0) {
+		printf("[FAIL]\tiopl(3) should fail if when unprivileged if iopl==0\n");
+		nerrs++;
+	} else {
+		printf("[OK]\tFailed as expected\n");
+	}
+
+done:
+	return nerrs ? 1 : 0;
+}
+

[tip:x86/urgent] x86/iopl/64: Properly context-switch IOPL on Xen PV

[tip-bot for Andy Lutomirski]

Commit-ID:  b7a584598aea7ca73140cb87b40319944dd3393f
Gitweb:     http://git.kernel.org/tip/b7a584598aea7ca73140cb87b40319944dd3393f
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Wed, 16 Mar 2016 14:14:21 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 17 Mar 2016 09:49:26 +0100

x86/iopl/64: Properly context-switch IOPL on Xen PV

On Xen PV, regs->flags doesn't reliably reflect IOPL and the
exit-to-userspace code doesn't change IOPL.  We need to context
switch it manually.

I'm doing this without going through paravirt because this is
specific to Xen PV.  After the dust settles, we can merge this with
the 32-bit code, tidy up the iopl syscall implementation, and remove
the set_iopl pvop entirely.

Fixes XSA-171.

Reviewewd-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/693c3bd7aeb4d3c27c92c622b7d0f554a458173c.1458162709.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/xen/hypervisor.h |  2 ++
 arch/x86/kernel/process_64.c          | 12 ++++++++++++
 arch/x86/xen/enlighten.c              |  2 +-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
index 8b2d4be..39171b3 100644
--- a/arch/x86/include/asm/xen/hypervisor.h
+++ b/arch/x86/include/asm/xen/hypervisor.h
@@ -62,4 +62,6 @@ void xen_arch_register_cpu(int num);
 void xen_arch_unregister_cpu(int num);
 #endif
 
+extern void xen_set_iopl_mask(unsigned mask);
+
 #endif /* _ASM_X86_XEN_HYPERVISOR_H */
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index b9d99e0..9f75187 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -48,6 +48,7 @@
 #include <asm/syscalls.h>
 #include <asm/debugreg.h>
 #include <asm/switch_to.h>
+#include <asm/xen/hypervisor.h>
 
 asmlinkage extern void ret_from_fork(void);
 
@@ -411,6 +412,17 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
 		     task_thread_info(prev_p)->flags & _TIF_WORK_CTXSW_PREV))
 		__switch_to_xtra(prev_p, next_p, tss);
 
+#ifdef CONFIG_XEN
+	/*
+	 * On Xen PV, IOPL bits in pt_regs->flags have no effect, and
+	 * current_pt_regs()->flags may not match the current task's
+	 * intended IOPL.  We need to switch it manually.
+	 */
+	if (unlikely(static_cpu_has(X86_FEATURE_XENPV) &&
+		     prev->iopl != next->iopl))
+		xen_set_iopl_mask(next->iopl);
+#endif
+
 	if (static_cpu_has_bug(X86_BUG_SYSRET_SS_ATTRS)) {
 		/*
 		 * AMD CPUs have a misfeature: SYSRET sets the SS selector but
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 2c26108..8381fb9 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -961,7 +961,7 @@ static void xen_load_sp0(struct tss_struct *tss,
 	tss->x86_tss.sp0 = thread->sp0;
 }
 
-static void xen_set_iopl_mask(unsigned mask)
+void xen_set_iopl_mask(unsigned mask)
 {
 	struct physdev_set_iopl set_iopl;
 

[tip:x86/urgent] x86/iopl: Fix iopl capability check on Xen PV

[tip-bot for Andy Lutomirski]

Commit-ID:  c29016cf41fe9fa994a5ecca607cf5f1cd98801e
Gitweb:     http://git.kernel.org/tip/c29016cf41fe9fa994a5ecca607cf5f1cd98801e
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Wed, 16 Mar 2016 14:14:22 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 17 Mar 2016 09:49:27 +0100

x86/iopl: Fix iopl capability check on Xen PV

iopl(3) is supposed to work if iopl is already 3, even if
unprivileged.  This didn't work right on Xen PV.  Fix it.

Reviewewd-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/8ce12013e6e4c0a44a97e316be4a6faff31bd5ea.1458162709.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/ioport.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index 37dae79..589b319 100644
--- a/arch/x86/kernel/ioport.c
+++ b/arch/x86/kernel/ioport.c
@@ -96,9 +96,14 @@ asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int turn_on)
 SYSCALL_DEFINE1(iopl, unsigned int, level)
 {
 	struct pt_regs *regs = current_pt_regs();
-	unsigned int old = (regs->flags >> 12) & 3;
 	struct thread_struct *t = &current->thread;
 
+	/*
+	 * Careful: the IOPL bits in regs->flags are undefined under Xen PV
+	 * and changing them has no effect.
+	 */
+	unsigned int old = t->iopl >> X86_EFLAGS_IOPL_BIT;
+
 	if (level > 3)
 		return -EINVAL;
 	/* Trying to gain more privileges? */
@@ -106,8 +111,9 @@ SYSCALL_DEFINE1(iopl, unsigned int, level)
 		if (!capable(CAP_SYS_RAWIO))
 			return -EPERM;
 	}
-	regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) | (level << 12);
-	t->iopl = level << 12;
+	regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) |
+		(level << X86_EFLAGS_IOPL_BIT);
+	t->iopl = level << X86_EFLAGS_IOPL_BIT;
 	set_iopl_mask(t->iopl);
 
 	return 0;

Re: [tip:x86/urgent] x86/iopl/64: Properly context-switch IOPL on Xen PV

[Borislav Petkov]

On Thu, Mar 17, 2016 at 02:19:12AM -0700, tip-bot for Andy Lutomirski wrote:
> Commit-ID:  b7a584598aea7ca73140cb87b40319944dd3393f
> Gitweb:     http://git.kernel.org/tip/b7a584598aea7ca73140cb87b40319944dd3393f
> Author:     Andy Lutomirski <luto@kernel.org>
> AuthorDate: Wed, 16 Mar 2016 14:14:21 -0700
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Thu, 17 Mar 2016 09:49:26 +0100
> 
> x86/iopl/64: Properly context-switch IOPL on Xen PV
> 
> On Xen PV, regs->flags doesn't reliably reflect IOPL and the
> exit-to-userspace code doesn't change IOPL.  We need to context
> switch it manually.
> 
> I'm doing this without going through paravirt because this is
> specific to Xen PV.  After the dust settles, we can merge this with
> the 32-bit code, tidy up the iopl syscall implementation, and remove
> the set_iopl pvop entirely.
> 
> Fixes XSA-171.
> 
> Reviewewd-by: Jan Beulich <JBeulich@suse.com>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Brian Gerst <brgerst@gmail.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Denys Vlasenko <dvlasenk@redhat.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Jan Beulich <JBeulich@suse.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: stable@vger.kernel.org
> Link: http://lkml.kernel.org/r/693c3bd7aeb4d3c27c92c622b7d0f554a458173c.1458162709.git.luto@kernel.org
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
>  arch/x86/include/asm/xen/hypervisor.h |  2 ++
>  arch/x86/kernel/process_64.c          | 12 ++++++++++++
>  arch/x86/xen/enlighten.c              |  2 +-
>  3 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
> index 8b2d4be..39171b3 100644
> --- a/arch/x86/include/asm/xen/hypervisor.h
> +++ b/arch/x86/include/asm/xen/hypervisor.h
> @@ -62,4 +62,6 @@ void xen_arch_register_cpu(int num);
>  void xen_arch_unregister_cpu(int num);
>  #endif
>  
> +extern void xen_set_iopl_mask(unsigned mask);
> +
>  #endif /* _ASM_X86_XEN_HYPERVISOR_H */
> diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
> index b9d99e0..9f75187 100644
> --- a/arch/x86/kernel/process_64.c
> +++ b/arch/x86/kernel/process_64.c
> @@ -48,6 +48,7 @@
>  #include <asm/syscalls.h>
>  #include <asm/debugreg.h>
>  #include <asm/switch_to.h>
> +#include <asm/xen/hypervisor.h>
>  
>  asmlinkage extern void ret_from_fork(void);
>  
> @@ -411,6 +412,17 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
>  		     task_thread_info(prev_p)->flags & _TIF_WORK_CTXSW_PREV))
>  		__switch_to_xtra(prev_p, next_p, tss);
>  
> +#ifdef CONFIG_XEN
> +	/*
> +	 * On Xen PV, IOPL bits in pt_regs->flags have no effect, and
> +	 * current_pt_regs()->flags may not match the current task's
> +	 * intended IOPL.  We need to switch it manually.
> +	 */
> +	if (unlikely(static_cpu_has(X86_FEATURE_XENPV) &&
> +		     prev->iopl != next->iopl))
> +		xen_set_iopl_mask(next->iopl);
> +#endif

I'm wondering if it would've been cleaner if this was a
arch_fixup_iopl_mask() defined in arch/x86/xen/enlighten.c and a stub
otherwise.

This would save you the ifdeffery and the export of
xen_set_iopl_mask()...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.