On Wed, 2017-10-11 at 16:57 +0200, Borislav Petkov wrote: > On Tue, Oct 03, 2017 at 08:54:17PM -0700, Ricardo Neri wrote: > > > > The segment descriptor contains information that is relevant to how linear > > addresses need to be computed. It contains the default size of addresses > > as well as the base address of the segment. Thus, given a segment > > selector, we ought to look at segment descriptor to correctly calculate > > the linear address. > > > > In protected mode, the segment selector might indicate a segment > > descriptor from either the global descriptor table or a local descriptor > > table. Both cases are considered in this function. > ... > > > > > +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 > > + struct desc_struct *desc = NULL; > > + struct ldt_struct *ldt; > You moved those out of the if-statement even though they're needed only > in that scope. Why?
No reason, this is not correct. I will move them into the if-statement. > Here's a diff that moves them back there and improves the function comment. > > --- > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c > index 62975f825556..c4e82bb4c4d3 100644 > --- a/arch/x86/lib/insn-eval.c > +++ b/arch/x86/lib/insn-eval.c > @@ -456,11 +456,11 @@ static int get_reg_offset(struct insn *insn, struct > pt_regs *regs, > } > > /** > - * get_desc() - Obtain address of segment descriptor > + * 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. > + * Given a segment selector, obtain a pointer to the corresponding segment > + * descriptor. Both global and local descriptor tables are supported. > * > * Returns: > * > @@ -474,10 +474,10 @@ static struct desc_struct *get_desc(unsigned short sel) > unsigned long desc_base; > > #ifdef CONFIG_MODIFY_LDT_SYSCALL > - struct desc_struct *desc = NULL; > - struct ldt_struct *ldt; > - > 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; > I will take your diff along with your Improvements-by: tag. Thanks and BR, Ricardo