Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030680AbdDTIdU (ORCPT ); Thu, 20 Apr 2017 04:33:20 -0400 Received: from mx2.suse.de ([195.135.220.15]:60020 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S941066AbdDTIdP (ORCPT ); Thu, 20 Apr 2017 04:33:15 -0400 Date: Thu, 20 Apr 2017 10:25:33 +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: [v6 PATCH 08/21] x86/insn-eval: Add utility function to get segment descriptor base address Message-ID: <20170420082533.hwc6eon3aom43qzo@pd.tnic> References: <20170308003254.27833-1-ricardo.neri-calderon@linux.intel.com> <20170308003254.27833-9-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: <20170308003254.27833-9-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: 5903 Lines: 169 On Tue, Mar 07, 2017 at 04:32:41PM -0800, 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 in the s/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. > > The segment selector is determined by get_seg_selector with the inputs Please end function names with parentheses: get_seg_selector(). > described above. Once the selector is known the base address is known, ... > determined. In protected mode, the selector is used to obtain the segment > descriptor and then its base address. If in 64-bit user mode, the segment = > base address is zero except when FS or GS are used. In virtual-8086 mode, > the base address is computed as the value of the segment selector shifted 4 > positions to the left. Good. > 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/include/asm/insn-eval.h | 2 ++ > arch/x86/lib/insn-eval.c | 66 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 68 insertions(+) > > diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h > index 754211b..b201742 100644 > --- a/arch/x86/include/asm/insn-eval.h > +++ b/arch/x86/include/asm/insn-eval.h > @@ -15,5 +15,7 @@ void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs); > int insn_get_reg_offset_modrm_rm(struct insn *insn, struct pt_regs *regs); > int insn_get_reg_offset_sib_base(struct insn *insn, struct pt_regs *regs); > int insn_get_reg_offset_sib_base(struct insn *insn, struct pt_regs *regs); > +unsigned long insn_get_seg_base(struct pt_regs *regs, struct insn *insn, > + int regoff, bool use_default_seg); > > #endif /* _ASM_X86_INSN_EVAL_H */ > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c > index 8608adf..383ca83 100644 > --- a/arch/x86/lib/insn-eval.c > +++ b/arch/x86/lib/insn-eval.c > @@ -355,6 +355,72 @@ static int get_desc(unsigned short seg, struct desc_struct **desc) > } > > /** > + * insn_get_seg_base() - Obtain base address contained in descriptor > + * @regs: Set of registers containing the segment selector > + * @insn: Instruction structure with selector override prefixes > + * @regoff: Operand offset, in pt_regs, of which the selector is needed > + * @use_default_seg: Use the default segment instead of prefix overrides I'm wondering whether you really need that bool or you can deduce this from pt_regs... I guess I'll see... > + * > + * 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. It may be zero in > + * certain cases for 64-bit builds and/or 64-bit applications. In virtual-8086 > + * mode, the segment selector shifed 4 positions to the right. -1L in case of s/shifed/shifted/ > + * error. > + */ > +unsigned long insn_get_seg_base(struct pt_regs *regs, struct insn *insn, > + int regoff, bool use_default_seg) > +{ > + struct desc_struct *desc; > + unsigned short seg; > + enum segment seg_type; > + int ret; > + > + seg_type = resolve_seg_selector(insn, regoff, use_default_seg); <--- error handling. And that's not really a "seg_type" but simply the "sel"-ector. And that "enum segment" is not really a segment but an segment override prefixes enum. Can we please get the nomenclature right first? > + > + seg = get_segment_selector(regs, seg_type); s/seg/sel/ > + if (seg < 0) > + return -1L; > + > + if (v8086_mode(regs)) > + /* > + * Base is simply the segment selector shifted 4 > + * positions to the right. > + */ > + return (unsigned long)(seg << 4); > + > +#ifdef CONFIG_X86_64 > + if (user_64bit_mode(regs)) { if (IS_ENABLED(CONFIG_X86_64) && user_64bit_mode(regs)) { > + /* > + * Only FS or GS will have a base address, the rest of > + * the segments' bases are forced to 0. > + */ > + unsigned long base; > + > + if (seg_type == SEG_FS) > + rdmsrl(MSR_FS_BASE, base); > + else if (seg_type == SEG_GS) > + /* > + * swapgs was called at the kernel entry point. Thus, > + * MSR_KERNEL_GS_BASE will have the user-space GS base. > + */ > + rdmsrl(MSR_KERNEL_GS_BASE, base); > + else > + base = 0; > + return base; > + } > +#endif > + ret = get_desc(seg, &desc); > + if (ret) > + return -1L; > + > + return get_desc_base(desc); > +} > + > +/** > * insn_get_reg_offset_modrm_rm - Obtain register in r/m part of ModRM byte > * @insn: Instruction structure containing the ModRM byte > * @regs: Set of registers indicated by the ModRM byte > -- > 2.9.3 > -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --