All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: x86@kernel.org, Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>, Borislav Petkov <bp@alien8.de>
Cc: Kees Cook <keescook@chromium.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	"H . Peter Anvin" <hpa@zytor.com>, Joerg Roedel <jroedel@suse.de>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	"Gustavo A . R . Silva" <gustavoars@kernel.org>,
	Jann Horn <jannh@google.com>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	Ricardo Neri <ricardo.neri-calderon@linux.intel.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2 1/3] x86/uprobes: Fix not using prefixes.nbytes for loop over prefixes.bytes
Date: Thu,  3 Dec 2020 13:50:37 +0900	[thread overview]
Message-ID: <160697103739.3146288.7437620795200799020.stgit@devnote2> (raw)
In-Reply-To: <160697102582.3146288.10127018634865687932.stgit@devnote2>

Since the insn.prefixes.nbytes can be bigger than the size of
insn.prefixes.bytes[] when a same prefix is repeated, we have to
check whether the insn.prefixes.bytes[i] != 0 and i < 4 instead
of insn.prefixes.nbytes.
This introduces for_each_insn_prefix() macro for this purpose.

Fixes: 2b1444983508 ("uprobes, mm, x86: Add the ability to install and remove uprobes breakpoints")
Reported-by: syzbot+9b64b619f10f19d19a7c@syzkaller.appspotmail.com
Debugged-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: stable@vger.kernel.org
---
 Changes in v2:
  - Add for_each_insn_prefix() macro and fix to check index first.
---
 arch/x86/include/asm/insn.h |   15 +++++++++++++++
 arch/x86/kernel/uprobes.c   |   10 ++++++----
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h
index 5c1ae3eff9d4..e6b38ebd3a1c 100644
--- a/arch/x86/include/asm/insn.h
+++ b/arch/x86/include/asm/insn.h
@@ -201,6 +201,21 @@ static inline int insn_offset_immediate(struct insn *insn)
 	return insn_offset_displacement(insn) + insn->displacement.nbytes;
 }
 
+/**
+ * for_each_insn_prefix() -- Iterate prefixes in the instruction
+ * @insn: Pointer to struct insn.
+ * @idx:  Index storage.
+ * @prefix: Prefix byte.
+ *
+ * Iterate prefix bytes of given @insn. Each prefix byte is stored in @prefix
+ * and the index is stored in @idx (note that this @idx is just for a cursor,
+ * do not change it.)
+ * Since prefixes.nbytes can be bigger than 4 if some prefixes are repeated,
+ * we can not use it for looping over the prefixes.
+ */
+#define for_each_insn_prefix(insn, idx, prefix)	\
+	for (idx = 0; idx < 4 && (prefix = insn->prefixes.bytes[idx]) != 0; idx++)
+
 #define POP_SS_OPCODE 0x1f
 #define MOV_SREG_OPCODE 0x8e
 
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 3fdaa042823d..138bdb1fd136 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -255,12 +255,13 @@ static volatile u32 good_2byte_insns[256 / 32] = {
 
 static bool is_prefix_bad(struct insn *insn)
 {
+	insn_byte_t p;
 	int i;
 
-	for (i = 0; i < insn->prefixes.nbytes; i++) {
+	for_each_insn_prefix(insn, i, p) {
 		insn_attr_t attr;
 
-		attr = inat_get_opcode_attribute(insn->prefixes.bytes[i]);
+		attr = inat_get_opcode_attribute(p);
 		switch (attr) {
 		case INAT_MAKE_PREFIX(INAT_PFX_ES):
 		case INAT_MAKE_PREFIX(INAT_PFX_CS):
@@ -715,6 +716,7 @@ static const struct uprobe_xol_ops push_xol_ops = {
 static int branch_setup_xol_ops(struct arch_uprobe *auprobe, struct insn *insn)
 {
 	u8 opc1 = OPCODE1(insn);
+	insn_byte_t p;
 	int i;
 
 	switch (opc1) {
@@ -746,8 +748,8 @@ static int branch_setup_xol_ops(struct arch_uprobe *auprobe, struct insn *insn)
 	 * Intel and AMD behavior differ in 64-bit mode: Intel ignores 66 prefix.
 	 * No one uses these insns, reject any branch insns with such prefix.
 	 */
-	for (i = 0; i < insn->prefixes.nbytes; i++) {
-		if (insn->prefixes.bytes[i] == 0x66)
+	for_each_insn_prefix(insn, i, p) {
+		if (p == 0x66)
 			return -ENOTSUPP;
 	}
 


  reply	other threads:[~2020-12-03  4:51 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-03  4:50 [PATCH v2 0/3] x86/insn: Fix not using prefixes.nbytes for loop over prefixes.bytes Masami Hiramatsu
2020-12-03  4:50 ` Masami Hiramatsu [this message]
2020-12-03 12:37   ` [PATCH v2 1/3] x86/uprobes: " Borislav Petkov
2020-12-03 12:41     ` Borislav Petkov
2020-12-03 12:48       ` Borislav Petkov
2020-12-03 16:45         ` Tom Lendacky
2020-12-03 16:54           ` Borislav Petkov
2020-12-03 17:01             ` Borislav Petkov
2020-12-03 18:10               ` Tom Lendacky
2020-12-03 18:17                 ` Borislav Petkov
2020-12-03 18:49                   ` Tom Lendacky
2020-12-04  0:56                     ` Masami Hiramatsu
2020-12-04  3:55                       ` Masami Hiramatsu
2020-12-04 11:06                       ` Borislav Petkov
2020-12-04 11:28                         ` Masami Hiramatsu
2020-12-04  0:16           ` Masami Hiramatsu
2020-12-04  0:18     ` Masami Hiramatsu
2020-12-04 15:04   ` [tip: x86/urgent] x86/uprobes: Do not use prefixes.nbytes when looping " tip-bot2 for Masami Hiramatsu
2020-12-05  0:12     ` Masami Hiramatsu
2020-12-05 10:17       ` Borislav Petkov
2020-12-06  3:53         ` Masami Hiramatsu
2020-12-06  9:02           ` Borislav Petkov
2020-12-09 18:01             ` Arnaldo Carvalho de Melo
2020-12-10 10:36               ` Borislav Petkov
2020-12-09 18:05       ` Arnaldo Carvalho de Melo
2020-12-06  9:09   ` tip-bot2 for Masami Hiramatsu
2020-12-03  4:50 ` [PATCH v2 2/3] x86/insn-eval: Fix not using prefixes.nbytes for loop " Masami Hiramatsu
2020-12-04 15:04   ` [tip: x86/urgent] x86/insn-eval: Use new for_each_insn_prefix() macro to loop over prefixes bytes tip-bot2 for Masami Hiramatsu
2020-12-06  9:09   ` tip-bot2 for Masami Hiramatsu
2020-12-03  4:51 ` [PATCH v2 3/3] x86/sev-es: Fix not using prefixes.nbytes for loop over prefixes.bytes Masami Hiramatsu
2020-12-04 15:04   ` [tip: x86/urgent] x86/sev-es: Use new for_each_insn_prefix() macro to loop over prefixes bytes tip-bot2 for Masami Hiramatsu
2020-12-06  9:09   ` tip-bot2 for Masami Hiramatsu

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=160697103739.3146288.7437620795200799020.stgit@devnote2 \
    --to=mhiramat@kernel.org \
    --cc=bp@alien8.de \
    --cc=gustavoars@kernel.org \
    --cc=hpa@zytor.com \
    --cc=jannh@google.com \
    --cc=jroedel@suse.de \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=ricardo.neri-calderon@linux.intel.com \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=thomas.lendacky@amd.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 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.