linux-kernel - [PATCH -tip] x86/kprobes: Handle removed INT3 in do

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <166933854220.2683864.10006153553442313230.stgit@devnote3>
Date:   Fri, 25 Nov 2022 10:09:02 +0900
From:   "Masami Hiramatsu (Google)" <mhiramat@...nel.org>
To:     x86@...nel.org
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        linux-kernel@...r.kernel.org, Steven Rostedt <rostedt@...dmis.org>,
        Huacai Chen <chenhuacai@...ngson.cn>,
        Jinyang He <hejinyang@...ngson.cn>,
        Tiezhu Yang <yangtiezhu@...ngson.cn>,
        "Naveen N . Rao" <naveen.n.rao@...ux.ibm.com>
Subject: [PATCH -tip] x86/kprobes: Handle removed INT3 in do_int3()

From: Masami Hiramatsu (Google) <mhiramat@...nel.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@...nel.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);