From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751151AbdEaQ6z (ORCPT ); Wed, 31 May 2017 12:58:55 -0400 Received: from mx2.suse.de ([195.135.220.15]:60055 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750916AbdEaQ6v (ORCPT ); Wed, 31 May 2017 12:58:51 -0400 Date: Wed, 31 May 2017 18:58:39 +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 , Alexandre Julliard , Stas Sergeev , Fenghua Yu , "Ravi V. Shankar" , Shuah Khan , linux-kernel@vger.kernel.org, x86@kernel.org, linux-msdos@vger.kernel.org, wine-devel@winehq.org, Adam Buchbinder , Colin Ian King , Lorenzo Stoakes , Qiaowei Ren , Arnaldo Carvalho de Melo , Adrian Hunter , Kees Cook , Thomas Garnier , Dmitry Vyukov Subject: Re: [PATCH v7 12/26] x86/insn-eval: Add utility functions to get segment descriptor base address and limit Message-ID: <20170531165839.6nlkmdlrqnuloulz@pd.tnic> References: <20170505181724.55000-1-ricardo.neri-calderon@linux.intel.com> <20170505181724.55000-13-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: <20170505181724.55000-13-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 On Fri, May 05, 2017 at 11:17:10AM -0700, Ricardo Neri wrote: > With segmentation, the base address of the segment descriptor is needed > to compute a linear address. The segment descriptor used in the address > computation depends on either any segment override prefixes in the > instruction or the default segment determined by the registers involved > in the address computation. Thus, both the instruction as well as the > register (specified as the offset from the base of pt_regs) are given as > inputs, along with a boolean variable to select between override and > default. ... > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c > index f46cb31..c77ed80 100644 > --- a/arch/x86/lib/insn-eval.c > +++ b/arch/x86/lib/insn-eval.c > @@ -476,6 +476,133 @@ static struct desc_struct *get_desc(unsigned short sel) > } > > /** > + * insn_get_seg_base() - Obtain base address of segment descriptor. > + * @regs: Structure with register values as seen when entering kernel mode > + * @insn: Instruction structure with selector override prefixes > + * @regoff: Operand offset, in pt_regs, of which the selector is needed > + * > + * Obtain the base address of the segment descriptor as indicated by either > + * any segment override prefixes contained in insn or the default segment > + * applicable to the register indicated by regoff. regoff is specified as the > + * offset in bytes from the base of pt_regs. > + * > + * Return: In protected mode, base address of the segment. Zero in for long > + * mode, except when FS or GS are used. In virtual-8086 mode, the segment > + * selector shifted 4 positions to the right. -1L in case of > + * error. > + */ > +unsigned long insn_get_seg_base(struct pt_regs *regs, struct insn *insn, > + int regoff) > +{ > + struct desc_struct *desc; > + unsigned short sel; > + enum segment_register seg_reg; > + > + seg_reg = resolve_seg_register(insn, regs, regoff); > + if (seg_reg == SEG_REG_INVAL) > + return -1L; > + > + sel = get_segment_selector(regs, seg_reg); > + if ((short)sel < 0) I guess it would be better if that function returned a signed short so you don't have to cast it here. (You're casting it to an unsigned long below anyway.) > + return -1L; > + > + if (v8086_mode(regs)) > + /* > + * Base is simply the segment selector shifted 4 > + * positions to the right. > + */ > + return (unsigned long)(sel << 4); > + ... > +static unsigned long get_seg_limit(struct pt_regs *regs, struct insn *insn, > + int regoff) > +{ > + struct desc_struct *desc; > + unsigned short sel; > + unsigned long limit; > + enum segment_register seg_reg; > + > + seg_reg = resolve_seg_register(insn, regs, regoff); > + if (seg_reg == SEG_REG_INVAL) > + return 0; > + > + sel = get_segment_selector(regs, seg_reg); > + if ((short)sel < 0) Ditto. > + return 0; > + > + if (user_64bit_mode(regs) || v8086_mode(regs)) > + return -1L; > + > + if (!sel) > + return 0; > + > + desc = get_desc(sel); > + if (!desc) > + return 0; > + > + /* > + * If the granularity bit is set, the limit is given in multiples > + * of 4096. When the granularity bit is set, the least 12 significant the 12 least significant bits > + * 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 <<= 12 | 0x7; That 0x7 doesn't look like 0xfff - it shifts limit by 15 instead. You can simply write it like you mean it: limit = (limit << 12) + 0xfff; -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --