From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932399AbdJJWmC (ORCPT ); Tue, 10 Oct 2017 18:42:02 -0400 Received: from mx2.suse.de ([195.135.220.15]:48965 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932225AbdJJWmA (ORCPT ); Tue, 10 Oct 2017 18:42:00 -0400 Date: Wed, 11 Oct 2017 00:41:44 +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 v9 13/29] x86/insn-eval: Add utility functions to get segment selector Message-ID: <20171010224144.wedursa2s3zpip43@pd.tnic> References: <1507089272-32733-1-git-send-email-ricardo.neri-calderon@linux.intel.com> <1507089272-32733-14-git-send-email-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: <1507089272-32733-14-git-send-email-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 Tue, Oct 03, 2017 at 08:54:16PM -0700, Ricardo Neri wrote: > When computing a linear address and segmentation is used, we need to know > the base address of the segment involved in the computation. In most of > the cases, the segment base address will be zero as in USER_DS/USER32_DS. ... > --- > arch/x86/include/asm/inat.h | 10 ++ > arch/x86/lib/insn-eval.c | 321 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 331 insertions(+) Ok, some more fixes ontop. I carved out the code under the resolve_default_idx: label into a separate function. This made resolve_seg_reg() pretty-much trivial to follow. Also renamed some functions and variables to better denote what they do. Please add Improvements-by: Borislav Petkov to your commit message if you use this. Thanks. --- diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c index 77b48f99d73a..d02b94ace0f1 100644 --- a/arch/x86/lib/insn-eval.c +++ b/arch/x86/lib/insn-eval.c @@ -49,7 +49,7 @@ static bool is_string_insn(struct insn *insn) } /** - * get_overridden_seg_reg_idx() - obtain segment register override index + * get_seg_reg_override_idx() - obtain segment register override index * @insn: Instruction with segment override prefixes * * Inspect the instruction prefixes and find segment overrides, if any. @@ -62,10 +62,10 @@ static bool is_string_insn(struct insn *insn) * * -EINVAL in case of error. */ -static int get_overridden_seg_reg_idx(struct insn *insn) +static int get_seg_reg_override_idx(struct insn *insn) { int idx = INAT_SEG_REG_DEFAULT; - int sel_overrides = 0, i; + int num_overrides = 0, i; if (!insn) return -EINVAL; @@ -80,41 +80,41 @@ static int get_overridden_seg_reg_idx(struct insn *insn) switch (attr) { case INAT_MAKE_PREFIX(INAT_PFX_CS): idx = INAT_SEG_REG_CS; - sel_overrides++; + num_overrides++; break; case INAT_MAKE_PREFIX(INAT_PFX_SS): idx = INAT_SEG_REG_SS; - sel_overrides++; + num_overrides++; break; case INAT_MAKE_PREFIX(INAT_PFX_DS): idx = INAT_SEG_REG_DS; - sel_overrides++; + num_overrides++; break; case INAT_MAKE_PREFIX(INAT_PFX_ES): idx = INAT_SEG_REG_ES; - sel_overrides++; + num_overrides++; break; case INAT_MAKE_PREFIX(INAT_PFX_FS): idx = INAT_SEG_REG_FS; - sel_overrides++; + num_overrides++; break; case INAT_MAKE_PREFIX(INAT_PFX_GS): idx = INAT_SEG_REG_GS; - sel_overrides++; + num_overrides++; break; /* No default action needed. */ } } /* More than one segment override prefix leads to undefined behavior. */ - if (sel_overrides > 1) + if (num_overrides > 1) return -EINVAL; return idx; } /** - * allow_seg_reg_overrides() - check if segment override prefixes are allowed + * check_seg_overrides() - check if segment override prefixes are allowed * @insn: Instruction with segment override prefixes * @regoff: Operand offset, in pt_regs, for which the check is performed * @@ -129,7 +129,7 @@ static int get_overridden_seg_reg_idx(struct insn *insn) * * -EINVAL in case of error. */ -static int allow_seg_reg_overrides(struct insn *insn, int regoff) +static int check_seg_overrides(struct insn *insn, int regoff) { /* * Segment override prefixes should not be used for rIP. It is not @@ -148,6 +148,55 @@ static int allow_seg_reg_overrides(struct insn *insn, int regoff) return 1; } +static int resolve_default_seg(struct insn *insn, struct pt_regs *regs, int off) +{ + if (user_64bit_mode(regs)) + return INAT_SEG_REG_IGNORE; + + /* + * If we are here, we use the default segment register as described + * in the Intel documentation: + * + * + DS for all references involving r[ABCD]X, and rSI. + * + If used in a string instruction, ES for rDI. Otherwise, DS. + * + AX, CX and DX are not valid register operands in 16-bit addresses. + * encodings but are valid for 32-bit and 64-bit encodings. + * + -EDOM is reserved to identify for cases in which no register + * is used (i.e., displacement-only addressing). Use DS. + * + SS for (E)SP or (E)BP. + * + CS for (E)IP. + */ + switch (off) { + case offsetof(struct pt_regs, ax): + case offsetof(struct pt_regs, cx): + case offsetof(struct pt_regs, dx): + /* Need insn to verify address size. */ + if (insn->addr_bytes == 2) + return -EINVAL; + + case -EDOM: + case offsetof(struct pt_regs, bx): + case offsetof(struct pt_regs, si): + return INAT_SEG_REG_DS; + + case offsetof(struct pt_regs, di): + if (is_string_insn(insn)) + return INAT_SEG_REG_ES; + return INAT_SEG_REG_DS; + + case offsetof(struct pt_regs, bp): + case offsetof(struct pt_regs, sp): + return INAT_SEG_REG_SS; + + case offsetof(struct pt_regs, ip): + return INAT_SEG_REG_CS; + + default: + return -EINVAL; + } +} + + /** * resolve_seg_reg() - obtain segment register index * @insn: Instruction with operands @@ -194,24 +243,24 @@ static int allow_seg_reg_overrides(struct insn *insn, int regoff) */ static int resolve_seg_reg(struct insn *insn, struct pt_regs *regs, int regoff) { - int use_pfx_overrides, idx; + int ret, idx; - use_pfx_overrides = allow_seg_reg_overrides(insn, regoff); - if (use_pfx_overrides < 0) - return use_pfx_overrides; + ret = check_seg_overrides(insn, regoff); + if (ret < 0) + return ret; - if (use_pfx_overrides == 0) - goto resolve_default_idx; + if (!ret) + return resolve_default_seg(insn, regs, regoff); if (!insn) return -EINVAL; - idx = get_overridden_seg_reg_idx(insn); + idx = get_seg_reg_override_idx(insn); if (idx < 0) return idx; if (idx == INAT_SEG_REG_DEFAULT) - goto resolve_default_idx; + return resolve_default_seg(insn, regs, regoff); /* * In long mode, segment override prefixes are ignored, except for @@ -224,53 +273,6 @@ static int resolve_seg_reg(struct insn *insn, struct pt_regs *regs, int regoff) } return idx; - -resolve_default_idx: - - if (user_64bit_mode(regs)) - return INAT_SEG_REG_IGNORE; - /* - * If we are here, we use the default segment register as described - * in the Intel documentation: - * - * + DS for all references involving r[ABCD]X, and rSI. - * + If used in a string instruction, ES for rDI. Otherwise, DS. - * + AX, CX and DX are not valid register operands in 16-bit addresses. - * encodings but are valid for 32-bit and 64-bit encodings. - * + -EDOM is reserved to identify for cases in which no register - * is used (i.e., displacement-only addressing). Use DS. - * + SS for (E)SP or (E)BP. - * + CS for (E)IP. - */ - - switch (regoff) { - case offsetof(struct pt_regs, ax): - case offsetof(struct pt_regs, cx): - case offsetof(struct pt_regs, dx): - /* Need insn to verify address size. */ - if (insn->addr_bytes == 2) - return -EINVAL; - - case -EDOM: - case offsetof(struct pt_regs, bx): - case offsetof(struct pt_regs, si): - return INAT_SEG_REG_DS; - - case offsetof(struct pt_regs, di): - if (is_string_insn(insn)) - return INAT_SEG_REG_ES; - return INAT_SEG_REG_DS; - - case offsetof(struct pt_regs, bp): - case offsetof(struct pt_regs, sp): - return INAT_SEG_REG_SS; - - case offsetof(struct pt_regs, ip): - return INAT_SEG_REG_CS; - - default: - return -EINVAL; - } } /** -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --