Messages in this thread Patch in this message |  | | Date | Tue, 26 Sep 2017 20:05:52 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH v8 13/28] x86/insn-eval: Add utility function to get segment descriptor |
| |
On Fri, Aug 18, 2017 at 05:27:54PM -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 look at segment descriptor to correctly calculate the ^ to
> 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. > > This function is a prerequisite for functions in subsequent commits that > will obtain the aforementioned attributes of the segment descriptor. > > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: Adam Buchbinder <adam.buchbinder@gmail.com> > Cc: Colin Ian King <colin.king@canonical.com> > Cc: Lorenzo Stoakes <lstoakes@gmail.com> > Cc: Qiaowei Ren <qiaowei.ren@intel.com> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com> > Cc: Masami Hiramatsu <mhiramat@kernel.org> > Cc: Adrian Hunter <adrian.hunter@intel.com> > Cc: Kees Cook <keescook@chromium.org> > Cc: Thomas Garnier <thgarnie@google.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Borislav Petkov <bp@suse.de> > Cc: Dmitry Vyukov <dvyukov@google.com> > Cc: Ravi V. Shankar <ravi.v.shankar@intel.com> > Cc: x86@kernel.org > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> > --- > arch/x86/lib/insn-eval.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 55 insertions(+) > > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c > index 86f58ce6c302..9cf2c49afc15 100644 > --- a/arch/x86/lib/insn-eval.c > +++ b/arch/x86/lib/insn-eval.c > @@ -6,9 +6,13 @@ > #include <linux/kernel.h> > #include <linux/string.h> > #include <linux/ratelimit.h> > +#include <linux/mmu_context.h> > +#include <asm/desc_defs.h> > +#include <asm/desc.h> > #include <asm/inat.h> > #include <asm/insn.h> > #include <asm/insn-eval.h> > +#include <asm/ldt.h> > #include <asm/vm86.h> > > enum reg_type { > @@ -402,6 +406,57 @@ static int get_reg_offset(struct insn *insn, struct pt_regs *regs, > } > > /** > + * get_desc() - Obtain address of segment descriptor
Get segment descriptor.
> + * @sel: Segment selector > + * > + * Given a segment selector, obtain a pointer to the segment descriptor. > + * Both global and local descriptor tables are supported. > + * > + * Return: pointer to segment descriptor on success. NULL on error. > + */ > +static struct desc_struct *get_desc(unsigned short sel)
I've simplified this function to be more readable, here's a diff ontop.
More specifically, if you flip the logic and move @desc inside the if, you don't need to have mutex_unlock() twice in there.
And having a local @ldt ptr makes the selector check more readable.
--- diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c index 9cf2c49afc15..48af787cb160 100644 --- a/arch/x86/lib/insn-eval.c +++ b/arch/x86/lib/insn-eval.c @@ -417,24 +417,24 @@ static int get_reg_offset(struct insn *insn, struct pt_regs *regs, static struct desc_struct *get_desc(unsigned short sel) { struct desc_ptr gdt_desc = {0, 0}; - struct desc_struct *desc = NULL; 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(¤t->active_mm->context.lock); - /* The size of the LDT refers to the number of entries. */ - if (!current->active_mm->context.ldt || - sel >= current->active_mm->context.ldt->nr_entries) { - mutex_unlock(¤t->active_mm->context.lock); - return NULL; - } - desc = ¤t->active_mm->context.ldt->entries[sel]; + ldt = current->active_mm->context.ldt; + if (ldt && sel < ldt->nr_entries) + desc = &ldt->entries[sel]; + mutex_unlock(¤t->active_mm->context.lock); + return desc; } #endif @@ -452,8 +452,7 @@ static struct desc_struct *get_desc(unsigned short sel) if (desc_base > gdt_desc.size) return NULL; - desc = (struct desc_struct *)(gdt_desc.address + desc_base); - return desc; + return (struct desc_struct *)(gdt_desc.address + desc_base); } /** -- Regards/Gruss, Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --
|  |