uprobes: x86: Reject probing MOV SS

Hi,

I found the CVE-2018-1087 and CVE-2018-8897 should be related to kprobes and uprobes too, since both are using #DB for single stepping.

I decided to just reject probes on MOV SS and POP SS because those are not recommended to use (Intel SDM recommend to use LSS instead), thus it might be rare case.

Oleg, could you review the uprobes patch?

Thank you,

---

Masami Hiramatsu (2):
kprobes: x86: Prohibit probing on exception masking instructions
uprobes: x86: Prohibit probing on MOV SS instruction

 arch/x86/include/asm/insn.h    |   18 ++++++++++++++++++
 arch/x86/kernel/kprobes/core.c |    4 ++++
 arch/x86/kernel/uprobes.c      |    4 ++++
 3 files changed, 26 insertions(+)

--
Masami Hiramatsu (Linaro) <[email protected]>

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.

---
 arch/x86/include/asm/insn.h    |   18 ++++++++++++++++++
 arch/x86/kernel/kprobes/core.c |    4 ++++
 2 files changed, 22 insertions(+)

index b3e32b010ab1..c2c01f84df75 100644
@@ -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 */

index 0715f827607c..6f4d42377fe5 100644
@@ -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)) {

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 uprobes.

uprobe already rejects probing on POP SS (0x1f), but allows probing on MOV SS (0x8e and reg == 2).
This checks the target instruction and if it is MOV SS or POP SS, returns -ENOTSUPP to reject probing.

---
 arch/x86/kernel/uprobes.c |    4 ++++
 1 file changed, 4 insertions(+)

index 85c7ef23d99f..c84bb5396958 100644
@@ -299,6 +299,10 @@ static int uprobe_init_insn(struct arch_uprobe *auprobe, struct insn *insn, bool
 	if (is_prefix_bad(insn))
 		return -ENOTSUPP;
 
+	/* We should not singlestep on the exception masking instructions */
+	if (insn_masking_exception(insn))
+		return -ENOTSUPP;
+
 	if (x86_64)
 		good_insns = good_insns_64;
 	else

...

Oops, I missed adding kprobes: and POP SS on subject...

Masami Hiramatsu <[email protected]>

Seems okay to me. I doubt we have a security problem, but I can easily imagine that the probes wouldn't work right.

Sorry for confusion, yes, that will not be a security issue, since kprobes and uprobes can be used only by root user.

Thank you,

Masami Hiramatsu <[email protected]>