OS-6906 LX not setting "accessed" bit in GDT, KPTI sadness ensues

Reviewed by: John Levon <john.levon@joyent.com>
Reviewed by: Robert Mustacchi <rm@joyent.com>
Reviewed by: Patrick Mooney <patrick.mooney@joyent.com>
Reviewed by: Dan McDonald <danmcd@joyent.com>
Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Approved by: Patrick Mooney <patrick.mooney@joyent.com>

5 files changed, 54 insertions, 72 deletions

diff --git a/usr/src/uts/common/brand/lx/sys/lx_ldt.h b/usr/src/uts/common/brand/lx/sys/lx_ldt.h
index 825933e86c..08d4d78efb 100644
--- a/usr/src/uts/common/brand/lx/sys/lx_ldt.h
+++ b/usr/src/uts/common/brand/lx/sys/lx_ldt.h
@@ -23,7 +23,7 @@
  * Use is subject to license terms.
  */
 /*
- * Copyright 2014 Joyent, Inc.  All rights reserved.
+ * Copyright 2018 Joyent, Inc.  All rights reserved.
  */
 
 #ifndef _SYS_LINUX_LDT_H
@@ -63,7 +63,7 @@ struct ldt_info {
 	USEGD_SETBASE(desc, (info)->base_addr);				\
 	USEGD_SETLIMIT(desc, (info)->limit);				\
 	(desc)->usd_type = ((info)->contents << 2) | 			\
-	    ((info)->read_exec_only ^ 1) << 1 | 0x10;			\
+	    ((info)->read_exec_only ^ 1) << 1 | SDT_S | SDT_A;		\
 	(desc)->usd_dpl = SEL_UPL;					\
 	(desc)->usd_p = (info)->seg_not_present ^ 1;			\
 	(desc)->usd_def32 = (info)->seg_32bit;				\
diff --git a/usr/src/uts/i86pc/sys/machparam.h b/usr/src/uts/i86pc/sys/machparam.h
index 3728f30ca6..f79b582df4 100644
--- a/usr/src/uts/i86pc/sys/machparam.h
+++ b/usr/src/uts/i86pc/sys/machparam.h
@@ -152,7 +152,7 @@ extern "C" {
 #define	GDT_VA		(DEBUG_INFO_VA - MMU_PAGESIZE)
 #define	IDT_VA		(GDT_VA - MMU_PAGESIZE)
 #define	LDT_VA		(IDT_VA - (16 * MMU_PAGESIZE))
-#define	KTSS_VA		(IDT_VA - MMU_PAGESIZE)
+#define	KTSS_VA		(LDT_VA - MMU_PAGESIZE)
 #define	DFTSS_VA	(KTSS_VA - MMU_PAGESIZE)
 #define	MISC_VA_BASE	(DFTSS_VA)
 #define	MISC_VA_SIZE	(KERNEL_TEXT - MISC_VA_BASE)
diff --git a/usr/src/uts/intel/ia32/os/desctbls.c b/usr/src/uts/intel/ia32/os/desctbls.c
index 5ef56b034c..66b14377a6 100644
--- a/usr/src/uts/intel/ia32/os/desctbls.c
+++ b/usr/src/uts/intel/ia32/os/desctbls.c
@@ -178,8 +178,6 @@ static struct interposing_handler brand_tbl[3];
  * can understand.
  */
 
-#if defined(__amd64)
-
 /*
  * In long mode we have the new L or long mode attribute bit
  * for code segments. Only the conforming bit in type is used along
@@ -193,6 +191,8 @@ set_usegd(user_desc_t *dp, uint_t lmode, void *base, size_t size,
     uint_t type, uint_t dpl, uint_t gran, uint_t defopsz)
 {
 	ASSERT(lmode == SDP_SHORT || lmode == SDP_LONG);
+	/* This should never be a "system" segment. */
+	ASSERT3U(type & SDT_S, !=, 0);
 
 	/*
 	 * 64-bit long mode.
@@ -205,6 +205,14 @@ set_usegd(user_desc_t *dp, uint_t lmode, void *base, size_t size,
 		 */
 		dp->usd_def32 = defopsz;	/* 0 = 16, 1 = 32-bit ops */
 
+	/*
+	 * We should always set the "accessed" bit (SDT_A), otherwise the CPU
+	 * will write to the GDT whenever we change segment registers around.
+	 * With KPTI on, the GDT is read-only in the user page table, which
+	 * causes crashes if we don't set this.
+	 */
+	ASSERT3U(type & SDT_A, !=, 0);
+
 	dp->usd_long = lmode;	/* 64-bit mode */
 	dp->usd_type = type;
 	dp->usd_dpl = dpl;
@@ -218,37 +226,10 @@ set_usegd(user_desc_t *dp, uint_t lmode, void *base, size_t size,
 	dp->usd_hilimit = (uintptr_t)size >> 16;
 }
 
-#elif defined(__i386)
-
-/*
- * Install user segment descriptor for code and data.
- */
-void
-set_usegd(user_desc_t *dp, void *base, size_t size, uint_t type,
-    uint_t dpl, uint_t gran, uint_t defopsz)
-{
-	dp->usd_lolimit = size;
-	dp->usd_hilimit = (uintptr_t)size >> 16;
-
-	dp->usd_lobase = (uintptr_t)base;
-	dp->usd_midbase = (uintptr_t)base >> 16;
-	dp->usd_hibase = (uintptr_t)base >> (16 + 8);
-
-	dp->usd_type = type;
-	dp->usd_dpl = dpl;
-	dp->usd_p = 1;
-	dp->usd_def32 = defopsz;	/* 0 = 16, 1 = 32 bit operands */
-	dp->usd_gran = gran;		/* 0 = bytes, 1 = pages */
-}
-
-#endif	/* __i386 */
-
 /*
  * Install system segment descriptor for LDT and TSS segments.
  */
 
-#if defined(__amd64)
-
 void
 set_syssegd(system_desc_t *dp, void *base, size_t size, uint_t type,
     uint_t dpl)
@@ -281,39 +262,6 @@ get_ssd_base(system_desc_t *dp)
 	return ((void *)base);
 }
 
-#elif defined(__i386)
-
-void
-set_syssegd(system_desc_t *dp, void *base, size_t size, uint_t type,
-    uint_t dpl)
-{
-	dp->ssd_lolimit = size;
-	dp->ssd_hilimit = (uintptr_t)size >> 16;
-
-	dp->ssd_lobase = (uintptr_t)base;
-	dp->ssd_midbase = (uintptr_t)base >> 16;
-	dp->ssd_hibase = (uintptr_t)base >> (16 + 8);
-
-	dp->ssd_type = type;
-	dp->ssd_zero = 0;	/* must be zero */
-	dp->ssd_dpl = dpl;
-	dp->ssd_p = 1;
-	dp->ssd_gran = 0;	/* force byte units */
-}
-
-void *
-get_ssd_base(system_desc_t *dp)
-{
-	uintptr_t	base;
-
-	base = (uintptr_t)dp->ssd_lobase |
-	    (uintptr_t)dp->ssd_midbase << 16 |
-	    (uintptr_t)dp->ssd_hibase << (16 + 8);
-	return ((void *)base);
-}
-
-#endif	/* __i386 */
-
 /*
  * Install gate segment descriptor for interrupt, trap, call and task gates.
  *
@@ -391,17 +339,30 @@ set_gatesegd(gate_desc_t *dp, void (*func)(void), selector_t sel,
 void
 gdt_update_usegd(uint_t sidx, user_desc_t *udp)
 {
-#if defined(__xpv)
+#if defined(DEBUG)
+	/* This should never be a "system" segment, but it might be null. */
+	if (udp->usd_p != 0 || udp->usd_type != 0) {
+		ASSERT3U(udp->usd_type & SDT_S, !=, 0);
+	}
+	/*
+	 * We should always set the "accessed" bit (SDT_A), otherwise the CPU
+	 * will write to the GDT whenever we change segment registers around.
+	 * With KPTI on, the GDT is read-only in the user page table, which
+	 * causes crashes if we don't set this.
+	 */
+	if (udp->usd_p != 0 || udp->usd_type != 0) {
+		ASSERT3U(udp->usd_type & SDT_A, !=, 0);
+	}
+#endif
 
+#if defined(__xpv)
 	uint64_t dpa = CPU->cpu_m.mcpu_gdtpa + sizeof (*udp) * sidx;
 
 	if (HYPERVISOR_update_descriptor(pa_to_ma(dpa), *(uint64_t *)udp))
 		panic("gdt_update_usegd: HYPERVISOR_update_descriptor");
 
 #else	/* __xpv */
-
 	CPU->cpu_gdt[sidx] = *udp;
-
 #endif	/* __xpv */
 }
 
@@ -412,8 +373,23 @@ gdt_update_usegd(uint_t sidx, user_desc_t *udp)
 int
 ldt_update_segd(user_desc_t *ldp, user_desc_t *udp)
 {
-#if defined(__xpv)
+#if defined(DEBUG)
+	/* This should never be a "system" segment, but it might be null. */
+	if (udp->usd_p != 0 || udp->usd_type != 0) {
+		ASSERT3U(udp->usd_type & SDT_S, !=, 0);
+	}
+	/*
+	 * We should always set the "accessed" bit (SDT_A), otherwise the CPU
+	 * will write to the LDT whenever we change segment registers around.
+	 * With KPTI on, the LDT is read-only in the user page table, which
+	 * causes crashes if we don't set this.
+	 */
+	if (udp->usd_p != 0 || udp->usd_type != 0) {
+		ASSERT3U(udp->usd_type & SDT_A, !=, 0);
+	}
+#endif
 
+#if defined(__xpv)
 	uint64_t dpa;
 
 	dpa = mmu_ptob(hat_getpfnum(kas.a_hat, (caddr_t)ldp)) |
@@ -427,7 +403,6 @@ ldt_update_segd(user_desc_t *ldp, user_desc_t *udp)
 		return (EINVAL);
 
 #else	/* __xpv */
-
 	*ldp = *udp;
 
 #endif	/* __xpv */
diff --git a/usr/src/uts/intel/ia32/os/sysi86.c b/usr/src/uts/intel/ia32/os/sysi86.c
index cd1129ea1f..f0cba7d7d5 100644
--- a/usr/src/uts/intel/ia32/os/sysi86.c
+++ b/usr/src/uts/intel/ia32/os/sysi86.c
@@ -284,9 +284,12 @@ ssd_to_usd(struct ssd *ssd, user_desc_t *usd)
 	USEGD_SETLIMIT(usd, ssd->ls);
 
 	/*
-	 * set type, dpl and present bits.
+	 * Set type, dpl and present bits.
+	 *
+	 * Force the "accessed" bit to on so that we don't run afoul of
+	 * KPTI.
 	 */
-	usd->usd_type = ssd->acc1;
+	usd->usd_type = ssd->acc1 | SDT_A;
 	usd->usd_dpl = ssd->acc1 >> 5;
 	usd->usd_p = ssd->acc1 >> (5 + 2);
 
diff --git a/usr/src/uts/intel/sys/segments.h b/usr/src/uts/intel/sys/segments.h
index 84eb363f00..6bf18b3082 100644
--- a/usr/src/uts/intel/sys/segments.h
+++ b/usr/src/uts/intel/sys/segments.h
@@ -482,6 +482,10 @@ void init_boot_gdt(user_desc_t *);
 #define	SDT_MEMERC	30	/* execute read conforming */
 #define	SDT_MEMERAC	31	/* execute read accessed conforming */
 
+/* These bits are within the "type" field, like the values above. */
+#define	SDT_A		0x01	/* accessed bit */
+#define	SDT_S		0x10	/* S-bit at the top of "type" for usegs */
+
 /*
  * Entries in the Interrupt Descriptor Table (IDT)
  */