All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>,
	Andi Kleen <andi@firstfloor.org>,
	linux-kernel@vger.kernel.org, mingo@elte.hu, acme@redhat.com,
	ming.m.lin@intel.com, robert.richter@amd.com, ravitillo@lbl.gov,
	yrl.pp-manager.tt@hitachi.com,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@elte.hu>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Stephane Eranian <eranian@google.com>,
	Andi Kleen <andi@firstfloor.org>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Subject: [PATCH] x86: Fix insn decoder for longer instruction
Date: Fri, 07 Oct 2011 22:31:55 +0900	[thread overview]
Message-ID: <20111007133155.10933.58577.stgit@localhost.localdomain> (raw)
In-Reply-To: <4E8EE885.9040703@hitachi.com>

Fix x86 insn decoder for hardening against invalid length
instructions. This adds length checkings for each byte-read
site and if it exceeds MAX_INSN_SIZE, returns immediately.
This can happen when decoding user-space binary.

Caller can check whether it happened by checking insn.*.got
member is set or not.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---

 arch/x86/lib/insn.c |   48 +++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 9f33b98..374562e 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -22,14 +22,23 @@
 #include <asm/inat.h>
 #include <asm/insn.h>
 
-#define get_next(t, insn)	\
-	({t r; r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); r; })
+/* Verify next sizeof(t) bytes can be on the same instruction */
+#define validate_next(t, insn, n)	\
+	((insn)->next_byte + sizeof(t) + n - (insn)->kaddr <= MAX_INSN_SIZE)
+
+#define __get_next(t, insn)	\
+	({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); r; })
+
+#define __peek_nbyte_next(t, insn, n)	\
+	({ t r = *(t*)((insn)->next_byte + n); r; })
 
-#define peek_next(t, insn)	\
-	({t r; r = *(t*)insn->next_byte; r; })
+#define get_next(t, insn)	\
+	({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
 
 #define peek_nbyte_next(t, insn, n)	\
-	({t r; r = *(t*)((insn)->next_byte + n); r; })
+	({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
+
+#define peek_next(t, insn)	peek_nbyte_next(t, insn, 0)
 
 /**
  * insn_init() - initialize struct insn
@@ -158,6 +167,8 @@ vex_end:
 	insn->vex_prefix.got = 1;
 
 	prefixes->got = 1;
+
+err_out:
 	return;
 }
 
@@ -208,6 +219,9 @@ void insn_get_opcode(struct insn *insn)
 		insn->attr = 0;	/* This instruction is bad */
 end:
 	opcode->got = 1;
+
+err_out:
+	return;
 }
 
 /**
@@ -241,6 +255,9 @@ void insn_get_modrm(struct insn *insn)
 	if (insn->x86_64 && inat_is_force64(insn->attr))
 		insn->opnd_bytes = 8;
 	modrm->got = 1;
+
+err_out:
+	return;
 }
 
 
@@ -290,6 +307,9 @@ void insn_get_sib(struct insn *insn)
 		}
 	}
 	insn->sib.got = 1;
+
+err_out:
+	return;
 }
 
 
@@ -351,6 +371,9 @@ void insn_get_displacement(struct insn *insn)
 	}
 out:
 	insn->displacement.got = 1;
+
+err_out:
+	return;
 }
 
 /* Decode moffset16/32/64 */
@@ -373,6 +396,9 @@ static void __get_moffset(struct insn *insn)
 		break;
 	}
 	insn->moffset1.got = insn->moffset2.got = 1;
+
+err_out:
+	return;
 }
 
 /* Decode imm v32(Iz) */
@@ -389,6 +415,9 @@ static void __get_immv32(struct insn *insn)
 		insn->immediate.nbytes = 4;
 		break;
 	}
+
+err_out:
+	return;
 }
 
 /* Decode imm v64(Iv/Ov) */
@@ -411,6 +440,9 @@ static void __get_immv(struct insn *insn)
 		break;
 	}
 	insn->immediate1.got = insn->immediate2.got = 1;
+
+err_out:
+	return;
 }
 
 /* Decode ptr16:16/32(Ap) */
@@ -432,6 +464,9 @@ static void __get_immptr(struct insn *insn)
 	insn->immediate2.value = get_next(unsigned short, insn);
 	insn->immediate2.nbytes = 2;
 	insn->immediate1.got = insn->immediate2.got = 1;
+
+err_out:
+	return;
 }
 
 /**
@@ -496,6 +531,9 @@ void insn_get_immediate(struct insn *insn)
 	}
 done:
 	insn->immediate.got = 1;
+
+err_out:
+	return;
 }
 
 /**


  reply	other threads:[~2011-10-07 13:32 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-06 14:49 [PATCH 00/12] perf_events: add support for sampling taken branches Stephane Eranian
2011-10-06 14:49 ` [PATCH 01/12] perf_events: add generic taken branch sampling support Stephane Eranian
2011-10-06 16:57   ` Peter Zijlstra
2011-10-07 10:28     ` Stephane Eranian
2011-10-07 10:32       ` Peter Zijlstra
2011-10-07 10:44         ` Stephane Eranian
2011-10-06 17:01   ` Peter Zijlstra
2011-10-06 14:49 ` [PATCH 02/12] perf_events: add Intel LBR MSR definitions Stephane Eranian
2011-10-06 14:49 ` [PATCH 03/12] perf_events: add Intel X86 LBR sharing logic Stephane Eranian
2011-10-06 14:49 ` [PATCH 04/12] perf_events: sync branch stack sampling with X86 precise_sampling Stephane Eranian
2011-10-06 17:25   ` Peter Zijlstra
2011-10-07 10:34     ` Stephane Eranian
2011-10-07 10:37       ` Peter Zijlstra
2011-10-06 14:49 ` [PATCH 05/12] perf_events: add LBR mappings for PERF_SAMPLE_BRANCH filters Stephane Eranian
2011-10-06 14:49 ` [PATCH 06/12] perf_events: implement PERF_SAMPLE_BRANCH for Intel X86 Stephane Eranian
2011-10-06 17:54   ` Peter Zijlstra
2011-10-06 18:05   ` Peter Zijlstra
2011-10-06 14:49 ` [PATCH 07/12] perf_events: add LBR software filter support " Stephane Eranian
2011-10-06 15:32   ` Andi Kleen
2011-10-06 16:43     ` Peter Zijlstra
2011-10-06 17:14       ` Andi Kleen
2011-10-10  6:08         ` Ingo Molnar
2011-10-10  9:39           ` Peter Zijlstra
2011-10-07  7:06       ` Masami Hiramatsu
2011-10-07 10:38     ` Stephane Eranian
2011-10-07 10:40       ` Stephane Eranian
2011-10-07 10:42         ` Peter Zijlstra
2011-10-07 10:49           ` Stephane Eranian
2011-10-07 11:18             ` Peter Zijlstra
2011-10-07 11:21             ` Peter Zijlstra
2011-10-07 11:54               ` Masami Hiramatsu
2011-10-07 13:31                 ` Masami Hiramatsu [this message]
2011-10-10  7:04                   ` [PATCH] x86: Fix insn decoder for longer instruction Ingo Molnar
2011-10-10  6:09                 ` [PATCH 07/12] perf_events: add LBR software filter support for Intel X86 Ingo Molnar
2011-10-10 14:05                   ` Masami Hiramatsu
2011-10-10 14:45                     ` Andi Kleen
2011-10-11 12:59                       ` Masami Hiramatsu
2011-10-12  7:06                         ` Ingo Molnar
2011-10-13 10:54                           ` Masami Hiramatsu
2011-10-13 11:01                           ` [RFC PATCH] x86: Add a sanity test of x86 decoder Masami Hiramatsu
2011-10-18  6:54                             ` Ingo Molnar
2011-10-19  4:29                               ` Masami Hiramatsu
2011-10-19  6:44                                 ` Ingo Molnar
2011-10-20 14:01                                   ` [RFC PATCH v2 1/2] " Masami Hiramatsu
2011-11-18 23:16                                     ` [tip:perf/core] x86, perf: Add a build-time sanity test to the " tip-bot for Masami Hiramatsu
2011-10-20 14:01                                   ` [RFC PATCH v2 2/2] [RESEND] x86: Fix insn decoder for longer instruction Masami Hiramatsu
2011-10-07 15:42             ` [PATCH 07/12] perf_events: add LBR software filter support for Intel X86 Andi Kleen
2011-10-07 11:25       ` Masami Hiramatsu
2011-10-07 11:40         ` Peter Zijlstra
2011-10-07 15:44           ` Andi Kleen
2011-10-07 15:09         ` Andi Kleen
2011-10-07 16:05           ` Peter Zijlstra
2011-10-06 14:49 ` [PATCH 08/12] perf_events: disable PERF_SAMPLE_BRANCH_* when not supported Stephane Eranian
2011-10-06 18:53   ` Peter Zijlstra
2011-10-06 14:49 ` [PATCH 09/12] perf_events: add hook to flush branch_stack on context switch Stephane Eranian
2011-10-06 14:49 ` [PATCH 10/12] perf: add code to support PERF_SAMPLE_BRANCH_STACK Stephane Eranian
2011-10-06 18:50   ` Peter Zijlstra
2011-10-07 10:25     ` Stephane Eranian
2011-10-07 10:27       ` Peter Zijlstra
2011-10-06 14:49 ` [PATCH 11/12] perf: add support for sampling taken branch to perf record Stephane Eranian
2011-10-06 14:49 ` [PATCH 12/12] perf: add support for taken branch sampling to perf report Stephane Eranian
2011-10-06 15:25 ` [PATCH 00/12] perf_events: add support for sampling taken branches Andi Kleen
2011-10-07 10:23   ` Stephane Eranian
2011-10-06 18:32 ` Peter Zijlstra
2011-10-06 21:41   ` Stephane Eranian

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=20111007133155.10933.58577.stgit@localhost.localdomain \
    --to=masami.hiramatsu.pt@hitachi.com \
    --cc=acme@redhat.com \
    --cc=andi@firstfloor.org \
    --cc=eranian@google.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.m.lin@intel.com \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=ravitillo@lbl.gov \
    --cc=robert.richter@amd.com \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=yrl.pp-manager.tt@hitachi.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.