From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752220AbdJLAq3 (ORCPT ); Wed, 11 Oct 2017 20:46:29 -0400 Received: from mga04.intel.com ([192.55.52.120]:2820 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751339AbdJLAq1 (ORCPT ); Wed, 11 Oct 2017 20:46:27 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,363,1503385200"; d="scan'208";a="1204884539" Message-ID: <1507769154.17492.30.camel@linux.intel.com> Subject: Re: [PATCH v9 14/29] x86/insn-eval: Add utility function to get segment descriptor From: Ricardo Neri To: Borislav Petkov 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, Adam Buchbinder , Colin Ian King , Lorenzo Stoakes , Qiaowei Ren , Arnaldo Carvalho de Melo , Adrian Hunter , Kees Cook , Thomas Garnier , Dmitry Vyukov Date: Wed, 11 Oct 2017 17:45:54 -0700 In-Reply-To: <20171011145729.j7rm3jduvczdhbek@pd.tnic> References: <1507089272-32733-1-git-send-email-ricardo.neri-calderon@linux.intel.com> <1507089272-32733-15-git-send-email-ricardo.neri-calderon@linux.intel.com> <20171011145729.j7rm3jduvczdhbek@pd.tnic> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.2-0ubuntu3.2 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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