From: Jann Horn jannh@google.com
commit de9f869616dd95e95c00bdd6b0fcd3421e8a4323 upstream.
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 Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- arch/x86/lib/insn-eval.c | 47 ++++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 23 deletions(-)
--- a/arch/x86/lib/insn-eval.c +++ b/arch/x86/lib/insn-eval.c @@ -557,7 +557,8 @@ static int get_reg_offset_16(struct insn }
/** - * 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. @@ -565,18 +566,18 @@ static int get_reg_offset_16(struct insn * * 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. */ @@ -584,12 +585,14 @@ static struct desc_struct *get_desc(unsi
mutex_lock(¤t->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(¤t->active_mm->context.lock);
- return desc; + return success; } #endif native_store_gdt(&gdt_desc); @@ -604,9 +607,10 @@ static struct desc_struct *get_desc(unsi 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(unsi */ 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 p 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 p */ 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(struc 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(struc * 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(struc */ 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,8 +754,7 @@ int insn_get_code_seg_params(struct pt_r if (sel < 0) return sel;
- desc = get_desc(sel); - if (!desc) + if (!get_desc(&desc, sel)) return -EINVAL;
/* @@ -761,10 +762,10 @@ int insn_get_code_seg_params(struct pt_r * 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.