| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 11 Oct 2017 00:41:44 +0200
From: Borislav Petkov <bp@...e.de>
To: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
Cc: Ingo Molnar <mingo@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
"H. Peter Anvin" <hpa@...or.com>,
Andy Lutomirski <luto@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Brian Gerst <brgerst@...il.com>,
Chris Metcalf <cmetcalf@...lanox.com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Paolo Bonzini <pbonzini@...hat.com>,
Masami Hiramatsu <mhiramat@...nel.org>,
Huang Rui <ray.huang@....com>, Jiri Slaby <jslaby@...e.cz>,
Jonathan Corbet <corbet@....net>,
"Michael S. Tsirkin" <mst@...hat.com>,
Paul Gortmaker <paul.gortmaker@...driver.com>,
Vlastimil Babka <vbabka@...e.cz>,
Chen Yucong <slaoub@...il.com>,
"Ravi V. Shankar" <ravi.v.shankar@...el.com>,
Shuah Khan <shuah@...nel.org>, linux-kernel@...r.kernel.org,
x86@...nel.org, ricardo.neri@...el.com,
Adam Buchbinder <adam.buchbinder@...il.com>,
Colin Ian King <colin.king@...onical.com>,
Lorenzo Stoakes <lstoakes@...il.com>,
Qiaowei Ren <qiaowei.ren@...el.com>,
Arnaldo Carvalho de Melo <acme@...hat.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Kees Cook <keescook@...omium.org>,
Thomas Garnier <thgarnie@...gle.com>,
Dmitry Vyukov <dvyukov@...gle.com>
Subject: Re: [PATCH v9 13/29] x86/insn-eval: Add utility functions to get
segment selector
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 <bp@...e.de>
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)
--
Powered by blists - more mailing lists