All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: x86@kernel.org, LKML <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>, Ingo Molnar <mingo@kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H . Peter Anvin" <hpa@zytor.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	"David S . Miller" <davem@davemloft.net>,
	Steven Rostedt <rostedt@goodmis.org>,
	Francis Deslauriers <francis.deslauriers@efficios.com>,
	Ricardo Neri <ricardo.neri-calderon@linux.intel.com>,
	Borislav Petkov <bp@suse.de>, Yonghong Song <yhs@fb.com>
Subject: [PATCH 1/2] kprobes: x86: Prohibit probing on exception masking instructions
Date: Wed,  9 May 2018 21:58:15 +0900	[thread overview]
Message-ID: <152587069574.17316.3311695234863248641.stgit@devbox> (raw)
In-Reply-To: <152587066475.17316.3035446966107675608.stgit@devbox>

Since MOV SS and POP SS instructions will delay the exceptions
until the next instruction is executed, we should not do
single-stepping on it by kprobes.

However, kprobes usually executes those instructions directly
on trampoline buffer (a.k.a. kprobe-booster), except for the
kprobes which has post_handler. Thus if kprobe user probes MOV SS
with post_handler, it will do single-stepping on the MOV SS.

This means it is safe that if we use it via ftrace or perf/bpf
since those don't use the post_handler.

Anyway, since the stack switching is a rare case, it is safer
just rejecting kprobes on such instructions.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 arch/x86/include/asm/insn.h    |   18 ++++++++++++++++++
 arch/x86/kernel/kprobes/core.c |    4 ++++
 2 files changed, 22 insertions(+)

diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h
index b3e32b010ab1..c2c01f84df75 100644
--- a/arch/x86/include/asm/insn.h
+++ b/arch/x86/include/asm/insn.h
@@ -208,4 +208,22 @@ static inline int insn_offset_immediate(struct insn *insn)
 	return insn_offset_displacement(insn) + insn->displacement.nbytes;
 }
 
+#define POP_SS_OPCODE 0x1f
+#define MOV_SREG_OPCODE 0x8e
+
+/*
+ * Intel SDM Vol.3A 6.8.3 states;
+ * "Any single-step trap that would be delivered following the MOV to SS
+ * instruction or POP to SS instruction (because EFLAGS.TF is 1) is
+ * suppressed."
+ * This function returns true if @insn is MOV SS or POP SS. On these
+ * instructions, single stepping is suppressed.
+ */
+static inline int insn_masking_exception(struct insn *insn)
+{
+	return insn->opcode.bytes[0] == POP_SS_OPCODE ||
+		(insn->opcode.bytes[0] == MOV_SREG_OPCODE &&
+		 X86_MODRM_REG(insn->modrm.bytes[0]) == 2);
+}
+
 #endif /* _ASM_X86_INSN_H */
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 0715f827607c..6f4d42377fe5 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -370,6 +370,10 @@ int __copy_instruction(u8 *dest, u8 *src, u8 *real, struct insn *insn)
 	if (insn->opcode.bytes[0] == BREAKPOINT_INSTRUCTION)
 		return 0;
 
+	/* We should not singlestep on the exception masking instructions */
+	if (insn_masking_exception(insn))
+		return 0;
+
 #ifdef CONFIG_X86_64
 	/* Only x86_64 has RIP relative instructions */
 	if (insn_rip_relative(insn)) {

  reply	other threads:[~2018-05-09 12:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-09 12:57 [PATCH 0/2] uprobes: x86: Reject probing MOV SS Masami Hiramatsu
2018-05-09 12:58 ` Masami Hiramatsu [this message]
2018-05-13 17:57   ` [tip:x86/urgent] kprobes/x86: Prohibit probing on exception masking instructions tip-bot for Masami Hiramatsu
2018-05-09 12:58 ` [PATCH 2/2] uprobes: x86: Prohibit probing on MOV SS instruction Masami Hiramatsu
2018-05-09 16:39   ` Oleg Nesterov
2018-05-13 17:58   ` [tip:x86/urgent] uprobes/x86: " tip-bot for Masami Hiramatsu
2018-05-09 13:01 ` [PATCH 0/2] uprobes: kprobes: x86: Reject probing MOV SS/POP SS Masami Hiramatsu
2018-05-09 14:36 ` [PATCH 0/2] uprobes: x86: Reject probing MOV SS Andy Lutomirski
2018-05-09 22:25   ` 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=152587069574.17316.3311695234863248641.stgit@devbox \
    --to=mhiramat@kernel.org \
    --cc=ast@kernel.org \
    --cc=bp@suse.de \
    --cc=davem@davemloft.net \
    --cc=francis.deslauriers@efficios.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=ricardo.neri-calderon@linux.intel.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    --cc=yhs@fb.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.