Skip to content

Commit de9f869

Browse files
thejhtorvalds
authored andcommitted
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: 670f928 ("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>
1 parent 1e1d926 commit de9f869

File tree

1 file changed

+24
-23
lines changed

1 file changed

+24
-23
lines changed

Diff for: arch/x86/lib/insn-eval.c

+24-23
Original file line numberDiff line numberDiff line change
@@ -557,39 +557,42 @@ static int get_reg_offset_16(struct insn *insn, struct pt_regs *regs,
557557
}
558558

559559
/**
560-
* get_desc() - Obtain pointer to a segment descriptor
560+
* get_desc() - Obtain contents of a segment descriptor
561+
* @out: Segment descriptor contents on success
561562
* @sel: Segment selector
562563
*
563564
* Given a segment selector, obtain a pointer to the segment descriptor.
564565
* Both global and local descriptor tables are supported.
565566
*
566567
* Returns:
567568
*
568-
* Pointer to segment descriptor on success.
569+
* True on success, false on failure.
569570
*
570571
* NULL on error.
571572
*/
572-
static struct desc_struct *get_desc(unsigned short sel)
573+
static bool get_desc(struct desc_struct *out, unsigned short sel)
573574
{
574575
struct desc_ptr gdt_desc = {0, 0};
575576
unsigned long desc_base;
576577

577578
#ifdef CONFIG_MODIFY_LDT_SYSCALL
578579
if ((sel & SEGMENT_TI_MASK) == SEGMENT_LDT) {
579-
struct desc_struct *desc = NULL;
580+
bool success = false;
580581
struct ldt_struct *ldt;
581582

582583
/* Bits [15:3] contain the index of the desired entry. */
583584
sel >>= 3;
584585

585586
mutex_lock(&current->active_mm->context.lock);
586587
ldt = current->active_mm->context.ldt;
587-
if (ldt && sel < ldt->nr_entries)
588-
desc = &ldt->entries[sel];
588+
if (ldt && sel < ldt->nr_entries) {
589+
*out = ldt->entries[sel];
590+
success = true;
591+
}
589592

590593
mutex_unlock(&current->active_mm->context.lock);
591594

592-
return desc;
595+
return success;
593596
}
594597
#endif
595598
native_store_gdt(&gdt_desc);
@@ -604,9 +607,10 @@ static struct desc_struct *get_desc(unsigned short sel)
604607
desc_base = sel & ~(SEGMENT_RPL_MASK | SEGMENT_TI_MASK);
605608

606609
if (desc_base > gdt_desc.size)
607-
return NULL;
610+
return false;
608611

609-
return (struct desc_struct *)(gdt_desc.address + desc_base);
612+
*out = *(struct desc_struct *)(gdt_desc.address + desc_base);
613+
return true;
610614
}
611615

612616
/**
@@ -628,7 +632,7 @@ static struct desc_struct *get_desc(unsigned short sel)
628632
*/
629633
unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx)
630634
{
631-
struct desc_struct *desc;
635+
struct desc_struct desc;
632636
short sel;
633637

634638
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)
666670
if (!sel)
667671
return -1L;
668672

669-
desc = get_desc(sel);
670-
if (!desc)
673+
if (!get_desc(&desc, sel))
671674
return -1L;
672675

673-
return get_desc_base(desc);
676+
return get_desc_base(&desc);
674677
}
675678

676679
/**
@@ -692,7 +695,7 @@ unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx)
692695
*/
693696
static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)
694697
{
695-
struct desc_struct *desc;
698+
struct desc_struct desc;
696699
unsigned long limit;
697700
short sel;
698701

@@ -706,8 +709,7 @@ static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)
706709
if (!sel)
707710
return 0;
708711

709-
desc = get_desc(sel);
710-
if (!desc)
712+
if (!get_desc(&desc, sel))
711713
return 0;
712714

713715
/*
@@ -716,8 +718,8 @@ static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)
716718
* not tested when checking the segment limits. In practice,
717719
* this means that the segment ends in (limit << 12) + 0xfff.
718720
*/
719-
limit = get_desc_limit(desc);
720-
if (desc->g)
721+
limit = get_desc_limit(&desc);
722+
if (desc.g)
721723
limit = (limit << 12) + 0xfff;
722724

723725
return limit;
@@ -741,7 +743,7 @@ static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)
741743
*/
742744
int insn_get_code_seg_params(struct pt_regs *regs)
743745
{
744-
struct desc_struct *desc;
746+
struct desc_struct desc;
745747
short sel;
746748

747749
if (v8086_mode(regs))
@@ -752,19 +754,18 @@ int insn_get_code_seg_params(struct pt_regs *regs)
752754
if (sel < 0)
753755
return sel;
754756

755-
desc = get_desc(sel);
756-
if (!desc)
757+
if (!get_desc(&desc, sel))
757758
return -EINVAL;
758759

759760
/*
760761
* The most significant byte of the Type field of the segment descriptor
761762
* determines whether a segment contains data or code. If this is a data
762763
* segment, return error.
763764
*/
764-
if (!(desc->type & BIT(3)))
765+
if (!(desc.type & BIT(3)))
765766
return -EINVAL;
766767

767-
switch ((desc->l << 1) | desc->d) {
768+
switch ((desc.l << 1) | desc.d) {
768769
case 0: /*
769770
* Legacy mode. CS.L=0, CS.D=0. Address and operand size are
770771
* both 16-bit.

0 commit comments

Comments
 (0)