[PATCH -tip] x86/kprobes: Handle removed INT3 in do_int3()

Masami Hiramatsu (Google)
2022-11-25T01:09:43Z

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Since x86 doesn't use stop_machine() to patch the kernel text,
there is a small chance that the another CPU removes the INT3
during do_int3(). In this case, if no INT3 notifier callbacks
handled that, the kernel calls die() because of a stray INT3.

Currently this is checked and recovered in the kprobe_int3_handler(),
but this is wrong because;

- If CONFIG_KPROBES is not set, kernel does not handle this case.

- After kprobe_int3_handler() ignores that INT3, that can be
removed before notify_die(DIE_INT3). And if the callback misses
it, kernel dies.

- It skips the INT3 notifier callbacks if the INT3 is NOT managed
by the kprobes. Another callback may be able to handle it.

Thus, move the removed INT3 recovering code to do_int3(),
after calling all callbacks.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
arch/x86/kernel/kprobes/core.c | 15 +--------------
arch/x86/kernel/traps.c | 15 +++++++++++++++
2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 66299682b6b7..aa414224ac8a 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -987,20 +987,7 @@ int kprobe_int3_handler(struct pt_regs *regs)
return 1;
}
}
-
- if (*addr != INT3_INSN_OPCODE) {
- /*
- * The breakpoint instruction was removed right
- * after we hit it. Another cpu has removed
- * either a probepoint or a debugger breakpoint
- * at this address. In either case, no further
- * handling of this interrupt is appropriate.
- * Back up over the (now missing) int3 and run
- * the original instruction.
- */
- regs->ip = (unsigned long)addr;
- return 1;
- } /* else: not a kprobe fault; let the kernel handle it */
+ /* This may not a kprobe fault; let the kernel handle it */

return 0;
}
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 8b83d8fbce71..2d22379bdf66 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -788,6 +788,7 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)

static bool do_int3(struct pt_regs *regs)
{
+ unsigned long addr = instruction_pointer(regs) - INT3_INSN_SIZE;
int res;

#ifdef CONFIG_KGDB_LOW_LEVEL_TRAP
@@ -802,6 +803,20 @@ static bool do_int3(struct pt_regs *regs)
#endif
res = notify_die(DIE_INT3, "int3", regs, 0, X86_TRAP_BP, SIGTRAP);

+ if (unlikely(res != NOTIFY_STOP)) {
+ if (*(u8 *)addr != INT3_INSN_OPCODE) {
+ /*
+ * Another CPU removed the INT3 instruction before
+ * callbacks handle it. This is not a stray INT3
+ * but recoverable.
+ * Back up over the (now missing) INT3 and run
+ * the original instruction.
+ */
+ regs->ip = addr;
+ return true;
+ }
+ }
+
return res == NOTIFY_STOP;
}
NOKPROBE_SYMBOL(do_int3);