linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sasha Levin <sasha.levin@oracle.com>
To: vegard.nossum@oracle.com, penberg@kernel.org
Cc: jamie.iles@oracle.com, hpa@zytor.com, mingo@redhat.com,
	tglx@linutronix.de, x86@kernel.org,
	masami.hiramatsu.pt@hitachi.com, linux-kernel@vger.kernel.org,
	linux-mm@vger.kernel.org, Sasha Levin <sasha.levin@oracle.com>
Subject: [PATCH 4/4] kmemcheck: Switch to using kernel disassembler
Date: Mon, 14 Apr 2014 13:44:10 -0400	[thread overview]
Message-ID: <1397497450-6440-4-git-send-email-sasha.levin@oracle.com> (raw)
In-Reply-To: <1397497450-6440-1-git-send-email-sasha.levin@oracle.com>

kmemcheck used to do it's own basic instruction decoding, which is
just a duplication of the work done in arch/x86/lib/insn.c.

Instead, switch it to using the already existing dissasembler, and
switch the magic opcode numbers into something meaningful.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 arch/x86/mm/kmemcheck/Makefile    |    2 +-
 arch/x86/mm/kmemcheck/kmemcheck.c |  105 ++++++++++++++++++------------------
 arch/x86/mm/kmemcheck/opcode.c    |  106 -------------------------------------
 arch/x86/mm/kmemcheck/opcode.h    |    9 ----
 arch/x86/mm/kmemcheck/selftest.c  |   12 +++--
 arch/x86/mm/kmemcheck/selftest.h  |    1 +
 6 files changed, 61 insertions(+), 174 deletions(-)
 delete mode 100644 arch/x86/mm/kmemcheck/opcode.c
 delete mode 100644 arch/x86/mm/kmemcheck/opcode.h

diff --git a/arch/x86/mm/kmemcheck/Makefile b/arch/x86/mm/kmemcheck/Makefile
index 520b3bc..f1749ad 100644
--- a/arch/x86/mm/kmemcheck/Makefile
+++ b/arch/x86/mm/kmemcheck/Makefile
@@ -1 +1 @@
-obj-y := error.o kmemcheck.o opcode.o pte.o selftest.o shadow.o
+obj-y := error.o kmemcheck.o pte.o selftest.o shadow.o
diff --git a/arch/x86/mm/kmemcheck/kmemcheck.c b/arch/x86/mm/kmemcheck/kmemcheck.c
index dd89a13..571664d 100644
--- a/arch/x86/mm/kmemcheck/kmemcheck.c
+++ b/arch/x86/mm/kmemcheck/kmemcheck.c
@@ -25,9 +25,10 @@
 #include <asm/kmemcheck.h>
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
+#include <asm/insn.h>
+#include <asm/inat-tables.h>
 
 #include "error.h"
-#include "opcode.h"
 #include "pte.h"
 #include "selftest.h"
 #include "shadow.h"
@@ -521,13 +522,30 @@ enum kmemcheck_method {
 	KMEMCHECK_WRITE,
 };
 
+char kmemcheck_get_addr_size(struct insn *insn)
+{
+	switch (insn->opcode.value) {
+	case 0xa4:
+	case 0xbe0f:
+	case 0xb60f:
+		return 1;
+
+	case 0xbf0f:
+	case 0xb70f:
+		return 2;
+	case 0x63:
+		if (!X86_REX_W(insn->rex_prefix.value))
+			break;
+		return 4;
+	}
+
+	return insn->mem_bytes;
+}
+
 static void kmemcheck_access(struct pt_regs *regs,
 	unsigned long fallback_address, enum kmemcheck_method fallback_method)
 {
-	const uint8_t *insn;
-	const uint8_t *insn_primary;
-	unsigned int size;
-
+	struct insn insn;
 	struct kmemcheck_context *data = &__get_cpu_var(kmemcheck_context);
 
 	/* Recursive fault -- ouch. */
@@ -539,63 +557,42 @@ static void kmemcheck_access(struct pt_regs *regs,
 
 	data->busy = true;
 
-	insn = (const uint8_t *) regs->ip;
-	insn_primary = kmemcheck_opcode_get_primary(insn);
-
-	kmemcheck_opcode_decode(insn, &size);
+	kernel_insn_init(&insn, (const void *)regs->ip);
+	insn_get_length(&insn);
 
-	switch (insn_primary[0]) {
+	switch (insn.mnemonic) {
 #ifdef CONFIG_KMEMCHECK_BITOPS_OK
-		/* AND, OR, XOR */
-		/*
-		 * Unfortunately, these instructions have to be excluded from
-		 * our regular checking since they access only some (and not
-		 * all) bits. This clears out "bogus" bitfield-access warnings.
-		 */
-	case 0x80:
-	case 0x81:
-	case 0x82:
-	case 0x83:
-		switch ((insn_primary[1] >> 3) & 7) {
-			/* OR */
-		case 1:
-			/* AND */
-		case 4:
-			/* XOR */
-		case 6:
-			kmemcheck_write(regs, fallback_address, size);
-			goto out;
-
-			/* ADD */
-		case 0:
-			/* ADC */
-		case 2:
-			/* SBB */
-		case 3:
-			/* SUB */
-		case 5:
-			/* CMP */
-		case 7:
-			break;
-		}
+	/*
+	 * Unfortunately, these instructions have to be excluded from
+	 * our regular checking since they access only some (and not
+	 * all) bits. This clears out "bogus" bitfield-access warnings.
+	 */
+	case INSN_OPC_OR:
+	case INSN_OPC_AND:
+	case INSN_OPC_XOR:
+		kmemcheck_write(regs, fallback_address, insn.mem_bytes);
+		goto out;
+
+	case INSN_OPC_ADD:
+	case INSN_OPC_ADC:
+	case INSN_OPC_SBB:
+	case INSN_OPC_SUB:
+	case INSN_OPC_CMP:
 		break;
 #endif
-
-		/* MOVS, MOVSB, MOVSW, MOVSD */
-	case 0xa4:
-	case 0xa5:
+	case INSN_OPC_MOVS_B:
+	case INSN_OPC_MOVS_W_D_Q:
 		/*
 		 * These instructions are special because they take two
 		 * addresses, but we only get one page fault.
 		 */
-		kmemcheck_copy(regs, regs->si, regs->di, size);
+		kmemcheck_copy(regs, regs->si, regs->di, insn.mem_bytes);
 		goto out;
 
-		/* CMPS, CMPSB, CMPSW, CMPSD */
-	case 0xa6:
-	case 0xa7:
-		kmemcheck_read(regs, regs->si, size);
-		kmemcheck_read(regs, regs->di, size);
+	case INSN_OPC_CMPS_B:
+	case INSN_OPC_CMPS_W_D:
+		kmemcheck_read(regs, regs->si, insn.mem_bytes);
+		kmemcheck_read(regs, regs->di, insn.mem_bytes);
 		goto out;
 	}
 
@@ -606,10 +603,10 @@ static void kmemcheck_access(struct pt_regs *regs,
 	 */
 	switch (fallback_method) {
 	case KMEMCHECK_READ:
-		kmemcheck_read(regs, fallback_address, size);
+		kmemcheck_read(regs, fallback_address, insn.mem_bytes);
 		goto out;
 	case KMEMCHECK_WRITE:
-		kmemcheck_write(regs, fallback_address, size);
+		kmemcheck_write(regs, fallback_address, insn.mem_bytes);
 		goto out;
 	}
 
diff --git a/arch/x86/mm/kmemcheck/opcode.c b/arch/x86/mm/kmemcheck/opcode.c
deleted file mode 100644
index 324aa3f..0000000
--- a/arch/x86/mm/kmemcheck/opcode.c
+++ /dev/null
@@ -1,106 +0,0 @@
-#include <linux/types.h>
-
-#include "opcode.h"
-
-static bool opcode_is_prefix(uint8_t b)
-{
-	return
-		/* Group 1 */
-		b == 0xf0 || b == 0xf2 || b == 0xf3
-		/* Group 2 */
-		|| b == 0x2e || b == 0x36 || b == 0x3e || b == 0x26
-		|| b == 0x64 || b == 0x65
-		/* Group 3 */
-		|| b == 0x66
-		/* Group 4 */
-		|| b == 0x67;
-}
-
-#ifdef CONFIG_X86_64
-static bool opcode_is_rex_prefix(uint8_t b)
-{
-	return (b & 0xf0) == 0x40;
-}
-#else
-static bool opcode_is_rex_prefix(uint8_t b)
-{
-	return false;
-}
-#endif
-
-#define REX_W (1 << 3)
-
-/*
- * This is a VERY crude opcode decoder. We only need to find the size of the
- * load/store that caused our #PF and this should work for all the opcodes
- * that we care about. Moreover, the ones who invented this instruction set
- * should be shot.
- */
-void kmemcheck_opcode_decode(const uint8_t *op, unsigned int *size)
-{
-	/* Default operand size */
-	int operand_size_override = 4;
-
-	/* prefixes */
-	for (; opcode_is_prefix(*op); ++op) {
-		if (*op == 0x66)
-			operand_size_override = 2;
-	}
-
-	/* REX prefix */
-	if (opcode_is_rex_prefix(*op)) {
-		uint8_t rex = *op;
-
-		++op;
-		if (rex & REX_W) {
-			switch (*op) {
-			case 0x63:
-				*size = 4;
-				return;
-			case 0x0f:
-				++op;
-
-				switch (*op) {
-				case 0xb6:
-				case 0xbe:
-					*size = 1;
-					return;
-				case 0xb7:
-				case 0xbf:
-					*size = 2;
-					return;
-				}
-
-				break;
-			}
-
-			*size = 8;
-			return;
-		}
-	}
-
-	/* escape opcode */
-	if (*op == 0x0f) {
-		++op;
-
-		/*
-		 * This is move with zero-extend and sign-extend, respectively;
-		 * we don't have to think about 0xb6/0xbe, because this is
-		 * already handled in the conditional below.
-		 */
-		if (*op == 0xb7 || *op == 0xbf)
-			operand_size_override = 2;
-	}
-
-	*size = (*op & 1) ? operand_size_override : 1;
-}
-
-const uint8_t *kmemcheck_opcode_get_primary(const uint8_t *op)
-{
-	/* skip prefixes */
-	while (opcode_is_prefix(*op))
-		++op;
-	if (opcode_is_rex_prefix(*op))
-		++op;
-	return op;
-}
diff --git a/arch/x86/mm/kmemcheck/opcode.h b/arch/x86/mm/kmemcheck/opcode.h
deleted file mode 100644
index 6956aad..0000000
--- a/arch/x86/mm/kmemcheck/opcode.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef ARCH__X86__MM__KMEMCHECK__OPCODE_H
-#define ARCH__X86__MM__KMEMCHECK__OPCODE_H
-
-#include <linux/types.h>
-
-void kmemcheck_opcode_decode(const uint8_t *op, unsigned int *size);
-const uint8_t *kmemcheck_opcode_get_primary(const uint8_t *op);
-
-#endif
diff --git a/arch/x86/mm/kmemcheck/selftest.c b/arch/x86/mm/kmemcheck/selftest.c
index c898d33..8976938 100644
--- a/arch/x86/mm/kmemcheck/selftest.c
+++ b/arch/x86/mm/kmemcheck/selftest.c
@@ -1,7 +1,9 @@
 #include <linux/bug.h>
 #include <linux/kernel.h>
 
-#include "opcode.h"
+#include <asm/insn.h>
+#include <asm/inat-tables.h>
+
 #include "selftest.h"
 
 struct selftest_opcode {
@@ -46,10 +48,12 @@ static const struct selftest_opcode selftest_opcodes[] = {
 
 static bool selftest_opcode_one(const struct selftest_opcode *op)
 {
-	unsigned size;
-
-	kmemcheck_opcode_decode(op->insn, &size);
+	struct insn insn;
+	char size;
 
+	kernel_insn_init(&insn, op->insn);
+	insn_get_length(&insn);
+	size = kmemcheck_get_addr_size(&insn);
 	if (size == op->expected_size)
 		return true;
 
diff --git a/arch/x86/mm/kmemcheck/selftest.h b/arch/x86/mm/kmemcheck/selftest.h
index 8fed4fe..4cd1c5e 100644
--- a/arch/x86/mm/kmemcheck/selftest.h
+++ b/arch/x86/mm/kmemcheck/selftest.h
@@ -2,5 +2,6 @@
 #define ARCH_X86_MM_KMEMCHECK_SELFTEST_H
 
 bool kmemcheck_selftest(void);
+extern char kmemcheck_get_addr_size(struct insn *insn);
 
 #endif
-- 
1.7.10.4


  parent reply	other threads:[~2014-04-14 17:45 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-14 17:44 [PATCH 1/4] kmemcheck: add additional selfchecks Sasha Levin
2014-04-14 17:44 ` [PATCH 2/4] x86: Move instruction decoder data into header Sasha Levin
2014-04-15  1:41   ` Masami Hiramatsu
2014-04-15  2:28     ` Sasha Levin
2014-04-15  3:10       ` Masami Hiramatsu
2014-04-15 14:24         ` Sasha Levin
2014-04-16  3:06           ` Masami Hiramatsu
2014-04-14 17:44 ` [PATCH 3/4] x86/insn: Extract more information about instructions Sasha Levin
2014-04-15  3:12   ` Masami Hiramatsu
2014-04-15  4:36     ` Masami Hiramatsu
2014-04-15 15:10     ` Sasha Levin
2014-04-16  3:26       ` H. Peter Anvin
2014-04-16  3:47         ` Sasha Levin
2014-04-16  3:54           ` H. Peter Anvin
2014-04-16  4:03             ` Sasha Levin
2014-04-16  4:31               ` H. Peter Anvin
2014-04-16  5:30               ` Masami Hiramatsu
2014-04-17 15:20                 ` Sasha Levin
2014-04-17 15:28                   ` H. Peter Anvin
2014-04-17 17:31                     ` Sasha Levin
2014-04-18  3:40                       ` Masami Hiramatsu
2014-04-18  3:45                         ` H. Peter Anvin
2014-04-18 15:47                           ` Sasha Levin
2014-04-18 16:48                             ` H. Peter Anvin
2014-04-16  5:44       ` Masami Hiramatsu
2014-04-17 15:33         ` Sasha Levin
2014-04-18  3:25           ` Masami Hiramatsu
2014-04-14 17:44 ` Sasha Levin [this message]
2014-04-15  8:17   ` [PATCH 4/4] kmemcheck: Switch to using kernel disassembler Pekka Enberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1397497450-6440-4-git-send-email-sasha.levin@oracle.com \
    --to=sasha.levin@oracle.com \
    --cc=hpa@zytor.com \
    --cc=jamie.iles@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@redhat.com \
    --cc=penberg@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=vegard.nossum@oracle.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).