Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S969880AbdIZSGG (ORCPT ); Tue, 26 Sep 2017 14:06:06 -0400 Received: from mx2.suse.de ([195.135.220.15]:41115 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S964775AbdIZSGE (ORCPT ); Tue, 26 Sep 2017 14:06:04 -0400 Date: Tue, 26 Sep 2017 20:05:52 +0200 From: Borislav Petkov To: Ricardo Neri Cc: Ingo Molnar , Thomas Gleixner , "H. Peter Anvin" , Andy Lutomirski , Peter Zijlstra , Andrew Morton , Brian Gerst , Chris Metcalf , Dave Hansen , Paolo Bonzini , Masami Hiramatsu , Huang Rui , Jiri Slaby , Jonathan Corbet , "Michael S. Tsirkin" , Paul Gortmaker , Vlastimil Babka , Chen Yucong , "Ravi V. Shankar" , Shuah Khan , linux-kernel@vger.kernel.org, x86@kernel.org, ricardo.neri@intel.com, Adam Buchbinder , Colin Ian King , Lorenzo Stoakes , Qiaowei Ren , Arnaldo Carvalho de Melo , Adrian Hunter , Kees Cook , Thomas Garnier , Dmitry Vyukov Subject: Re: [PATCH v8 13/28] x86/insn-eval: Add utility function to get segment descriptor Message-ID: <20170926180552.ofu7dij4gik5c33a@pd.tnic> References: <20170819002809.111312-1-ricardo.neri-calderon@linux.intel.com> <20170819002809.111312-14-ricardo.neri-calderon@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170819002809.111312-14-ricardo.neri-calderon@linux.intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4486 Lines: 134 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 > Cc: Adam Buchbinder > Cc: Colin Ian King > Cc: Lorenzo Stoakes > Cc: Qiaowei Ren > Cc: Arnaldo Carvalho de Melo > Cc: Masami Hiramatsu > Cc: Adrian Hunter > Cc: Kees Cook > Cc: Thomas Garnier > Cc: Peter Zijlstra > Cc: Borislav Petkov > Cc: Dmitry Vyukov > Cc: Ravi V. Shankar > Cc: x86@kernel.org > Signed-off-by: Ricardo Neri > --- > 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 > #include > #include > +#include > +#include > +#include > #include > #include > #include > +#include > #include > > 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) --