x86: modify_ldt improvement, test, and config option

modify_ldt has questionable locking and does not synchronize threads. Improve it: redesign the locking and synchronize all threads' LDTs using an IPI on all modifications.

This will dramatically slow down modify_ldt in multithreaded programs, but there shouldn't be any multithreaded programs that care about modify_ldt's performance in the first place.

Cc [email protected]

Signed-off-by Andy Lutomirski <[email protected]>

modify_ldt has questionable locking and does not synchronize threads. Improve it: redesign the locking and synchronize all threads' LDTs using an IPI on all modifications.

This will dramatically slow down modify_ldt in multithreaded programs, but there shouldn't be any multithreaded programs that care about modify_ldt's performance in the first place.

Cc [email protected]

Signed-off-by Andy Lutomirski <[email protected]>

arch/x86/include/asm/mmu_context.h static inline void load_mm_cr4(struct mm_struct *mm) {} · line 54

Quoted 5 lines · Andy Lutomirski

arch/x86/include/asm/mmu_context.h +5 -0

+
+static inline void load_mm_ldt(struct mm_struct *mm)
+{
+       struct ldt_struct *ldt;
+       DEBUG_LOCKS_WARN_ON(!irqs_disabled());

I thought this was supposed to be checking preemptible()?

-boris

arch/x86/include/asm/mmu_context.h static inline void load_mm_cr4(struct mm_struct *mm) {} · line 54

Quoted 5 lines · Andy Lutomirski

arch/x86/include/asm/mmu_context.h +5 -0

+
+static inline void load_mm_ldt(struct mm_struct *mm)
+{
+       struct ldt_struct *ldt;
+       DEBUG_LOCKS_WARN_ON(!irqs_disabled());

I thought this was supposed to be checking preemptible()?

-boris

Quoted 10 lines · Boris Ostrovsky

arch/x86/include/asm/mmu_context.h static inline void load_mm_cr4(struct mm_struct *mm) {} · line 54

Quoted 5 lines · Andy Lutomirski

arch/x86/include/asm/mmu_context.h +5 -0

+
+static inline void load_mm_ldt(struct mm_struct *mm)
+{
+       struct ldt_struct *ldt;
+       DEBUG_LOCKS_WARN_ON(!irqs_disabled());

I thought this was supposed to be checking preemptible()?

v6 fixes that. Check your future inbox :) I'm goint to rework the Xen bit too based on the long discussion.

Is that the only failure you're seeing?

ldt_gdt_32 passes on 64-bit for me

Quoted 10 lines · Boris Ostrovsky

arch/x86/include/asm/mmu_context.h static inline void load_mm_cr4(struct mm_struct *mm) {} · line 54

Quoted 5 lines · Andy Lutomirski

arch/x86/include/asm/mmu_context.h +5 -0

+
+static inline void load_mm_ldt(struct mm_struct *mm)
+{
+       struct ldt_struct *ldt;
+       DEBUG_LOCKS_WARN_ON(!irqs_disabled());

I thought this was supposed to be checking preemptible()?

v6 fixes that. Check your future inbox :) I'm goint to rework the Xen bit too based on the long discussion.

Is that the only failure you're seeing?

ldt_gdt_32 passes on 64-bit for me

Quoted 16 lines · Andy Lutomirski

Quoted 12 lines · Boris Ostrovsky

arch/x86/include/asm/mmu_context.h static inline void load_mm_cr4(struct mm_struct *mm) {} · line 54

Quoted 5 lines · Andy Lutomirski

arch/x86/include/asm/mmu_context.h +5 -0

+
+static inline void load_mm_ldt(struct mm_struct *mm)
+{
+       struct ldt_struct *ldt;
+       DEBUG_LOCKS_WARN_ON(!irqs_disabled());

I thought this was supposed to be checking preemptible()?

Quoted 4 lines

v6 fixes that. Check your future inbox :) I'm goint to rework the Xen bit too based on the long discussion.

Is that the only failure you're seeing?

Yes.

Quoted 1 line

ldt_gdt_32 passes on 64-bit for me

With your patch:

root@haswell> uname -a
Linux dhcp-burlington7-2nd-B-east-10-152-55-89.usdhcp.oraclecorp.com 4.2.0-rc4 #107 SMP Thu Jul 30 11:05:19 EDT 2015 x86_64 x86_64 x86_64 GNU/Linux
root@haswell> cd tmp/linux/tools/testing/selftests/x86/ root@haswell> ls -l ldt_gdt_32
-rwxr-xr-x 1 root root 25975 Jul 30 11:48 ldt_gdt_32 root@haswell> ./ldt_gdt_32

[OK]    LDT entry 0 has AR 0x0040FA00 and limit 0x0000000A
[OK]    LDT entry 0 has AR 0x00C0FA00 and limit 0x0000AFFF
[OK]    LDT entry 1 is invalid
[OK]    LDT entry 2 has AR 0x00C0FA00 and limit 0x0000AFFF
[OK]    LDT entry 1 is invalid
[OK]    LDT entry 2 has AR 0x00C0FA00 and limit 0x0000AFFF
[OK]    LDT entry 2 has AR 0x00D0FA00 and limit 0x0000AFFF
[OK]    LDT entry 2 has AR 0x00D07A00 and limit 0x0000AFFF
[OK]    LDT entry 2 has AR 0x00907A00 and limit 0x0000AFFF
[OK]    LDT entry 2 has AR 0x00D07200 and limit 0x0000AFFF
[OK]    LDT entry 2 has AR 0x00D07000 and limit 0x0000AFFF
[OK]    LDT entry 2 has AR 0x00D07400 and limit 0x0000AFFF
[OK]    LDT entry 2 has AR 0x00507600 and limit 0x0000000A
[OK]    LDT entry 2 has AR 0x00507E00 and limit 0x0000000A
[OK]    LDT entry 2 has AR 0x00507C00 and limit 0x0000000A
[OK]    LDT entry 2 has AR 0x00507A00 and limit 0x0000000A
[OK]    LDT entry 2 has AR 0x00507800 and limit 0x0000000A
[RUN]   Test fork
[OK]    LDT entry 2 has AR 0x00507800 and limit 0x0000000A
[OK]    LDT entry 1 is invalid
[OK]    Child succeeded
[RUN]   Test size
[DONE]  Size test
[OK]    modify_ldt failure 22
[OK]    LDT entry 0 has AR 0x0000F200 and limit 0x00000000
[OK]    LDT entry 0 has AR 0x00007200 and limit 0x00000000
[OK]    LDT entry 0 has AR 0x0000F000 and limit 0x00000000
[OK]    LDT entry 0 has AR 0x00007200 and limit 0x00000000
[OK]    LDT entry 0 has AR 0x00007000 and limit 0x00000001
[OK]    LDT entry 0 has AR 0x00007000 and limit 0x00000000
[OK]    LDT entry 0 is invalid
[OK]    LDT entry 0 has AR 0x0040F200 and limit 0x00000000
[OK]    LDT entry 0 is invalid
[RUN]   Cross-CPU LDT invalidation

Segmentation fault (core dumped)

Quoted 16 lines · Andy Lutomirski

Quoted 12 lines · Boris Ostrovsky

arch/x86/include/asm/mmu_context.h static inline void load_mm_cr4(struct mm_struct *mm) {} · line 54

Quoted 5 lines · Andy Lutomirski

arch/x86/include/asm/mmu_context.h +5 -0

+
+static inline void load_mm_ldt(struct mm_struct *mm)
+{
+       struct ldt_struct *ldt;
+       DEBUG_LOCKS_WARN_ON(!irqs_disabled());

I thought this was supposed to be checking preemptible()?

Quoted 4 lines

v6 fixes that. Check your future inbox :) I'm goint to rework the Xen bit too based on the long discussion.

Is that the only failure you're seeing?

Yes.

Quoted 1 line

ldt_gdt_32 passes on 64-bit for me

With your patch:

root@haswell> uname -a
Linux dhcp-burlington7-2nd-B-east-10-152-55-89.usdhcp.oraclecorp.com 4.2.0-rc4 #107 SMP Thu Jul 30 11:05:19 EDT 2015 x86_64 x86_64 x86_64 GNU/Linux
root@haswell> cd tmp/linux/tools/testing/selftests/x86/ root@haswell> ls -l ldt_gdt_32
-rwxr-xr-x 1 root root 25975 Jul 30 11:48 ldt_gdt_32 root@haswell> ./ldt_gdt_32

[OK]    LDT entry 0 has AR 0x0040FA00 and limit 0x0000000A
[OK]    LDT entry 0 has AR 0x00C0FA00 and limit 0x0000AFFF
[OK]    LDT entry 1 is invalid
[OK]    LDT entry 2 has AR 0x00C0FA00 and limit 0x0000AFFF
[OK]    LDT entry 1 is invalid
[OK]    LDT entry 2 has AR 0x00C0FA00 and limit 0x0000AFFF
[OK]    LDT entry 2 has AR 0x00D0FA00 and limit 0x0000AFFF
[OK]    LDT entry 2 has AR 0x00D07A00 and limit 0x0000AFFF
[OK]    LDT entry 2 has AR 0x00907A00 and limit 0x0000AFFF
[OK]    LDT entry 2 has AR 0x00D07200 and limit 0x0000AFFF
[OK]    LDT entry 2 has AR 0x00D07000 and limit 0x0000AFFF
[OK]    LDT entry 2 has AR 0x00D07400 and limit 0x0000AFFF
[OK]    LDT entry 2 has AR 0x00507600 and limit 0x0000000A
[OK]    LDT entry 2 has AR 0x00507E00 and limit 0x0000000A
[OK]    LDT entry 2 has AR 0x00507C00 and limit 0x0000000A
[OK]    LDT entry 2 has AR 0x00507A00 and limit 0x0000000A
[OK]    LDT entry 2 has AR 0x00507800 and limit 0x0000000A
[RUN]   Test fork
[OK]    LDT entry 2 has AR 0x00507800 and limit 0x0000000A
[OK]    LDT entry 1 is invalid
[OK]    Child succeeded
[RUN]   Test size
[DONE]  Size test
[OK]    modify_ldt failure 22
[OK]    LDT entry 0 has AR 0x0000F200 and limit 0x00000000
[OK]    LDT entry 0 has AR 0x00007200 and limit 0x00000000
[OK]    LDT entry 0 has AR 0x0000F000 and limit 0x00000000
[OK]    LDT entry 0 has AR 0x00007200 and limit 0x00000000
[OK]    LDT entry 0 has AR 0x00007000 and limit 0x00000001
[OK]    LDT entry 0 has AR 0x00007000 and limit 0x00000000
[OK]    LDT entry 0 is invalid
[OK]    LDT entry 0 has AR 0x0040F200 and limit 0x00000000
[OK]    LDT entry 0 is invalid
[RUN]   Cross-CPU LDT invalidation

Segmentation fault (core dumped)

Quoted 4 lines · Boris Ostrovsky

[OK]    LDT entry 0 has AR 0x0040F200 and limit 0x00000000
[OK]    LDT entry 0 is invalid
[RUN]   Cross-CPU LDT invalidation

Segmentation fault (core dumped)

That's not good.

Can you backtrace it? (I.e. compite ldt_gdt_32 with -g and load the core dumb in gdb?) My best guesses are either a signal delivery failure (although that shouldn't be a problem for 32-bit userspace on any kernel) or an actual LDT access fault, and the latter would be interesting.

I haven't been able to reproduce this.

Quoted 4 lines · Boris Ostrovsky

[OK]    LDT entry 0 has AR 0x0040F200 and limit 0x00000000
[OK]    LDT entry 0 is invalid
[RUN]   Cross-CPU LDT invalidation

Segmentation fault (core dumped)

That's not good.

Can you backtrace it? (I.e. compite ldt_gdt_32 with -g and load the core dumb in gdb?) My best guesses are either a signal delivery failure (although that shouldn't be a problem for 32-bit userspace on any kernel) or an actual LDT access fault, and the latter would be interesting.

I haven't been able to reproduce this.

modify_ldt has questionable locking and does not synchronize threads. Improve it: redesign the locking and synchronize all threads' LDTs using an IPI on all modifications.

This will dramatically slow down modify_ldt in multithreaded programs, but there shouldn't be any multithreaded programs that care about modify_ldt's performance in the first place.

modify_ldt has questionable locking and does not synchronize threads. Improve it: redesign the locking and synchronize all threads' LDTs using an IPI on all modifications.

This will dramatically slow down modify_ldt in multithreaded programs, but there shouldn't be any multithreaded programs that care about modify_ldt's performance in the first place.

This tests general modify_ldt behavior (only writes, so far) as well as synchronous updates via IPI. It fails on old kernels.

I called this ldt_gdt because I'll add set_thread_area tests to it at some point.

Signed-off-by Andy Lutomirski <[email protected]>

---
 tools/testing/selftests/x86/Makefile  |   2 +-
 tools/testing/selftests/x86/ldt_gdt.c | 520 ++++++++++++++++++++++++++++++++++
 2 files changed, 521 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/x86/ldt_gdt.c

tools/testing/selftests/x86/Makefile +1 -1

index caa60d56d7d1..4138387b892c 100644
@@ -4,7 +4,7 @@ include ../lib.mk
 
 .PHONY: all all_32 all_64 warn_32bit_failure clean
 
-TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs
+TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs ldt_gdt
 TARGETS_C_32BIT_ONLY := entry_from_vm86
 
 TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY)

tools/testing/selftests/x86/ldt_gdt.c new file +520 -1

new file mode 100644
index 000000000000..c27adfc9ae72
@@ -0,0 +1,520 @@
+/*
+ * ldt_gdt.c - Test cases for LDT and GDT access
+ * 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/syscall.h>
+#include <asm/ldt.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <stdbool.h>
+#include <pthread.h>
+#include <sched.h>
+#include <linux/futex.h>
+
+#define AR_ACCESSED            (1<<8)
+
+#define AR_TYPE_RODATA         (0 * (1<<9))
+#define AR_TYPE_RWDATA         (1 * (1<<9))
+#define AR_TYPE_RODATA_EXPDOWN (2 * (1<<9))
+#define AR_TYPE_RWDATA_EXPDOWN (3 * (1<<9))
+#define AR_TYPE_XOCODE         (4 * (1<<9))
+#define AR_TYPE_XRCODE         (5 * (1<<9))
+#define AR_TYPE_XOCODE_CONF    (6 * (1<<9))
+#define AR_TYPE_XRCODE_CONF    (7 * (1<<9))
+
+#define AR_DPL3                        (3 * (1<<13))
+
+#define AR_S                   (1 << 12)
+#define AR_P                   (1 << 15)
+#define AR_AVL                 (1 << 20)
+#define AR_L                   (1 << 21)
+#define AR_DB                  (1 << 22)
+#define AR_G                   (1 << 23)
+
+static int nerrs;
+
+static void check_invalid_segment(uint16_t index, int ldt)
+{
+       uint32_t has_limit = 0, has_ar = 0, limit, ar;
+       uint32_t selector = (index << 3) | (ldt << 2) | 3;
+
+       asm ("lsl %[selector], %[limit]\n\t"
+            "jnz 1f\n\t"
+            "movl $1, %[has_limit]\n\t"
+            "1:"
+            : [limit] "=r" (limit), [has_limit] "+rm" (has_limit)
+            : [selector] "r" (selector));
+       asm ("larl %[selector], %[ar]\n\t"
+            "jnz 1f\n\t"
+            "movl $1, %[has_ar]\n\t"
+            "1:"
+            : [ar] "=r" (ar), [has_ar] "+rm" (has_ar)
+            : [selector] "r" (selector));
+
+       if (has_limit || has_ar) {
+               printf("[FAIL]\t%s entry %hu is valid but should be invalid\n",
+                      (ldt ? "LDT" : "GDT"), index);
+               nerrs++;
+       } else {
+               printf("[OK]\t%s entry %hu is invalid\n",
+                      (ldt ? "LDT" : "GDT"), index);
+       }
+}
+
+static void check_valid_segment(uint16_t index, int ldt,
+                               uint32_t expected_ar, uint32_t expected_limit,
+                               bool verbose)
+{
+       uint32_t has_limit = 0, has_ar = 0, limit, ar;
+       uint32_t selector = (index << 3) | (ldt << 2) | 3;
+
+       asm ("lsl %[selector], %[limit]\n\t"
+            "jnz 1f\n\t"
+            "movl $1, %[has_limit]\n\t"
+            "1:"
+            : [limit] "=r" (limit), [has_limit] "+rm" (has_limit)
+            : [selector] "r" (selector));
+       asm ("larl %[selector], %[ar]\n\t"
+            "jnz 1f\n\t"
+            "movl $1, %[has_ar]\n\t"
+            "1:"
+            : [ar] "=r" (ar), [has_ar] "+rm" (has_ar)
+            : [selector] "r" (selector));
+
+       if (!has_limit || !has_ar) {
+               printf("[FAIL]\t%s entry %hu is invalid but should be valid\n",
+                      (ldt ? "LDT" : "GDT"), index);
+               nerrs++;
+               return;
+       }
+
+       if (ar != expected_ar) {
+               printf("[FAIL]\t%s entry %hu has AR 0x%08X but expected 0x%08X\n",
+                      (ldt ? "LDT" : "GDT"), index, ar, expected_ar);
+               nerrs++;
+       } else if (limit != expected_limit) {
+               printf("[FAIL]\t%s entry %hu has limit 0x%08X but expected 0x%08X\n",
+                      (ldt ? "LDT" : "GDT"), index, limit, expected_limit);
+               nerrs++;
+       } else if (verbose) {
+               printf("[OK]\t%s entry %hu has AR 0x%08X and limit 0x%08X\n",
+                      (ldt ? "LDT" : "GDT"), index, ar, limit);
+       }
+}
+
+static bool install_valid_mode(const struct user_desc *desc, uint32_t ar,
+                              bool oldmode)
+{
+       int ret = syscall(SYS_modify_ldt, oldmode ? 1 : 0x11,
+                         desc, sizeof(*desc));
+       if (ret < -1)
+               errno = -ret;
+       if (ret == 0) {
+               uint32_t limit = desc->limit;
+               if (desc->limit_in_pages)
+                       limit = (limit << 12) + 4095;
+               check_valid_segment(desc->entry_number, 1, ar, limit, true);
+               return true;
+       } else if (errno == ENOSYS) {
+               printf("[OK]\tmodify_ldt returned -ENOSYS\n");
+               return false;
+       } else {
+               if (desc->seg_32bit) {
+                       printf("[FAIL]\tUnexpected modify_ldt failure %d\n",
+                              errno);
+                       nerrs++;
+                       return false;
+               } else {
+                       printf("[OK]\tmodify_ldt rejected 16 bit segment\n");
+                       return false;
+               }
+       }
+}
+
+static bool install_valid(const struct user_desc *desc, uint32_t ar)
+{
+       return install_valid_mode(desc, ar, false);
+}
+
+static void install_invalid(const struct user_desc *desc, bool oldmode)
+{
+       int ret = syscall(SYS_modify_ldt, oldmode ? 1 : 0x11,
+                         desc, sizeof(*desc));
+       if (ret < -1)
+               errno = -ret;
+       if (ret == 0) {
+               check_invalid_segment(desc->entry_number, 1);
+       } else if (errno == ENOSYS) {
+               printf("[OK]\tmodify_ldt returned -ENOSYS\n");
+       } else {
+               if (desc->seg_32bit) {
+                       printf("[FAIL]\tUnexpected modify_ldt failure %d\n",
+                              errno);
+                       nerrs++;
+               } else {
+                       printf("[OK]\tmodify_ldt rejected 16 bit segment\n");
+               }
+       }
+}
+
+static int safe_modify_ldt(int func, struct user_desc *ptr,
+                          unsigned long bytecount)
+{
+       int ret = syscall(SYS_modify_ldt, 0x11, ptr, bytecount);
+       if (ret < -1)
+               errno = -ret;
+       return ret;
+}
+
+static void fail_install(struct user_desc *desc)
+{
+       if (safe_modify_ldt(0x11, desc, sizeof(*desc)) == 0) {
+               printf("[FAIL]\tmodify_ldt accepted a bad descriptor\n");
+               nerrs++;
+       } else if (errno == ENOSYS) {
+               printf("[OK]\tmodify_ldt returned -ENOSYS\n");
+       } else {
+               printf("[OK]\tmodify_ldt failure %d\n", errno);
+       }
+}
+
+static void do_simple_tests(void)
+{
+       struct user_desc desc = {
+               .entry_number    = 0,
+               .base_addr       = 0,
+               .limit           = 10,
+               .seg_32bit       = 1,
+               .contents        = 2, /* Code, not conforming */
+               .read_exec_only  = 0,
+               .limit_in_pages  = 0,
+               .seg_not_present = 0,
+               .useable         = 0
+       };
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE | AR_S | AR_P | AR_DB);
+
+       desc.limit_in_pages = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE |
+                     AR_S | AR_P | AR_DB | AR_G);
+
+       check_invalid_segment(1, 1);
+
+       desc.entry_number = 2;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE |
+                     AR_S | AR_P | AR_DB | AR_G);
+
+       check_invalid_segment(1, 1);
+
+       desc.base_addr = 0xf0000000;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE |
+                     AR_S | AR_P | AR_DB | AR_G);
+
+       desc.useable = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE |
+                     AR_S | AR_P | AR_DB | AR_G | AR_AVL);
+
+       desc.seg_not_present = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE |
+                     AR_S | AR_DB | AR_G | AR_AVL);
+
+       desc.seg_32bit = 0;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE |
+                     AR_S | AR_G | AR_AVL);
+
+       desc.seg_32bit = 1;
+       desc.contents = 0;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RWDATA |
+                     AR_S | AR_DB | AR_G | AR_AVL);
+
+       desc.read_exec_only = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RODATA |
+                     AR_S | AR_DB | AR_G | AR_AVL);
+
+       desc.contents = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RODATA_EXPDOWN |
+                     AR_S | AR_DB | AR_G | AR_AVL);
+
+       desc.read_exec_only = 0;
+       desc.limit_in_pages = 0;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RWDATA_EXPDOWN |
+                     AR_S | AR_DB | AR_AVL);
+
+       desc.contents = 3;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE_CONF |
+                     AR_S | AR_DB | AR_AVL);
+
+       desc.read_exec_only = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XOCODE_CONF |
+                     AR_S | AR_DB | AR_AVL);
+
+       desc.read_exec_only = 0;
+       desc.contents = 2;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE |
+                     AR_S | AR_DB | AR_AVL);
+
+       desc.read_exec_only = 1;
+
+#ifdef __x86_64__
+       desc.lm = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XOCODE |
+                     AR_S | AR_DB | AR_AVL);
+       desc.lm = 0;
+#endif
+
+       bool entry1_okay = install_valid(&desc, AR_DPL3 | AR_TYPE_XOCODE |
+                                        AR_S | AR_DB | AR_AVL);
+
+       if (entry1_okay) {
+               printf("[RUN]\tTest fork\n");
+               pid_t child = fork();
+               if (child == 0) {
+                       nerrs = 0;
+                       check_valid_segment(desc.entry_number, 1,
+                                           AR_DPL3 | AR_TYPE_XOCODE |
+                                           AR_S | AR_DB | AR_AVL, desc.limit,
+                                           true);
+                       check_invalid_segment(1, 1);
+                       exit(nerrs ? 1 : 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]\tTest size\n");
+               int i;
+               for (i = 0; i < 8192; i++) {
+                       desc.entry_number = i;
+                       desc.limit = i;
+                       if (safe_modify_ldt(0x11, &desc, sizeof(desc)) != 0) {
+                               printf("[FAIL]\tFailed to install entry %d\n", i);
+                               nerrs++;
+                               break;
+                       }
+               }
+               for (int j = 0; j < i; j++) {
+                       check_valid_segment(j, 1, AR_DPL3 | AR_TYPE_XOCODE |
+                                           AR_S | AR_DB | AR_AVL, j, false);
+               }
+               printf("[DONE]\tSize test\n");
+       } else {
+               printf("[SKIP]\tSkipping fork and size tests because we have no LDT\n");
+       }
+
+       /* Test entry_number too high. */
+       desc.entry_number = 8192;
+       fail_install(&desc);
+
+       /* Test deletion and actions mistakeable for deletion. */
+       memset(&desc, 0, sizeof(desc));
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RWDATA | AR_S | AR_P);
+
+       desc.seg_not_present = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RWDATA | AR_S);
+
+       desc.seg_not_present = 0;
+       desc.read_exec_only = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RODATA | AR_S | AR_P);
+
+       desc.read_exec_only = 0;
+       desc.seg_not_present = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RWDATA | AR_S);
+
+       desc.read_exec_only = 1;
+       desc.limit = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RODATA | AR_S);
+
+       desc.limit = 0;
+       desc.base_addr = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RODATA | AR_S);
+
+       desc.base_addr = 0;
+       install_invalid(&desc, false);
+
+       desc.seg_not_present = 0;
+       desc.read_exec_only = 0;
+       desc.seg_32bit = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RWDATA | AR_S | AR_P | AR_DB);
+       install_invalid(&desc, true);
+}
+
+/*
+ * 0: thread is idle
+ * 1: thread armed
+ * 2: thread should clear LDT entry 0
+ * 3: thread should exit
+ */
+static volatile unsigned int ftx;
+
+static void *threadproc(void *ctx)
+{
+       cpu_set_t cpuset;
+       CPU_ZERO(&cpuset);
+       CPU_SET(1, &cpuset);
+       if (sched_setaffinity(0, sizeof(cpuset), &cpuset) != 0)
+               err(1, "sched_setaffinity to CPU 1");   /* should never fail */
+
+       while (1) {
+               syscall(SYS_futex, &ftx, FUTEX_WAIT, 0, NULL, NULL, 0);
+               while (ftx != 2) {
+                       if (ftx >= 3)
+                               return NULL;
+               }
+
+               /* clear LDT entry 0 */
+               const struct user_desc desc = {};
+               if (syscall(SYS_modify_ldt, 1, &desc, sizeof(desc)) != 0)
+                       err(1, "modify_ldt");
+
+               /* If ftx == 2, set it to zero.  If ftx == 100, quit. */
+               unsigned int x = -2;
+               asm volatile ("lock xaddl %[x], %[ftx]" :
+                             [x] "+r" (x), [ftx] "+m" (ftx));
+               if (x != 2)
+                       return NULL;
+       }
+}
+
+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 *info, void *ctx_void)
+{
+       siglongjmp(jmpbuf, 1);
+}
+
+static void do_multicpu_tests(void)
+{
+       cpu_set_t cpuset;
+       pthread_t thread;
+       int failures = 0, iters = 5, i;
+       unsigned short orig_ss;
+
+       CPU_ZERO(&cpuset);
+       CPU_SET(1, &cpuset);
+       if (sched_setaffinity(0, sizeof(cpuset), &cpuset) != 0) {
+               printf("[SKIP]\tCannot set affinity to CPU 1\n");
+               return;
+       }
+
+       CPU_ZERO(&cpuset);
+       CPU_SET(0, &cpuset);
+       if (sched_setaffinity(0, sizeof(cpuset), &cpuset) != 0) {
+               printf("[SKIP]\tCannot set affinity to CPU 0\n");
+               return;
+       }
+
+       sethandler(SIGSEGV, sigsegv, 0);
+#ifdef __i386__
+       /* True 32-bit kernels send SIGILL instead of SIGSEGV on IRET faults. */
+       sethandler(SIGILL, sigsegv, 0);
+#endif
+
+       printf("[RUN]\tCross-CPU LDT invalidation\n");
+
+       if (pthread_create(&thread, 0, threadproc, 0) != 0)
+               err(1, "pthread_create");
+
+       asm volatile ("mov %%ss, %0" : "=rm" (orig_ss));
+
+       for (i = 0; i < 5; i++) {
+               if (sigsetjmp(jmpbuf, 1) != 0)
+                       continue;
+
+               /* Make sure the thread is ready after the last test. */
+               while (ftx != 0)
+                       ;
+
+               struct user_desc desc = {
+                       .entry_number    = 0,
+                       .base_addr       = 0,
+                       .limit           = 0xfffff,
+                       .seg_32bit       = 1,
+                       .contents        = 0, /* Data */
+                       .read_exec_only  = 0,
+                       .limit_in_pages  = 1,
+                       .seg_not_present = 0,
+                       .useable         = 0
+               };
+
+               if (safe_modify_ldt(0x11, &desc, sizeof(desc)) != 0) {
+                       if (errno != ENOSYS)
+                               err(1, "modify_ldt");
+                       printf("[SKIP]\tmodify_ldt unavailable\n");
+                       break;
+               }
+
+               /* Arm the thread. */
+               ftx = 1;
+               syscall(SYS_futex, &ftx, FUTEX_WAKE, 0, NULL, NULL, 0);
+
+               asm volatile ("mov %0, %%ss" : : "r" (0x7));
+
+               /* Go! */
+               ftx = 2;
+
+               while (ftx != 0)
+                       ;
+
+               /*
+                * On success, modify_ldt will segfault us synchronously,
+                * and we'll escape via siglongjmp.
+                */
+
+               failures++;
+               asm volatile ("mov %0, %%ss" : : "rm" (orig_ss));
+       };
+
+       ftx = 100;  /* Kill the thread. */
+       syscall(SYS_futex, &ftx, FUTEX_WAKE, 0, NULL, NULL, 0);
+
+       if (pthread_join(thread, NULL) != 0)
+               err(1, "pthread_join");
+
+       if (failures) {
+               printf("[FAIL]\t%d of %d iterations failed\n", failures, iters);
+               nerrs++;
+       } else {
+               printf("[OK]\tAll %d iterations succeeded\n", iters);
+       }
+}
+
+int main()
+{
+       do_simple_tests();
+
+       do_multicpu_tests();
+
+       return nerrs ? 1 : 0;
+}
--

2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

This tests general modify_ldt behavior (only writes, so far) as well as synchronous updates via IPI. It fails on old kernels.

I called this ldt_gdt because I'll add set_thread_area tests to it at some point.

Signed-off-by Andy Lutomirski <[email protected]>

---
 tools/testing/selftests/x86/Makefile  |   2 +-
 tools/testing/selftests/x86/ldt_gdt.c | 520 ++++++++++++++++++++++++++++++++++
 2 files changed, 521 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/x86/ldt_gdt.c

tools/testing/selftests/x86/Makefile +1 -1

index caa60d56d7d1..4138387b892c 100644
@@ -4,7 +4,7 @@ include ../lib.mk
 
 .PHONY: all all_32 all_64 warn_32bit_failure clean
 
-TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs
+TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs ldt_gdt
 TARGETS_C_32BIT_ONLY := entry_from_vm86
 
 TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY)

tools/testing/selftests/x86/ldt_gdt.c new file +520 -1

new file mode 100644
index 000000000000..c27adfc9ae72
@@ -0,0 +1,520 @@
+/*
+ * ldt_gdt.c - Test cases for LDT and GDT access
+ * 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/syscall.h>
+#include <asm/ldt.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <stdbool.h>
+#include <pthread.h>
+#include <sched.h>
+#include <linux/futex.h>
+
+#define AR_ACCESSED            (1<<8)
+
+#define AR_TYPE_RODATA         (0 * (1<<9))
+#define AR_TYPE_RWDATA         (1 * (1<<9))
+#define AR_TYPE_RODATA_EXPDOWN (2 * (1<<9))
+#define AR_TYPE_RWDATA_EXPDOWN (3 * (1<<9))
+#define AR_TYPE_XOCODE         (4 * (1<<9))
+#define AR_TYPE_XRCODE         (5 * (1<<9))
+#define AR_TYPE_XOCODE_CONF    (6 * (1<<9))
+#define AR_TYPE_XRCODE_CONF    (7 * (1<<9))
+
+#define AR_DPL3                        (3 * (1<<13))
+
+#define AR_S                   (1 << 12)
+#define AR_P                   (1 << 15)
+#define AR_AVL                 (1 << 20)
+#define AR_L                   (1 << 21)
+#define AR_DB                  (1 << 22)
+#define AR_G                   (1 << 23)
+
+static int nerrs;
+
+static void check_invalid_segment(uint16_t index, int ldt)
+{
+       uint32_t has_limit = 0, has_ar = 0, limit, ar;
+       uint32_t selector = (index << 3) | (ldt << 2) | 3;
+
+       asm ("lsl %[selector], %[limit]\n\t"
+            "jnz 1f\n\t"
+            "movl $1, %[has_limit]\n\t"
+            "1:"
+            : [limit] "=r" (limit), [has_limit] "+rm" (has_limit)
+            : [selector] "r" (selector));
+       asm ("larl %[selector], %[ar]\n\t"
+            "jnz 1f\n\t"
+            "movl $1, %[has_ar]\n\t"
+            "1:"
+            : [ar] "=r" (ar), [has_ar] "+rm" (has_ar)
+            : [selector] "r" (selector));
+
+       if (has_limit || has_ar) {
+               printf("[FAIL]\t%s entry %hu is valid but should be invalid\n",
+                      (ldt ? "LDT" : "GDT"), index);
+               nerrs++;
+       } else {
+               printf("[OK]\t%s entry %hu is invalid\n",
+                      (ldt ? "LDT" : "GDT"), index);
+       }
+}
+
+static void check_valid_segment(uint16_t index, int ldt,
+                               uint32_t expected_ar, uint32_t expected_limit,
+                               bool verbose)
+{
+       uint32_t has_limit = 0, has_ar = 0, limit, ar;
+       uint32_t selector = (index << 3) | (ldt << 2) | 3;
+
+       asm ("lsl %[selector], %[limit]\n\t"
+            "jnz 1f\n\t"
+            "movl $1, %[has_limit]\n\t"
+            "1:"
+            : [limit] "=r" (limit), [has_limit] "+rm" (has_limit)
+            : [selector] "r" (selector));
+       asm ("larl %[selector], %[ar]\n\t"
+            "jnz 1f\n\t"
+            "movl $1, %[has_ar]\n\t"
+            "1:"
+            : [ar] "=r" (ar), [has_ar] "+rm" (has_ar)
+            : [selector] "r" (selector));
+
+       if (!has_limit || !has_ar) {
+               printf("[FAIL]\t%s entry %hu is invalid but should be valid\n",
+                      (ldt ? "LDT" : "GDT"), index);
+               nerrs++;
+               return;
+       }
+
+       if (ar != expected_ar) {
+               printf("[FAIL]\t%s entry %hu has AR 0x%08X but expected 0x%08X\n",
+                      (ldt ? "LDT" : "GDT"), index, ar, expected_ar);
+               nerrs++;
+       } else if (limit != expected_limit) {
+               printf("[FAIL]\t%s entry %hu has limit 0x%08X but expected 0x%08X\n",
+                      (ldt ? "LDT" : "GDT"), index, limit, expected_limit);
+               nerrs++;
+       } else if (verbose) {
+               printf("[OK]\t%s entry %hu has AR 0x%08X and limit 0x%08X\n",
+                      (ldt ? "LDT" : "GDT"), index, ar, limit);
+       }
+}
+
+static bool install_valid_mode(const struct user_desc *desc, uint32_t ar,
+                              bool oldmode)
+{
+       int ret = syscall(SYS_modify_ldt, oldmode ? 1 : 0x11,
+                         desc, sizeof(*desc));
+       if (ret < -1)
+               errno = -ret;
+       if (ret == 0) {
+               uint32_t limit = desc->limit;
+               if (desc->limit_in_pages)
+                       limit = (limit << 12) + 4095;
+               check_valid_segment(desc->entry_number, 1, ar, limit, true);
+               return true;
+       } else if (errno == ENOSYS) {
+               printf("[OK]\tmodify_ldt returned -ENOSYS\n");
+               return false;
+       } else {
+               if (desc->seg_32bit) {
+                       printf("[FAIL]\tUnexpected modify_ldt failure %d\n",
+                              errno);
+                       nerrs++;
+                       return false;
+               } else {
+                       printf("[OK]\tmodify_ldt rejected 16 bit segment\n");
+                       return false;
+               }
+       }
+}
+
+static bool install_valid(const struct user_desc *desc, uint32_t ar)
+{
+       return install_valid_mode(desc, ar, false);
+}
+
+static void install_invalid(const struct user_desc *desc, bool oldmode)
+{
+       int ret = syscall(SYS_modify_ldt, oldmode ? 1 : 0x11,
+                         desc, sizeof(*desc));
+       if (ret < -1)
+               errno = -ret;
+       if (ret == 0) {
+               check_invalid_segment(desc->entry_number, 1);
+       } else if (errno == ENOSYS) {
+               printf("[OK]\tmodify_ldt returned -ENOSYS\n");
+       } else {
+               if (desc->seg_32bit) {
+                       printf("[FAIL]\tUnexpected modify_ldt failure %d\n",
+                              errno);
+                       nerrs++;
+               } else {
+                       printf("[OK]\tmodify_ldt rejected 16 bit segment\n");
+               }
+       }
+}
+
+static int safe_modify_ldt(int func, struct user_desc *ptr,
+                          unsigned long bytecount)
+{
+       int ret = syscall(SYS_modify_ldt, 0x11, ptr, bytecount);
+       if (ret < -1)
+               errno = -ret;
+       return ret;
+}
+
+static void fail_install(struct user_desc *desc)
+{
+       if (safe_modify_ldt(0x11, desc, sizeof(*desc)) == 0) {
+               printf("[FAIL]\tmodify_ldt accepted a bad descriptor\n");
+               nerrs++;
+       } else if (errno == ENOSYS) {
+               printf("[OK]\tmodify_ldt returned -ENOSYS\n");
+       } else {
+               printf("[OK]\tmodify_ldt failure %d\n", errno);
+       }
+}
+
+static void do_simple_tests(void)
+{
+       struct user_desc desc = {
+               .entry_number    = 0,
+               .base_addr       = 0,
+               .limit           = 10,
+               .seg_32bit       = 1,
+               .contents        = 2, /* Code, not conforming */
+               .read_exec_only  = 0,
+               .limit_in_pages  = 0,
+               .seg_not_present = 0,
+               .useable         = 0
+       };
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE | AR_S | AR_P | AR_DB);
+
+       desc.limit_in_pages = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE |
+                     AR_S | AR_P | AR_DB | AR_G);
+
+       check_invalid_segment(1, 1);
+
+       desc.entry_number = 2;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE |
+                     AR_S | AR_P | AR_DB | AR_G);
+
+       check_invalid_segment(1, 1);
+
+       desc.base_addr = 0xf0000000;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE |
+                     AR_S | AR_P | AR_DB | AR_G);
+
+       desc.useable = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE |
+                     AR_S | AR_P | AR_DB | AR_G | AR_AVL);
+
+       desc.seg_not_present = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE |
+                     AR_S | AR_DB | AR_G | AR_AVL);
+
+       desc.seg_32bit = 0;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE |
+                     AR_S | AR_G | AR_AVL);
+
+       desc.seg_32bit = 1;
+       desc.contents = 0;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RWDATA |
+                     AR_S | AR_DB | AR_G | AR_AVL);
+
+       desc.read_exec_only = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RODATA |
+                     AR_S | AR_DB | AR_G | AR_AVL);
+
+       desc.contents = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RODATA_EXPDOWN |
+                     AR_S | AR_DB | AR_G | AR_AVL);
+
+       desc.read_exec_only = 0;
+       desc.limit_in_pages = 0;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RWDATA_EXPDOWN |
+                     AR_S | AR_DB | AR_AVL);
+
+       desc.contents = 3;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE_CONF |
+                     AR_S | AR_DB | AR_AVL);
+
+       desc.read_exec_only = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XOCODE_CONF |
+                     AR_S | AR_DB | AR_AVL);
+
+       desc.read_exec_only = 0;
+       desc.contents = 2;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XRCODE |
+                     AR_S | AR_DB | AR_AVL);
+
+       desc.read_exec_only = 1;
+
+#ifdef __x86_64__
+       desc.lm = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_XOCODE |
+                     AR_S | AR_DB | AR_AVL);
+       desc.lm = 0;
+#endif
+
+       bool entry1_okay = install_valid(&desc, AR_DPL3 | AR_TYPE_XOCODE |
+                                        AR_S | AR_DB | AR_AVL);
+
+       if (entry1_okay) {
+               printf("[RUN]\tTest fork\n");
+               pid_t child = fork();
+               if (child == 0) {
+                       nerrs = 0;
+                       check_valid_segment(desc.entry_number, 1,
+                                           AR_DPL3 | AR_TYPE_XOCODE |
+                                           AR_S | AR_DB | AR_AVL, desc.limit,
+                                           true);
+                       check_invalid_segment(1, 1);
+                       exit(nerrs ? 1 : 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]\tTest size\n");
+               int i;
+               for (i = 0; i < 8192; i++) {
+                       desc.entry_number = i;
+                       desc.limit = i;
+                       if (safe_modify_ldt(0x11, &desc, sizeof(desc)) != 0) {
+                               printf("[FAIL]\tFailed to install entry %d\n", i);
+                               nerrs++;
+                               break;
+                       }
+               }
+               for (int j = 0; j < i; j++) {
+                       check_valid_segment(j, 1, AR_DPL3 | AR_TYPE_XOCODE |
+                                           AR_S | AR_DB | AR_AVL, j, false);
+               }
+               printf("[DONE]\tSize test\n");
+       } else {
+               printf("[SKIP]\tSkipping fork and size tests because we have no LDT\n");
+       }
+
+       /* Test entry_number too high. */
+       desc.entry_number = 8192;
+       fail_install(&desc);
+
+       /* Test deletion and actions mistakeable for deletion. */
+       memset(&desc, 0, sizeof(desc));
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RWDATA | AR_S | AR_P);
+
+       desc.seg_not_present = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RWDATA | AR_S);
+
+       desc.seg_not_present = 0;
+       desc.read_exec_only = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RODATA | AR_S | AR_P);
+
+       desc.read_exec_only = 0;
+       desc.seg_not_present = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RWDATA | AR_S);
+
+       desc.read_exec_only = 1;
+       desc.limit = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RODATA | AR_S);
+
+       desc.limit = 0;
+       desc.base_addr = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RODATA | AR_S);
+
+       desc.base_addr = 0;
+       install_invalid(&desc, false);
+
+       desc.seg_not_present = 0;
+       desc.read_exec_only = 0;
+       desc.seg_32bit = 1;
+       install_valid(&desc, AR_DPL3 | AR_TYPE_RWDATA | AR_S | AR_P | AR_DB);
+       install_invalid(&desc, true);
+}
+
+/*
+ * 0: thread is idle
+ * 1: thread armed
+ * 2: thread should clear LDT entry 0
+ * 3: thread should exit
+ */
+static volatile unsigned int ftx;
+
+static void *threadproc(void *ctx)
+{
+       cpu_set_t cpuset;
+       CPU_ZERO(&cpuset);
+       CPU_SET(1, &cpuset);
+       if (sched_setaffinity(0, sizeof(cpuset), &cpuset) != 0)
+               err(1, "sched_setaffinity to CPU 1");   /* should never fail */
+
+       while (1) {
+               syscall(SYS_futex, &ftx, FUTEX_WAIT, 0, NULL, NULL, 0);
+               while (ftx != 2) {
+                       if (ftx >= 3)
+                               return NULL;
+               }
+
+               /* clear LDT entry 0 */
+               const struct user_desc desc = {};
+               if (syscall(SYS_modify_ldt, 1, &desc, sizeof(desc)) != 0)
+                       err(1, "modify_ldt");
+
+               /* If ftx == 2, set it to zero.  If ftx == 100, quit. */
+               unsigned int x = -2;
+               asm volatile ("lock xaddl %[x], %[ftx]" :
+                             [x] "+r" (x), [ftx] "+m" (ftx));
+               if (x != 2)
+                       return NULL;
+       }
+}
+
+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 *info, void *ctx_void)
+{
+       siglongjmp(jmpbuf, 1);
+}
+
+static void do_multicpu_tests(void)
+{
+       cpu_set_t cpuset;
+       pthread_t thread;
+       int failures = 0, iters = 5, i;
+       unsigned short orig_ss;
+
+       CPU_ZERO(&cpuset);
+       CPU_SET(1, &cpuset);
+       if (sched_setaffinity(0, sizeof(cpuset), &cpuset) != 0) {
+               printf("[SKIP]\tCannot set affinity to CPU 1\n");
+               return;
+       }
+
+       CPU_ZERO(&cpuset);
+       CPU_SET(0, &cpuset);
+       if (sched_setaffinity(0, sizeof(cpuset), &cpuset) != 0) {
+               printf("[SKIP]\tCannot set affinity to CPU 0\n");
+               return;
+       }
+
+       sethandler(SIGSEGV, sigsegv, 0);
+#ifdef __i386__
+       /* True 32-bit kernels send SIGILL instead of SIGSEGV on IRET faults. */
+       sethandler(SIGILL, sigsegv, 0);
+#endif
+
+       printf("[RUN]\tCross-CPU LDT invalidation\n");
+
+       if (pthread_create(&thread, 0, threadproc, 0) != 0)
+               err(1, "pthread_create");
+
+       asm volatile ("mov %%ss, %0" : "=rm" (orig_ss));
+
+       for (i = 0; i < 5; i++) {
+               if (sigsetjmp(jmpbuf, 1) != 0)
+                       continue;
+
+               /* Make sure the thread is ready after the last test. */
+               while (ftx != 0)
+                       ;
+
+               struct user_desc desc = {
+                       .entry_number    = 0,
+                       .base_addr       = 0,
+                       .limit           = 0xfffff,
+                       .seg_32bit       = 1,
+                       .contents        = 0, /* Data */
+                       .read_exec_only  = 0,
+                       .limit_in_pages  = 1,
+                       .seg_not_present = 0,
+                       .useable         = 0
+               };
+
+               if (safe_modify_ldt(0x11, &desc, sizeof(desc)) != 0) {
+                       if (errno != ENOSYS)
+                               err(1, "modify_ldt");
+                       printf("[SKIP]\tmodify_ldt unavailable\n");
+                       break;
+               }
+
+               /* Arm the thread. */
+               ftx = 1;
+               syscall(SYS_futex, &ftx, FUTEX_WAKE, 0, NULL, NULL, 0);
+
+               asm volatile ("mov %0, %%ss" : : "r" (0x7));
+
+               /* Go! */
+               ftx = 2;
+
+               while (ftx != 0)
+                       ;
+
+               /*
+                * On success, modify_ldt will segfault us synchronously,
+                * and we'll escape via siglongjmp.
+                */
+
+               failures++;
+               asm volatile ("mov %0, %%ss" : : "rm" (orig_ss));
+       };
+
+       ftx = 100;  /* Kill the thread. */
+       syscall(SYS_futex, &ftx, FUTEX_WAKE, 0, NULL, NULL, 0);
+
+       if (pthread_join(thread, NULL) != 0)
+               err(1, "pthread_join");
+
+       if (failures) {
+               printf("[FAIL]\t%d of %d iterations failed\n", failures, iters);
+               nerrs++;
+       } else {
+               printf("[OK]\tAll %d iterations succeeded\n", iters);
+       }
+}
+
+int main()
+{
+       do_simple_tests();
+
+       do_multicpu_tests();
+
+       return nerrs ? 1 : 0;
+}
--

2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

The modify_ldt syscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/Kconfig                   | 17 +++++++++++++++++
 arch/x86/include/asm/mmu.h         |  2 ++
 arch/x86/include/asm/mmu_context.h | 31 +++++++++++++++++++++++--------
 arch/x86/kernel/Makefile           |  3 ++-
 arch/x86/kernel/cpu/perf_event.c   |  4 ++++
 arch/x86/kernel/process_64.c       |  2 ++
 arch/x86/kernel/step.c             |  2 ++
 kernel/sys_ni.c                    |  1 +
 8 files changed, 53 insertions(+), 9 deletions(-)

arch/x86/Kconfig +1 -0

index b3a1a5d77d92..ede52be845db 100644
@@ -1015,6 +1015,7 @@ config VM86
 config X86_16BIT
        bool "Enable support for 16-bit segments" if EXPERT
        default y
+       depends on MODIFY_LDT_SYSCALL
        ---help---
          This option is required by programs like Wine to run 16-bit
          protected mode legacy code on x86 processors.  Disabling

arch/x86/Kconfig +11 -0

+        Local Descriptor Table (LDT) using the modify_ldt(2) system
+        call.  This is required to run 16-bit or segmented code such as
+        DOSEMU or some Wine programs.  It is also used by some very old
+        threading libraries.
+
+        Enabling this feature adds a small amount of overhead to
+        context switches and increases the low-level kernel attack
+        surface.  Disabling it removes the modify_ldt(2) system call.
+
+        Saying 'N' here may make sense for embedded or server kernels.
+
 source "kernel/livepatch/Kconfig"
 
 endmenu

arch/x86/include/asm/mmu.h +2 -0

index 364d27481a52..55234d5e7160 100644
@@ -9,7 +9,9 @@
  * we put the segment information here.
  */
 typedef struct {
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
        struct ldt_struct *ldt;
+#endif
 
 #ifdef CONFIG_X86_64
        /* True if mm supports a task running in 32 bit compatibility mode. */

arch/x86/include/asm/mmu_context.h +23 -8

index 3fcff70c398e..08094eded318 100644
@@ -33,6 +33,7 @@ static inline void load_mm_cr4(struct mm_struct *mm)
 static inline void load_mm_cr4(struct mm_struct *mm) {}
 #endif
 
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
 /*
  * ldt_structs can be allocated, used, and freed, but they are never
  * modified while live.

@@ -48,10 +49,24 @@ struct ldt_struct {
        int size;
 };
 
+/*
+ * Used for LDT copy/destruction.
+ */
+int init_new_context(struct task_struct *tsk, struct mm_struct *mm);
+void destroy_context(struct mm_struct *mm);
+#else  /* CONFIG_MODIFY_LDT_SYSCALL */
+static inline int init_new_context(struct task_struct *tsk,
+                                  struct mm_struct *mm)
+{
+       return 0;
+}
+static inline void destroy_context(struct mm_struct *mm) {}
+#endif
+
 static inline void load_mm_ldt(struct mm_struct *mm)
 {
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
        struct ldt_struct *ldt;
-       DEBUG_LOCKS_WARN_ON(!irqs_disabled());
 
        /* lockless_dereference synchronizes with smp_store_release */
        ldt = lockless_dereference(mm->context.ldt);

@@ -74,14 +89,12 @@ static inline void load_mm_ldt(struct mm_struct *mm)
                set_ldt(ldt->entries, ldt->size);
        else
                clear_LDT();
-}
-
-/*
- * Used for LDT copy/destruction.
- */
-int init_new_context(struct task_struct *tsk, struct mm_struct *mm);
-void destroy_context(struct mm_struct *mm);
+#else
+       clear_LDT();
+#endif
 
+       DEBUG_LOCKS_WARN_ON(!irqs_disabled());
+}
 
 static inline void enter_lazy_tlb(struct mm_struct *mm, struct task_struct *tsk)
 {

@@ -113,6 +126,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
                /* Load per-mm CR4 state */
                load_mm_cr4(next);
 
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
                /*
                 * Load the LDT, if the LDT is different.
                 *

@@ -127,6 +141,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
                 */
                if (unlikely(prev->context.ldt != next->context.ldt))
                        load_mm_ldt(next);
+#endif
        }
 #ifdef CONFIG_SMP
          else {

arch/x86/kernel/Makefile +2 -1

index 0f15af41bd80..2b507befcd3f 100644
@@ -24,7 +24,8 @@ CFLAGS_irq.o := -I$(src)/../include/asm/trace
 
 obj-y                  := process_$(BITS).o signal.o
 obj-y                  += traps.o irq.o irq_$(BITS).o dumpstack_$(BITS).o
-obj-y                  += time.o ioport.o ldt.o dumpstack.o nmi.o
+obj-y                  += time.o ioport.o dumpstack.o nmi.o
+obj-$(CONFIG_MODIFY_LDT_SYSCALL)       += ldt.o
 obj-y                  += setup.o x86_init.o i8259.o irqinit.o jump_label.o
 obj-$(CONFIG_IRQ_WORK)  += irq_work.o
 obj-y                  += probe_roms.o

arch/x86/kernel/cpu/perf_event.c +4 -0

index 9469dfa55607..58b872ef2329 100644
@@ -2179,6 +2179,7 @@ static unsigned long get_segment_base(unsigned int segment)
        int idx = segment >> 3;
 
        if ((segment & SEGMENT_TI_MASK) == SEGMENT_LDT) {
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
                struct ldt_struct *ldt;
 
                if (idx > LDT_ENTRIES)

@@ -2190,6 +2191,9 @@ static unsigned long get_segment_base(unsigned int segment)
                        return 0;
 
                desc = &ldt->entries[idx];
+#else
+               return 0;
+#endif
        } else {
                if (idx > GDT_ENTRIES)
                        return 0;

arch/x86/kernel/process_64.c +2 -0

index f6b916387590..941295ddf802 100644
@@ -121,6 +121,7 @@ void __show_regs(struct pt_regs *regs, int all)
 void release_thread(struct task_struct *dead_task)
 {
        if (dead_task->mm) {
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
                if (dead_task->mm->context.ldt) {
                        pr_warn("WARNING: dead process %s still has LDT? <%p/%d>\n",
                                dead_task->comm,

@@ -128,6 +129,7 @@ void release_thread(struct task_struct *dead_task)
                                dead_task->mm->context.ldt->size);
                        BUG();
                }
+#endif
        }
 }
 

arch/x86/kernel/step.c +2 -0

index 6273324186ac..fd88e152d584 100644
@@ -18,6 +18,7 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *re
                return addr;
        }
 
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
        /*
         * We'll assume that the code segments in the GDT
         * are all zero-based. That is largely true: the

@@ -45,6 +46,7 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *re
                }
                mutex_unlock(&child->mm->context.lock);
        }
+#endif
 
        return addr;
 }

kernel/sys_ni.c +1 -1

index 7995ef5868d8..ca7d84f438f1 100644
@@ -140,6 +140,7 @@ cond_syscall(sys_sgetmask);
 cond_syscall(sys_ssetmask);
 cond_syscall(sys_vm86old);
 cond_syscall(sys_vm86);
+cond_syscall(sys_modify_ldt);
 cond_syscall(sys_ipc);
 cond_syscall(compat_sys_ipc);
 cond_syscall(compat_sys_sysctl);
--

2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

The modify_ldt syscall exposes a large attack surface and is unnecessary for modern userspace. Make it optional.

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/Kconfig                   | 17 +++++++++++++++++
 arch/x86/include/asm/mmu.h         |  2 ++
 arch/x86/include/asm/mmu_context.h | 31 +++++++++++++++++++++++--------
 arch/x86/kernel/Makefile           |  3 ++-
 arch/x86/kernel/cpu/perf_event.c   |  4 ++++
 arch/x86/kernel/process_64.c       |  2 ++
 arch/x86/kernel/step.c             |  2 ++
 kernel/sys_ni.c                    |  1 +
 8 files changed, 53 insertions(+), 9 deletions(-)

arch/x86/Kconfig +1 -0

index b3a1a5d77d92..ede52be845db 100644
@@ -1015,6 +1015,7 @@ config VM86
 config X86_16BIT
        bool "Enable support for 16-bit segments" if EXPERT
        default y
+       depends on MODIFY_LDT_SYSCALL
        ---help---
          This option is required by programs like Wine to run 16-bit
          protected mode legacy code on x86 processors.  Disabling

arch/x86/Kconfig +11 -0

+        Local Descriptor Table (LDT) using the modify_ldt(2) system
+        call.  This is required to run 16-bit or segmented code such as
+        DOSEMU or some Wine programs.  It is also used by some very old
+        threading libraries.
+
+        Enabling this feature adds a small amount of overhead to
+        context switches and increases the low-level kernel attack
+        surface.  Disabling it removes the modify_ldt(2) system call.
+
+        Saying 'N' here may make sense for embedded or server kernels.
+
 source "kernel/livepatch/Kconfig"
 
 endmenu

arch/x86/include/asm/mmu.h +2 -0

index 364d27481a52..55234d5e7160 100644
@@ -9,7 +9,9 @@
  * we put the segment information here.
  */
 typedef struct {
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
        struct ldt_struct *ldt;
+#endif
 
 #ifdef CONFIG_X86_64
        /* True if mm supports a task running in 32 bit compatibility mode. */

arch/x86/include/asm/mmu_context.h +23 -8

index 3fcff70c398e..08094eded318 100644
@@ -33,6 +33,7 @@ static inline void load_mm_cr4(struct mm_struct *mm)
 static inline void load_mm_cr4(struct mm_struct *mm) {}
 #endif
 
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
 /*
  * ldt_structs can be allocated, used, and freed, but they are never
  * modified while live.

@@ -48,10 +49,24 @@ struct ldt_struct {
        int size;
 };
 
+/*
+ * Used for LDT copy/destruction.
+ */
+int init_new_context(struct task_struct *tsk, struct mm_struct *mm);
+void destroy_context(struct mm_struct *mm);
+#else  /* CONFIG_MODIFY_LDT_SYSCALL */
+static inline int init_new_context(struct task_struct *tsk,
+                                  struct mm_struct *mm)
+{
+       return 0;
+}
+static inline void destroy_context(struct mm_struct *mm) {}
+#endif
+
 static inline void load_mm_ldt(struct mm_struct *mm)
 {
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
        struct ldt_struct *ldt;
-       DEBUG_LOCKS_WARN_ON(!irqs_disabled());
 
        /* lockless_dereference synchronizes with smp_store_release */
        ldt = lockless_dereference(mm->context.ldt);

@@ -74,14 +89,12 @@ static inline void load_mm_ldt(struct mm_struct *mm)
                set_ldt(ldt->entries, ldt->size);
        else
                clear_LDT();
-}
-
-/*
- * Used for LDT copy/destruction.
- */
-int init_new_context(struct task_struct *tsk, struct mm_struct *mm);
-void destroy_context(struct mm_struct *mm);
+#else
+       clear_LDT();
+#endif
 
+       DEBUG_LOCKS_WARN_ON(!irqs_disabled());
+}
 
 static inline void enter_lazy_tlb(struct mm_struct *mm, struct task_struct *tsk)
 {

@@ -113,6 +126,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
                /* Load per-mm CR4 state */
                load_mm_cr4(next);
 
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
                /*
                 * Load the LDT, if the LDT is different.
                 *

@@ -127,6 +141,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
                 */
                if (unlikely(prev->context.ldt != next->context.ldt))
                        load_mm_ldt(next);
+#endif
        }
 #ifdef CONFIG_SMP
          else {

arch/x86/kernel/Makefile +2 -1

index 0f15af41bd80..2b507befcd3f 100644
@@ -24,7 +24,8 @@ CFLAGS_irq.o := -I$(src)/../include/asm/trace
 
 obj-y                  := process_$(BITS).o signal.o
 obj-y                  += traps.o irq.o irq_$(BITS).o dumpstack_$(BITS).o
-obj-y                  += time.o ioport.o ldt.o dumpstack.o nmi.o
+obj-y                  += time.o ioport.o dumpstack.o nmi.o
+obj-$(CONFIG_MODIFY_LDT_SYSCALL)       += ldt.o
 obj-y                  += setup.o x86_init.o i8259.o irqinit.o jump_label.o
 obj-$(CONFIG_IRQ_WORK)  += irq_work.o
 obj-y                  += probe_roms.o

arch/x86/kernel/cpu/perf_event.c +4 -0

index 9469dfa55607..58b872ef2329 100644
@@ -2179,6 +2179,7 @@ static unsigned long get_segment_base(unsigned int segment)
        int idx = segment >> 3;
 
        if ((segment & SEGMENT_TI_MASK) == SEGMENT_LDT) {
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
                struct ldt_struct *ldt;
 
                if (idx > LDT_ENTRIES)

@@ -2190,6 +2191,9 @@ static unsigned long get_segment_base(unsigned int segment)
                        return 0;
 
                desc = &ldt->entries[idx];
+#else
+               return 0;
+#endif
        } else {
                if (idx > GDT_ENTRIES)
                        return 0;

arch/x86/kernel/process_64.c +2 -0

index f6b916387590..941295ddf802 100644
@@ -121,6 +121,7 @@ void __show_regs(struct pt_regs *regs, int all)
 void release_thread(struct task_struct *dead_task)
 {
        if (dead_task->mm) {
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
                if (dead_task->mm->context.ldt) {
                        pr_warn("WARNING: dead process %s still has LDT? <%p/%d>\n",
                                dead_task->comm,

@@ -128,6 +129,7 @@ void release_thread(struct task_struct *dead_task)
                                dead_task->mm->context.ldt->size);
                        BUG();
                }
+#endif
        }
 }
 

arch/x86/kernel/step.c +2 -0

index 6273324186ac..fd88e152d584 100644
@@ -18,6 +18,7 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *re
                return addr;
        }
 
+#ifdef CONFIG_MODIFY_LDT_SYSCALL
        /*
         * We'll assume that the code segments in the GDT
         * are all zero-based. That is largely true: the

@@ -45,6 +46,7 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *re
                }
                mutex_unlock(&child->mm->context.lock);
        }
+#endif
 
        return addr;
 }

kernel/sys_ni.c +1 -1

index 7995ef5868d8..ca7d84f438f1 100644
@@ -140,6 +140,7 @@ cond_syscall(sys_sgetmask);
 cond_syscall(sys_ssetmask);
 cond_syscall(sys_vm86old);
 cond_syscall(sys_vm86);
+cond_syscall(sys_modify_ldt);
 cond_syscall(sys_ipc);
 cond_syscall(compat_sys_ipc);
 cond_syscall(compat_sys_sysctl);
--

2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

I look forward to the runtime disabling patch. :)

I look forward to the runtime disabling patch. :)

Hi Kees,

Quoted 1 line · Kees Cook

I look forward to the runtime disabling patch. :)

Did you get my response to your comments regarding the proposed patch ?

I can rebase it and update it if needed, I just want to make sure everyone's on the same line regarding this.

Hi Kees,

Quoted 1 line · Kees Cook

I look forward to the runtime disabling patch. :)

Did you get my response to your comments regarding the proposed patch ?

I can rebase it and update it if needed, I just want to make sure everyone's on the same line regarding this.

Quoted 5 lines · Willy Tarreau

Hi Kees,

Quoted 1 line · Kees Cook

I look forward to the runtime disabling patch. :)

Quoted 4 lines

Did you get my response to your comments regarding the proposed patch ?

I can rebase it and update it if needed, I just want to make sure everyone's on the same line regarding this.

Yeah, I'm fine with what you have. I'd still like a "off until next reboot", but I'll live. :)

Quoted 5 lines · Willy Tarreau

Hi Kees,

Quoted 1 line · Kees Cook

I look forward to the runtime disabling patch. :)

Quoted 4 lines

Did you get my response to your comments regarding the proposed patch ?

I can rebase it and update it if needed, I just want to make sure everyone's on the same line regarding this.

Yeah, I'm fine with what you have. I'd still like a "off until next reboot", but I'll live. :)

This is intended for x86/urgent. Sorry for taking so long, but it seemed nice to avoid breaking Xen.

This fixes the "dazed and confused" issue which was exposed by the CVE-2015-5157 fix. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Boris, could I get a Tested-by, assuming this works for you?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Changes from v4:
- Fix Xen even better (patch 1 is new).
- Reorder the patches to make a little more sense.

Changes from v3:
- Hopefully fixed Xen.
- Fixed 32-bit test case on 32-bit native kernel.
- Fix bogus vumnap for some LDT sizes.
- Strengthen test case to check all LDT sizes (catches bogus vunmap).
- Lots of cleanups, mostly from Borislav.
- Simplify IPI code using on_each_cpu_mask.

Changes from v2:
- Allocate ldt_struct and the LDT entries separately. This should fix Xen.
- Stop using write_ldt_entry, since I'm pretty sure it's unnecessary now that we no longer mutate an in-use LDT. (Xen people, can you check?)

Changes from v1:
- The config option is new.
- The test case is new.
- Fixed a missing allocation failure check.
- Fixed a use-after-free on fork().

Andy Lutomirski (4):
x86/xen: Unmap aliases in xen_alloc_ldt and xen_free_ldt x86/ldt: Make modify_ldt synchronous selftests/x86, x86/ldt: Add a selftest for modify_ldt x86/ldt: Make modify_ldt optional

  arch/x86/Kconfig                      |  17 ++
  arch/x86/include/asm/desc.h           |  15 -
  arch/x86/include/asm/mmu.h            |   5 +-
  arch/x86/include/asm/mmu_context.h    |  68 ++++-
  arch/x86/kernel/Makefile              |   3 +-
  arch/x86/kernel/cpu/common.c          |   4 +-
  arch/x86/kernel/cpu/perf_event.c      |  16 +-
  arch/x86/kernel/ldt.c                 | 262 +++++++++--------
  arch/x86/kernel/process_64.c          |   6 +-
  arch/x86/kernel/step.c                |   8 +-
  arch/x86/power/cpu.c                  |   3 +-
  arch/x86/xen/enlighten.c              |  12 +
  kernel/sys_ni.c                       |   1 +
  tools/testing/selftests/x86/Makefile  |   2 +-
  tools/testing/selftests/x86/ldt_gdt.c | 520 ++++++++++++++++++++++++++++++++++
  15 files changed, 787 insertions(+), 155 deletions(-)

create mode 100644 tools/testing/selftests/x86/ldt_gdt.c

This is intended for x86/urgent. Sorry for taking so long, but it seemed nice to avoid breaking Xen.

This fixes the "dazed and confused" issue which was exposed by the CVE-2015-5157 fix. It's also probably a good general attack surface reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt. This makes that possible.

Boris, could I get a Tested-by, assuming this works for you?

Willy and Kees: I left the config option alone. The -tiny people will like it, and we can always add a sysctl of some sort later.

Changes from v4:
- Fix Xen even better (patch 1 is new).
- Reorder the patches to make a little more sense.

Changes from v3:
- Hopefully fixed Xen.
- Fixed 32-bit test case on 32-bit native kernel.
- Fix bogus vumnap for some LDT sizes.
- Strengthen test case to check all LDT sizes (catches bogus vunmap).
- Lots of cleanups, mostly from Borislav.
- Simplify IPI code using on_each_cpu_mask.

Changes from v2:
- Allocate ldt_struct and the LDT entries separately. This should fix Xen.
- Stop using write_ldt_entry, since I'm pretty sure it's unnecessary now that we no longer mutate an in-use LDT. (Xen people, can you check?)

Changes from v1:
- The config option is new.
- The test case is new.
- Fixed a missing allocation failure check.
- Fixed a use-after-free on fork().

Andy Lutomirski (4):
x86/xen: Unmap aliases in xen_alloc_ldt and xen_free_ldt x86/ldt: Make modify_ldt synchronous selftests/x86, x86/ldt: Add a selftest for modify_ldt x86/ldt: Make modify_ldt optional

  arch/x86/Kconfig                      |  17 ++
  arch/x86/include/asm/desc.h           |  15 -
  arch/x86/include/asm/mmu.h            |   5 +-
  arch/x86/include/asm/mmu_context.h    |  68 ++++-
  arch/x86/kernel/Makefile              |   3 +-
  arch/x86/kernel/cpu/common.c          |   4 +-
  arch/x86/kernel/cpu/perf_event.c      |  16 +-
  arch/x86/kernel/ldt.c                 | 262 +++++++++--------
  arch/x86/kernel/process_64.c          |   6 +-
  arch/x86/kernel/step.c                |   8 +-
  arch/x86/power/cpu.c                  |   3 +-
  arch/x86/xen/enlighten.c              |  12 +
  kernel/sys_ni.c                       |   1 +
  tools/testing/selftests/x86/Makefile  |   2 +-
  tools/testing/selftests/x86/ldt_gdt.c | 520 ++++++++++++++++++++++++++++++++++
  15 files changed, 787 insertions(+), 155 deletions(-)

create mode 100644 tools/testing/selftests/x86/ldt_gdt.c

As far as Xen guests are concerned,

Tested-by Boris Ostrovsky <[email protected]>

As far as Xen guests are concerned,

Tested-by Boris Ostrovsky <[email protected]>

Quoted 3 lines · Boris Ostrovsky

As far as Xen guests are concerned,

Tested-by Boris Ostrovsky <[email protected]>

Does that mean, this patch 1/4 fixes the 32bit issue you guys are still debugging on the v4 thread? Or does that need more fixing?

Quoted 3 lines · Borislav Petkov

Quoted 2 lines · Boris Ostrovsky

As far as Xen guests are concerned,

Quoted 4 lines

Quoted 1 line

Tested-by Boris Ostrovsky <[email protected]>

Does that mean, this patch 1/4 fixes the 32bit issue you guys are still debugging on the v4 thread? Or does that need more fixing?

I was going to say... This v5 pre-dates figuring out what was wrong with 32bit Xen. v5 1/4 is still susceptible.

Boris: does your Tested-by cover v5 + proposed fix?

Quoted 3 lines · Borislav Petkov

Quoted 2 lines · Boris Ostrovsky

As far as Xen guests are concerned,

Quoted 4 lines

Quoted 1 line

Tested-by Boris Ostrovsky <[email protected]>

Does that mean, this patch 1/4 fixes the 32bit issue you guys are still debugging on the v4 thread? Or does that need more fixing?

I was going to say... This v5 pre-dates figuring out what was wrong with 32bit Xen. v5 1/4 is still susceptible.

Boris: does your Tested-by cover v5 + proposed fix?

Quoted 8 lines · Andrew Cooper

Quoted 7 lines · Borislav Petkov

Quoted 3 lines · Boris Ostrovsky

As far as Xen guests are concerned,

Tested-by Boris Ostrovsky <[email protected]>

Does that mean, this patch 1/4 fixes the 32bit issue you guys are still debugging on the v4 thread? Or does that need more fixing?

Quoted 5 lines

I was going to say... This v5 pre-dates figuring out what was wrong with 32bit Xen. v5 1/4 is still susceptible.

Boris: does your Tested-by cover v5 + proposed fix?

Only V5, no extra changes.

And perhaps dropping aliases in xen_alloc_ldt() may be sufficient since with that done we will only have one mapping so a subsequent fault will have "correct" cr2 provided by the hypervisor (from your earlier email it sounded that hypervisor may have been providing incorrect cr2 if alias exists)

Quoted 8 lines · Andrew Cooper

Quoted 7 lines · Borislav Petkov

Quoted 3 lines · Boris Ostrovsky

As far as Xen guests are concerned,

Tested-by Boris Ostrovsky <[email protected]>

Does that mean, this patch 1/4 fixes the 32bit issue you guys are still debugging on the v4 thread? Or does that need more fixing?

Quoted 5 lines

I was going to say... This v5 pre-dates figuring out what was wrong with 32bit Xen. v5 1/4 is still susceptible.

Boris: does your Tested-by cover v5 + proposed fix?

Only V5, no extra changes.

And perhaps dropping aliases in xen_alloc_ldt() may be sufficient since with that done we will only have one mapping so a subsequent fault will have "correct" cr2 provided by the hypervisor (from your earlier email it sounded that hypervisor may have been providing incorrect cr2 if alias exists)

Quoted 9 lines · Boris Ostrovsky

Quoted 8 lines · Andrew Cooper

Quoted 7 lines · Borislav Petkov

Quoted 3 lines · Boris Ostrovsky

As far as Xen guests are concerned,

Tested-by Boris Ostrovsky <[email protected]>

Does that mean, this patch 1/4 fixes the 32bit issue you guys are still debugging on the v4 thread? Or does that need more fixing?

Quoted 6 lines

Quoted 5 lines

I was going to say... This v5 pre-dates figuring out what was wrong with 32bit Xen. v5 1/4 is still susceptible.

Boris: does your Tested-by cover v5 + proposed fix?

Only V5, no extra changes.

Including running the ldt_gdt test?

Quoted 6 lines

And perhaps dropping aliases in xen_alloc_ldt() may be sufficient since with that done we will only have one mapping so a subsequent fault will have "correct" cr2 provided by the hypervisor (from your earlier email it sounded that hypervisor may have been providing incorrect cr2 if alias exists)

They are sufficient to fix the first of the two bugs, but the free side still has no protection against a missing l2, unless I am missing something in the rest of the series?

Quoted 9 lines · Boris Ostrovsky

Quoted 8 lines · Andrew Cooper

Quoted 7 lines · Borislav Petkov

Quoted 3 lines · Boris Ostrovsky

As far as Xen guests are concerned,

Tested-by Boris Ostrovsky <[email protected]>

Does that mean, this patch 1/4 fixes the 32bit issue you guys are still debugging on the v4 thread? Or does that need more fixing?

Quoted 6 lines

Quoted 5 lines

I was going to say... This v5 pre-dates figuring out what was wrong with 32bit Xen. v5 1/4 is still susceptible.

Boris: does your Tested-by cover v5 + proposed fix?

Only V5, no extra changes.

Including running the ldt_gdt test?

Quoted 6 lines

And perhaps dropping aliases in xen_alloc_ldt() may be sufficient since with that done we will only have one mapping so a subsequent fault will have "correct" cr2 provided by the hypervisor (from your earlier email it sounded that hypervisor may have been providing incorrect cr2 if alias exists)

They are sufficient to fix the first of the two bugs, but the free side still has no protection against a missing l2, unless I am missing something in the rest of the series?

Quoted 2 lines

Quoted 1 line

Only V5, no extra changes.

Including running the ldt_gdt test?

Yes, except that 32-on-64 doesn't work, but that's not Xen-specific.

Quoted 2 lines

Quoted 1 line

Only V5, no extra changes.

Including running the ldt_gdt test?

Yes, except that 32-on-64 doesn't work, but that's not Xen-specific.

Hey Boris,

Quoted 3 lines · Boris Ostrovsky

Quoted 2 lines

Quoted 1 line

Only V5, no extra changes.

Including running the ldt_gdt test?

Yes, except that 32-on-64 doesn't work, but that's not Xen-specific.

so which tests are you running exactly and where can I get them? Andy's repo?

Hey Boris,

Quoted 3 lines · Boris Ostrovsky

Quoted 2 lines

Quoted 1 line

Only V5, no extra changes.

Including running the ldt_gdt test?

Yes, except that 32-on-64 doesn't work, but that's not Xen-specific.

so which tests are you running exactly and where can I get them? Andy's repo?