lkml.org 
[lkml]   [2017]   [Dec]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [tip:x86/mpx] x86/insn-eval: Add utility function to get segment descriptor
    On Wed, Nov 01, 2017 at 02:00:28PM -0700, tip-bot for Ricardo Neri wrote:
    > +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

    This is broken right? You unlock and then return @desc, which afaict can
    at that point get freed by free_ldt_struct().

    Something like the below ought to cure; although its not entirely
    pretty either.

    ---

    diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
    index e664058c4491..c234ef2b4430 100644
    --- a/arch/x86/lib/insn-eval.c
    +++ b/arch/x86/lib/insn-eval.c
    @@ -572,6 +572,11 @@ static struct desc_struct *get_desc(unsigned short sel)
    struct desc_ptr gdt_desc = {0, 0};
    unsigned long desc_base;

    + /*
    + * Relies on IRQs being disabled to serialize against the LDT.
    + */
    + lockdep_assert_irqs_disabled();
    +
    #ifdef CONFIG_MODIFY_LDT_SYSCALL
    if ((sel & SEGMENT_TI_MASK) == SEGMENT_LDT) {
    struct desc_struct *desc = NULL;
    @@ -580,13 +585,10 @@ static struct desc_struct *get_desc(unsigned short sel)
    /* 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_va[sel];

    - mutex_unlock(&current->active_mm->context.lock);
    -
    return desc;
    }
    #endif
    @@ -626,6 +628,7 @@ static struct desc_struct *get_desc(unsigned short sel)
    */
    unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx)
    {
    + unsigned long base, flags;
    struct desc_struct *desc;
    short sel;

    @@ -664,11 +667,15 @@ unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx)
    if (!sel)
    return -1L;

    + base = -1;
    +
    + local_irq_save(flags);
    desc = get_desc(sel);
    - if (!desc)
    - return -1L;
    + if (desc)
    + base = get_desc_base(desc);
    + local_irq_restore(flags);

    - return get_desc_base(desc);
    + return base;
    }

    /**
    @@ -690,8 +697,8 @@ 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)
    {
    + unsigned long flags, limit = 0;
    struct desc_struct *desc;
    - unsigned long limit;
    short sel;

    sel = get_segment_selector(regs, seg_reg_idx);
    @@ -704,19 +711,20 @@ static unsigned long get_seg_limit(struct pt_regs *regs, int seg_reg_idx)
    if (!sel)
    return 0;

    + local_irq_save(flags);
    desc = get_desc(sel);
    - if (!desc)
    - return 0;
    -
    - /*
    - * If the granularity bit is set, the limit is given in multiples
    - * of 4096. This also means that the 12 least significant bits are
    - * 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;
    + if (desc) {
    + /*
    + * If the granularity bit is set, the limit is given in multiples
    + * of 4096. This also means that the 12 least significant bits are
    + * 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;
    + }
    + local_irq_restore(flags);

    return limit;
    }
    @@ -740,19 +748,23 @@ 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;
    + unsigned long flags;
    + int ret = -EINVAL;
    short sel;

    - if (v8086_mode(regs))
    + if (v8086_mode(regs)) {
    /* Address and operand size are both 16-bit. */
    return INSN_CODE_SEG_PARAMS(2, 2);
    + }

    sel = get_segment_selector(regs, INAT_SEG_REG_CS);
    if (sel < 0)
    return sel;

    + local_irq_save(flags);
    desc = get_desc(sel);
    if (!desc)
    - return -EINVAL;
    + goto out;

    /*
    * The most significant byte of the Type field of the segment descriptor
    @@ -760,29 +772,37 @@ int insn_get_code_seg_params(struct pt_regs *regs)
    * segment, return error.
    */
    if (!(desc->type & BIT(3)))
    - return -EINVAL;
    + goto out;

    switch ((desc->l << 1) | desc->d) {
    case 0: /*
    * Legacy mode. CS.L=0, CS.D=0. Address and operand size are
    * both 16-bit.
    */
    - return INSN_CODE_SEG_PARAMS(2, 2);
    + ret = INSN_CODE_SEG_PARAMS(2, 2);
    + break;
    case 1: /*
    * Legacy mode. CS.L=0, CS.D=1. Address and operand size are
    * both 32-bit.
    */
    - return INSN_CODE_SEG_PARAMS(4, 4);
    + ret = INSN_CODE_SEG_PARAMS(4, 4);
    + break;
    case 2: /*
    * IA-32e 64-bit mode. CS.L=1, CS.D=0. Address size is 64-bit;
    * operand size is 32-bit.
    */
    - return INSN_CODE_SEG_PARAMS(4, 8);
    + ret = INSN_CODE_SEG_PARAMS(4, 8);
    + break;
    +
    case 3: /* Invalid setting. CS.L=1, CS.D=1 */
    /* fall through */
    default:
    - return -EINVAL;
    + break;
    }
    +out:
    + local_irq_restore(flags);
    +
    + return ret;
    }

    /**
    \
     
     \ /
      Last update: 2017-12-05 18:49    [W:4.432 / U:0.100 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site