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


Groups > linux.kernel > #1359368 > unrolled thread

[PATCH v4 0/3] Xen iopl fixes

Started byAndy Lutomirski <luto@kernel.org>
First post2016-03-16 22:20 +0100
Last post2016-03-17 10:30 +0100
Articles 3 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v4 0/3] Xen iopl fixes Andy Lutomirski <luto@kernel.org> - 2016-03-16 22:20 +0100
    [PATCH v4 1/3] selftests/x86: Add a iopl test Andy Lutomirski <luto@kernel.org> - 2016-03-16 22:20 +0100
      [tip:x86/urgent] selftests/x86: Add an iopl test tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-03-17 10:30 +0100

#1359368 — [PATCH v4 0/3] Xen iopl fixes

FromAndy Lutomirski <luto@kernel.org>
Date2016-03-16 22:20 +0100
Subject[PATCH v4 0/3] Xen iopl fixes
Message-ID<rdr1f-6av-3@gated-at.bofh.it>
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

[toc] | [next] | [standalone]


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

FromAndy Lutomirski <luto@kernel.org>
Date2016-03-16 22:20 +0100
Subject[PATCH v4 1/3] selftests/x86: Add a iopl test
Message-ID<rdr1g-6av-17@gated-at.bofh.it>
In reply to#1359368
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

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


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

Fromtip-bot for Andy Lutomirski <tipbot@zytor.com>
Date2016-03-17 10:30 +0100
Subject[tip:x86/urgent] selftests/x86: Add an iopl test
Message-ID<rdCpJ-5uU-27@gated-at.bofh.it>
In reply to#1359370
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;
+}
+

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web