Vulnerability: In arch/x86/lib/insn-eval.c in the Linux kernel before 5.1.9, there is a use-after-free for access to an LDT entry because of a race condition between modify_ldt() and a #BR exception for an MPX bounds violation.
Vulnerability ID: CVE-2019-13233
CWE: CWE-362
Severity score: 6.4
Project: https://github.com/torvalds/linux
Published: 2019-07-04
File: arch/x86/lib/insn-eval.c
Commit message: x86/insn-eval: Fix use-after-free access to LDT entry

get_desc() computes a pointer into the LDT while holding a lock that
protects the LDT from being freed, but then drops the lock and returns the
(now potentially dangling) pointer to its caller.

Fix it by giving the caller a copy of the LDT entry instead.

Fixes: 670f928ba09b ("x86/insn-eval: Add utility function to get segment descriptor")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Commit author: Jann Horn

Vulnerable version (before fix):
 static int get_reg_offset_16(struct insn *insn, struct pt_regs *regs, }  /**  * get_desc() - Obtain pointer to a segment descriptor  * @sel:	Segment selector  *  * Given a segment selector, obtain a pointer to the segment descriptor.  * Both global and local descriptor tables are supported.  *  * Returns:  *  * Pointer to segment descriptor on success.  *  * NULL on error.  */ static struct desc_struct *get_desc(unsigned short sel) { 	struct desc_ptr gdt_desc = {0, 0}; 	unsigned long desc_base;  #ifdef CONFIG_MODIFY_LDT_SYSCALL 	if ((sel & SEGMENT_TI_MASK) == SEGMENT_LDT) { 		struct desc_struct *desc = NULL; 		struct ldt_struct *ldt;  		/* Bits [15:3] contain the index of the desired entry. */ 		sel >>= 3;  		mutex_lock(&current->active_mm->context.lock); 		ldt = current->active_mm->context.ldt; 		if (ldt && sel < ldt->nr_entries) 			desc = &ldt->entries[sel];  		mutex_unlock(&current->active_mm->context.lock);  		return desc; 	} #endif 	native_store_gdt(&gdt_desc); static struct desc_struct *get_desc(unsigned short sel) 	desc_base = sel & ~(SEGMENT_RPL_MASK | SEGMENT_TI_MASK);  	if (desc_base > gdt_desc.size) 		return NULL;  	return (struct desc_struct *)(gdt_desc.address + desc_base); }  /** static struct desc_struct *get_desc(unsigned short sel)  */ unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx) { 	struct desc_struct *desc; 	short sel;  	sel = get_segment_selector(regs, seg_reg_idx); unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx) 	if (!sel) 		return -1L;  	desc = get_desc(sel); 	if (!desc) 		return -1L;  	return get_desc_base(desc); }  /** unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx)  */ static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx) { 	struct desc_struct *desc; 	unsigned long limit; 	short sel;  static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx) 	if (!sel) 		return 0;  	desc = get_desc(sel); 	if (!desc) 		return 0;  	/* static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx) 	 * not tested when checking the segment limits. In practice, 	 * this means that the segment ends in (limit << 12) + 0xfff. 	 */ 	limit = get_desc_limit(desc); 	if (desc->g) 		limit = (limit << 12) + 0xfff;  	return limit; static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)  */ int insn_get_code_seg_params(struct pt_regs *regs) { 	struct desc_struct *desc; 	short sel;  	if (v8086_mode(regs)) int insn_get_code_seg_params(struct pt_regs *regs) 	if (sel < 0) 		return sel;  	desc = get_desc(sel); 	if (!desc) 		return -EINVAL;  	/* 	 * The most significant byte of the Type field of the segment descriptor 	 * determines whether a segment contains data or code. If this is a data 	 * segment, return error. 	 */ 	if (!(desc->type & BIT(3))) 		return -EINVAL;  	switch ((desc->l << 1) | desc->d) { 	case 0: /* 		 * Legacy mode. CS.L=0, CS.D=0. Address and operand size are 		 * both 16-bit.

Fixed version (after fix):
 static int get_reg_offset_16(struct insn *insn, struct pt_regs *regs, }  /**  * get_desc() - Obtain contents of a segment descriptor  * @out:	Segment descriptor contents on success  * @sel:	Segment selector  *  * Given a segment selector, obtain a pointer to the segment descriptor.  * Both global and local descriptor tables are supported.  *  * Returns:  *  * True on success, false on failure.  *  * NULL on error.  */ static bool get_desc(struct desc_struct *out, unsigned short sel) { 	struct desc_ptr gdt_desc = {0, 0}; 	unsigned long desc_base;  #ifdef CONFIG_MODIFY_LDT_SYSCALL 	if ((sel & SEGMENT_TI_MASK) == SEGMENT_LDT) { 		bool success = false; 		struct ldt_struct *ldt;  		/* Bits [15:3] contain the index of the desired entry. */ 		sel >>= 3;  		mutex_lock(&current->active_mm->context.lock); 		ldt = current->active_mm->context.ldt; 		if (ldt && sel < ldt->nr_entries) { 			*out = ldt->entries[sel]; 			success = true; 		}  		mutex_unlock(&current->active_mm->context.lock);  		return success; 	} #endif 	native_store_gdt(&gdt_desc); static struct desc_struct *get_desc(unsigned short sel) 	desc_base = sel & ~(SEGMENT_RPL_MASK | SEGMENT_TI_MASK);  	if (desc_base > gdt_desc.size) 		return false;  	*out = *(struct desc_struct *)(gdt_desc.address + desc_base); 	return true; }  /** static struct desc_struct *get_desc(unsigned short sel)  */ unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx) { 	struct desc_struct desc; 	short sel;  	sel = get_segment_selector(regs, seg_reg_idx); unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx) 	if (!sel) 		return -1L;  	if (!get_desc(&desc, sel)) 		return -1L;  	return get_desc_base(&desc); }  /** unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx)  */ static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx) { 	struct desc_struct desc; 	unsigned long limit; 	short sel;  static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx) 	if (!sel) 		return 0;  	if (!get_desc(&desc, sel)) 		return 0;  	/* static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx) 	 * not tested when checking the segment limits. In practice, 	 * this means that the segment ends in (limit << 12) + 0xfff. 	 */ 	limit = get_desc_limit(&desc); 	if (desc.g) 		limit = (limit << 12) + 0xfff;  	return limit; static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)  */ int insn_get_code_seg_params(struct pt_regs *regs) { 	struct desc_struct desc; 	short sel;  	if (v8086_mode(regs)) int insn_get_code_seg_params(struct pt_regs *regs) 	if (sel < 0) 		return sel;  	if (!get_desc(&desc, sel)) 		return -EINVAL;  	/* 	 * The most significant byte of the Type field of the segment descriptor 	 * determines whether a segment contains data or code. If this is a data 	 * segment, return error. 	 */ 	if (!(desc.type & BIT(3))) 		return -EINVAL;  	switch ((desc.l << 1) | desc.d) { 	case 0: /* 		 * Legacy mode. CS.L=0, CS.D=0. Address and operand size are 		 * both 16-bit.

Patch:
patch: @@ -557,39 +557,42 @@ static int get_reg_offset_16(struct insn *insn, struct pt_regs *regs,
 }
 
 /**
- * get_desc() - Obtain pointer to a segment descriptor
+ * get_desc() - Obtain contents of a segment descriptor
+ * @out:	Segment descriptor contents on success
  * @sel:	Segment selector
  *
  * Given a segment selector, obtain a pointer to the segment descriptor.
  * Both global and local descriptor tables are supported.
  *
  * Returns:
  *
- * Pointer to segment descriptor on success.
+ * True on success, false on failure.
  *
  * NULL on error.
  */
-static struct desc_struct *get_desc(unsigned short sel)
+static bool get_desc(struct desc_struct *out, unsigned short sel)
 {
 	struct desc_ptr gdt_desc = {0, 0};
 	unsigned long desc_base;
 
 #ifdef CONFIG_MODIFY_LDT_SYSCALL
 	if ((sel & SEGMENT_TI_MASK) == SEGMENT_LDT) {
-		struct desc_struct *desc = NULL;
+		bool success = false;
 		struct ldt_struct *ldt;
 
 		/* Bits [15:3] contain the index of the desired entry. */
 		sel >>= 3;
 
 		mutex_lock(&current->active_mm->context.lock);
 		ldt = current->active_mm->context.ldt;
-		if (ldt && sel < ldt->nr_entries)
-			desc = &ldt->entries[sel];
+		if (ldt && sel < ldt->nr_entries) {
+			*out = ldt->entries[sel];
+			success = true;
+		}
 
 		mutex_unlock(&current->active_mm->context.lock);
 
-		return desc;
+		return success;
 	}
 #endif
 	native_store_gdt(&gdt_desc);
@@ -604,9 +607,10 @@ static struct desc_struct *get_desc(unsigned short sel)
 	desc_base = sel & ~(SEGMENT_RPL_MASK | SEGMENT_TI_MASK);
 
 	if (desc_base > gdt_desc.size)
-		return NULL;
+		return false;
 
-	return (struct desc_struct *)(gdt_desc.address + desc_base);
+	*out = *(struct desc_struct *)(gdt_desc.address + desc_base);
+	return true;
 }
 
 /**
@@ -628,7 +632,7 @@ static struct desc_struct *get_desc(unsigned short sel)
  */
 unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx)
 {
-	struct desc_struct *desc;
+	struct desc_struct desc;
 	short sel;
 
 	sel = get_segment_selector(regs, seg_reg_idx);
@@ -666,11 +670,10 @@ unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx)
 	if (!sel)
 		return -1L;
 
-	desc = get_desc(sel);
-	if (!desc)
+	if (!get_desc(&desc, sel))
 		return -1L;
 
-	return get_desc_base(desc);
+	return get_desc_base(&desc);
 }
 
 /**
@@ -692,7 +695,7 @@ unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx)
  */
 static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)
 {
-	struct desc_struct *desc;
+	struct desc_struct desc;
 	unsigned long limit;
 	short sel;
 
@@ -706,8 +709,7 @@ static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)
 	if (!sel)
 		return 0;
 
-	desc = get_desc(sel);
-	if (!desc)
+	if (!get_desc(&desc, sel))
 		return 0;
 
 	/*
@@ -716,8 +718,8 @@ static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)
 	 * not tested when checking the segment limits. In practice,
 	 * this means that the segment ends in (limit << 12) + 0xfff.
 	 */
-	limit = get_desc_limit(desc);
-	if (desc->g)
+	limit = get_desc_limit(&desc);
+	if (desc.g)
 		limit = (limit << 12) + 0xfff;
 
 	return limit;
@@ -741,7 +743,7 @@ static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)
  */
 int insn_get_code_seg_params(struct pt_regs *regs)
 {
-	struct desc_struct *desc;
+	struct desc_struct desc;
 	short sel;
 
 	if (v8086_mode(regs))
@@ -752,19 +754,18 @@ int insn_get_code_seg_params(struct pt_regs *regs)
 	if (sel < 0)
 		return sel;
 
-	desc = get_desc(sel);
-	if (!desc)
+	if (!get_desc(&desc, sel))
 		return -EINVAL;
 
 	/*
 	 * The most significant byte of the Type field of the segment descriptor
 	 * determines whether a segment contains data or code. If this is a data
 	 * segment, return error.
 	 */
-	if (!(desc->type & BIT(3)))
+	if (!(desc.type & BIT(3)))
 		return -EINVAL;
 
-	switch ((desc->l << 1) | desc->d) {
+	switch ((desc.l << 1) | desc.d) {
 	case 0: /*
 		 * Legacy mode. CS.L=0, CS.D=0. Address and operand size are
 		 * both 16-bit.
path: arch/x86/lib/insn-eval.c