[RFC PATCH 0/9] arm64: Make the kprobes debug exception path noinstr

[RFC PATCH 0/9] arm64: Make the kprobes debug exception path noinstr

[Hongyan Xia]

From: Hongyan Xia <hongyan.xia@transsion.com>

Commit 879a6754d3d1 ("arm64: kprobes: Only handle faults originating
from XOL slot") and 23f851ac0078 ("arm64: kprobes: Allow reentering
kprobes while single-stepping") band-aided two corner cases in the arm64
kprobe path: Unrelated page faults inside Kprobe and nested Kprobe SS
handling. Although the two patches were taken, Will asked whether we
should keep patching things like this each time we find a corner case,
and whether making Kprobe noinstr is the correct answer [1].

This series attempts to do exactly that: Making the Kprobe path noinstr.
The kprobe handlers become noinstr end to end, and everything that is
inherently instrumentable is confined to instrumentation_begin/end()
windows, similar to the x86 side. After this series, the two
corner-cases mentioned above are also gone, hence the revert in 7/9 and
8/9. Some of the instrumentation_begin/end() sections are suggested by
AI, so they may need careful review.

Note that arm64 has no objtool to validate noinstr, so I wrote a small
script finding bl/blr and validated that their calls are confined within
noinstr. This is exactly what found the Clang outline compile problem
that motivated patch 9/9.

One question is whether other BRK paths get the same treatment.
Currently they stay instrumentable inside a big window.

[1] https://lore.kernel.org/all/alpuL10h7-OK2hFb@willie-the-truck/

Hongyan Xia (9):
  arm64/entry: Bound certain debug exception paths in instrumentation
    windows
  arm64/entry: Make debug_exception_enter/exit() noinstr
  arm64/debug-monitors: Make do_el1_brk64()/do_el1_softstep() noinstr
  arm64/kprobes: Make the single-step machinery noinstr
  arm64/kprobes: Invoke pre/post handlers inside instrumentation
  arm64/kprobes: Make kprobe_fault_handler() noinstr
  arm64/kprobes: Drop the KPROBE_HIT_SS reentry special case
  arm64/kprobes: Drop the XOL single-step fault PC check
  arm64/debug: Mark debug exception helpers __always_inline

 arch/arm64/include/asm/esr.h       |  2 +-
 arch/arm64/include/asm/kprobes.h   | 15 +----
 arch/arm64/include/asm/percpu.h    |  2 +-
 arch/arm64/include/asm/preempt.h   |  4 +-
 arch/arm64/include/asm/ptrace.h    |  4 +-
 arch/arm64/kernel/debug-monitors.c | 76 ++++++++++++-----------
 arch/arm64/kernel/entry-common.c   | 43 +++++++++----
 arch/arm64/kernel/probes/kprobes.c | 96 ++++++++++++++----------------
 include/linux/kprobes.h            |  8 +--
 9 files changed, 131 insertions(+), 119 deletions(-)

-- 
2.47.3

[RFC PATCH 1/9] arm64/entry: Bound certain debug exception paths in instrumentation windows

[Hongyan Xia]

From: Hongyan Xia <hongyan.xia@transsion.com>

The el1 debug exception handlers are noinstr, but their payload calls
(do_breakpoint(), do_watchpoint(), try_step_suspended_breakpoints()
and the Cortex-A76 erratum handler) are ordinary instrumentable code.

Mark the boundaries explicitly with instrumentation_begin()/end(), in
the same style as x86 exception entries.

The BRK handlers (do_el1_brk64(), do_el1_softstep()) will be dealt with
in later patches.

Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
---
 arch/arm64/kernel/entry-common.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index ceb4eb11232a..466529cce1c3 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/context_tracking.h>
+#include <linux/instrumentation.h>
 #include <linux/irq-entry-common.h>
 #include <linux/kasan.h>
 #include <linux/linkage.h>
@@ -380,7 +381,9 @@ static void noinstr el1_breakpt(struct pt_regs *regs, unsigned long esr)
 
 	state = arm64_enter_el1_dbg(regs);
 	debug_exception_enter(regs);
+	instrumentation_begin();
 	do_breakpoint(esr, regs);
+	instrumentation_end();
 	debug_exception_exit(regs);
 	arm64_exit_el1_dbg(regs, state);
 }
@@ -388,9 +391,15 @@ static void noinstr el1_breakpt(struct pt_regs *regs, unsigned long esr)
 static void noinstr el1_softstp(struct pt_regs *regs, unsigned long esr)
 {
 	irqentry_state_t state;
+	bool handled;
 
 	state = arm64_enter_el1_dbg(regs);
-	if (!cortex_a76_erratum_1463225_debug_handler(regs)) {
+
+	instrumentation_begin();
+	handled = cortex_a76_erratum_1463225_debug_handler(regs);
+	instrumentation_end();
+
+	if (!handled) {
 		debug_exception_enter(regs);
 		/*
 		 * After handling a breakpoint, we suspend the breakpoint
@@ -398,7 +407,11 @@ static void noinstr el1_softstp(struct pt_regs *regs, unsigned long esr)
 		 * If we are stepping a suspended breakpoint there's nothing more to do:
 		 * the single-step is complete.
 		 */
-		if (!try_step_suspended_breakpoints(regs))
+		instrumentation_begin();
+		handled = try_step_suspended_breakpoints(regs);
+		instrumentation_end();
+
+		if (!handled)
 			do_el1_softstep(esr, regs);
 		debug_exception_exit(regs);
 	}
@@ -413,7 +426,9 @@ static void noinstr el1_watchpt(struct pt_regs *regs, unsigned long esr)
 
 	state = arm64_enter_el1_dbg(regs);
 	debug_exception_enter(regs);
+	instrumentation_begin();
 	do_watchpoint(far, esr, regs);
+	instrumentation_end();
 	debug_exception_exit(regs);
 	arm64_exit_el1_dbg(regs, state);
 }
-- 
2.47.3

[RFC PATCH 2/9] arm64/entry: Make debug_exception_enter/exit() noinstr

[Hongyan Xia]

From: Hongyan Xia <hongyan.xia@transsion.com>

Commit 879a6754d3d11e30af24b7dc486f561510d62641 ran into a crash because
debug_exception_enter/exit() triggered page faults caused by perf dwarf
call graph tracing. That patch was a band-aid on top. Instead of trying
to band-aid all possible paths that can happen during instrumentation or
perf tracing, simply make both functions noinstr.

Drop the RCU_LOCKDEP_WARN(): arm64_enter_el1_dbg() runs first and
enters NMI context via ct_nmi_enter(), so RCU is always watching by the
time debug_exception_enter() runs.

Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
---
 arch/arm64/kernel/entry-common.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 466529cce1c3..f06b5e2437cd 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -293,20 +293,26 @@ static __always_inline void fpsimd_syscall_exit(void)
  * accidentally schedule in exception context and it will force a warning
  * if we somehow manage to schedule by accident.
  */
-static void debug_exception_enter(struct pt_regs *regs)
+static void noinstr debug_exception_enter(struct pt_regs *regs)
 {
-	preempt_disable();
-
-	/* This code is a bit fragile.  Test it. */
-	RCU_LOCKDEP_WARN(!rcu_is_watching(), "exception_enter didn't work");
+	/*
+	 * debug_exception_enter/exit() can be quite delicate. The normal
+	 * preempt_disable/enable() can be instrumented or traced by perf,
+	 * which leads to a can of worms including triggering page faults.
+	 *
+	 * Instead of trying to make all of them work properly here, just
+	 * open-code the simpler version of preempt_disable/enable() and make
+	 * enter/exit() both noinstr to avoid the entire complexity.
+	 */
+	__preempt_count_inc();
+	barrier();
 }
-NOKPROBE_SYMBOL(debug_exception_enter);
 
-static void debug_exception_exit(struct pt_regs *regs)
+static void noinstr debug_exception_exit(struct pt_regs *regs)
 {
-	preempt_enable_no_resched();
+	barrier();
+	__preempt_count_dec();
 }
-NOKPROBE_SYMBOL(debug_exception_exit);
 
 UNHANDLED(el1t, 64, sync)
 UNHANDLED(el1t, 64, irq)
-- 
2.47.3

[RFC PATCH 3/9] arm64/debug-monitors: Make do_el1_brk64()/do_el1_softstep() noinstr

[Hongyan Xia]

From: Hongyan Xia <hongyan.xia@transsion.com>

Convert do_el1_brk64(), do_el1_softstep() and call_el1_break_hook() to
noinstr. The kprobe and kretprobe BRK handlers (converted to noinstr in
the following patches) are dispatched directly. Every other BRK handler
are ordinary instrumentable code and now run bounded by
instrumentation_begin()/end().

With this, everything on the el1 debug exception path from the vectors
down to the kprobe handlers is noinstr, and instrumentation only runs
inside explicit instrumentation windows.

Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
---
 arch/arm64/kernel/debug-monitors.c | 74 +++++++++++++++++-------------
 1 file changed, 41 insertions(+), 33 deletions(-)

diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 29307642f4c9..a970ab6327cd 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -11,6 +11,7 @@
 #include <linux/debugfs.h>
 #include <linux/hardirq.h>
 #include <linux/init.h>
+#include <linux/instrumentation.h>
 #include <linux/ptrace.h>
 #include <linux/kprobes.h>
 #include <linux/stat.h>
@@ -193,59 +194,65 @@ void do_el0_softstep(unsigned long esr, struct pt_regs *regs)
 	user_rewind_single_step(current);
 }
 
-void do_el1_softstep(unsigned long esr, struct pt_regs *regs)
+void noinstr do_el1_softstep(unsigned long esr, struct pt_regs *regs)
 {
-	if (kgdb_single_step_handler(regs, esr) == DBG_HOOK_HANDLED)
+	int handled;
+
+	instrumentation_begin();
+	handled = kgdb_single_step_handler(regs, esr);
+	instrumentation_end();
+
+	if (handled == DBG_HOOK_HANDLED)
 		return;
 
+	instrumentation_begin();
 	pr_warn("Unexpected kernel single-step exception at EL1\n");
+	instrumentation_end();
 	/*
 	 * Re-enable stepping since we know that we will be
 	 * returning to regs.
 	 */
 	set_regs_spsr_ss(regs);
 }
-NOKPROBE_SYMBOL(do_el1_softstep);
 
-static int call_el1_break_hook(struct pt_regs *regs, unsigned long esr)
+static int noinstr call_el1_break_hook(struct pt_regs *regs, unsigned long esr)
 {
-	if (esr_brk_comment(esr) == BUG_BRK_IMM)
-		return bug_brk_handler(regs, esr);
-
-	if (IS_ENABLED(CONFIG_CFI) && esr_is_cfi_brk(esr))
-		return cfi_brk_handler(regs, esr);
-
-	if (esr_brk_comment(esr) == FAULT_BRK_IMM)
-		return reserved_fault_brk_handler(regs, esr);
-
-	if (IS_ENABLED(CONFIG_KASAN_SW_TAGS) &&
-		(esr_brk_comment(esr) & ~KASAN_BRK_MASK) == KASAN_BRK_IMM)
-		return kasan_brk_handler(regs, esr);
-
-	if (IS_ENABLED(CONFIG_UBSAN_TRAP) && esr_is_ubsan_brk(esr))
-		return ubsan_brk_handler(regs, esr);
-
-	if (IS_ENABLED(CONFIG_KGDB)) {
-		if (esr_brk_comment(esr) == KGDB_DYN_DBG_BRK_IMM)
-			return kgdb_brk_handler(regs, esr);
-		if (esr_brk_comment(esr) == KGDB_COMPILED_DBG_BRK_IMM)
-			return kgdb_compiled_brk_handler(regs, esr);
-	}
+	unsigned long comment = esr_brk_comment(esr);
+	int ret = DBG_HOOK_ERROR;
 
 	if (IS_ENABLED(CONFIG_KPROBES)) {
-		if (esr_brk_comment(esr) == KPROBES_BRK_IMM)
+		if (comment == KPROBES_BRK_IMM)
 			return kprobe_brk_handler(regs, esr);
-		if (esr_brk_comment(esr) == KPROBES_BRK_SS_IMM)
+		if (comment == KPROBES_BRK_SS_IMM)
 			return kprobe_ss_brk_handler(regs, esr);
 	}
 
 	if (IS_ENABLED(CONFIG_KRETPROBES) &&
-		esr_brk_comment(esr) == KRETPROBES_BRK_IMM)
+	    comment == KRETPROBES_BRK_IMM)
 		return kretprobe_brk_handler(regs, esr);
 
-	return DBG_HOOK_ERROR;
+	instrumentation_begin();
+	if (comment == BUG_BRK_IMM)
+		ret = bug_brk_handler(regs, esr);
+	else if (IS_ENABLED(CONFIG_CFI) && esr_is_cfi_brk(esr))
+		ret = cfi_brk_handler(regs, esr);
+	else if (comment == FAULT_BRK_IMM)
+		ret = reserved_fault_brk_handler(regs, esr);
+	else if (IS_ENABLED(CONFIG_KASAN_SW_TAGS) &&
+		 (comment & ~KASAN_BRK_MASK) == KASAN_BRK_IMM)
+		ret = kasan_brk_handler(regs, esr);
+	else if (IS_ENABLED(CONFIG_UBSAN_TRAP) && esr_is_ubsan_brk(esr))
+		ret = ubsan_brk_handler(regs, esr);
+	else if (IS_ENABLED(CONFIG_KGDB)) {
+		if (comment == KGDB_DYN_DBG_BRK_IMM)
+			ret = kgdb_brk_handler(regs, esr);
+		else if (comment == KGDB_COMPILED_DBG_BRK_IMM)
+			ret = kgdb_compiled_brk_handler(regs, esr);
+	}
+	instrumentation_end();
+
+	return ret;
 }
-NOKPROBE_SYMBOL(call_el1_break_hook);
 
 /*
  * We have already unmasked interrupts and enabled preemption
@@ -261,14 +268,15 @@ void do_el0_brk64(unsigned long esr, struct pt_regs *regs)
 	send_user_sigtrap(TRAP_BRKPT);
 }
 
-void do_el1_brk64(unsigned long esr, struct pt_regs *regs)
+void noinstr do_el1_brk64(unsigned long esr, struct pt_regs *regs)
 {
 	if (call_el1_break_hook(regs, esr) == DBG_HOOK_HANDLED)
 		return;
 
+	instrumentation_begin();
 	die("Oops - BRK", regs, esr);
+	instrumentation_end();
 }
-NOKPROBE_SYMBOL(do_el1_brk64);
 
 #ifdef CONFIG_COMPAT
 void do_bkpt32(unsigned long esr, struct pt_regs *regs)
-- 
2.47.3

[RFC PATCH 4/9] arm64/kprobes: Make the single-step machinery noinstr

[Hongyan Xia]

From: Hongyan Xia <hongyan.xia@transsion.com>

Convert the core of the arm64 kprobe single-step flow to noinstr:
save_previous_kprobe(), restore_previous_kprobe(), set_current_kprobe(),
kprobes_save_local_irqflag(), kprobes_restore_local_irqflag(),
setup_singlestep(), reenter_kprobe() and post_kprobe_handler().

These functions only touch per-cpu state, pt_regs and DAIF, except for:

- kprobes_inc_nmissed_count(), which is generic and instrumentable; add
  an arm64-local wrapper that calls it bounded by
  instrumentation_begin()/end();
- the instruction simulation path in setup_singlestep(), whose decode
  handlers are instrumentable; bound arch_simulate_insn() with an
  instrumentation window;
- the unrecoverable-reentry pr_warn()/dump_kprobe()/BUG() path and the
  default WARN_ON(), both bounded with instrumentation windows.

The __kprobes attribute (notrace + .kprobes.text) is replaced by
noinstr, which is a strict superset for these functions.

Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
---
 arch/arm64/kernel/probes/kprobes.c | 39 ++++++++++++++++++------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 4e0efad5caf2..1b12341b2af3 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -14,6 +14,7 @@
 #include <linux/extable.h>
 #include <linux/kasan.h>
 #include <linux/kernel.h>
+#include <linux/instrumentation.h>
 #include <linux/kprobes.h>
 #include <linux/sched/debug.h>
 #include <linux/set_memory.h>
@@ -39,7 +40,7 @@
 DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
 DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
 
-static void __kprobes
+static void noinstr
 post_kprobe_handler(struct kprobe *, struct kprobe_ctlblk *, struct pt_regs *);
 
 void *alloc_insn_page(void)
@@ -170,7 +171,7 @@ void __kprobes arch_remove_kprobe(struct kprobe *p)
 	}
 }
 
-static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)
+static void noinstr save_previous_kprobe(struct kprobe_ctlblk *kcb)
 {
 	kcb->prev_kprobe.kp = kprobe_running();
 	kcb->prev_kprobe.status = kcb->kprobe_status;
@@ -184,7 +185,7 @@ static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)
 	kcb->prev_kprobe.saved_irqflag = kcb->saved_irqflag;
 }
 
-static void __kprobes restore_previous_kprobe(struct kprobe_ctlblk *kcb)
+static void noinstr restore_previous_kprobe(struct kprobe_ctlblk *kcb)
 {
 	__this_cpu_write(current_kprobe, kcb->prev_kprobe.kp);
 	kcb->kprobe_status = kcb->prev_kprobe.status;
@@ -197,7 +198,7 @@ static void __kprobes restore_previous_kprobe(struct kprobe_ctlblk *kcb)
 	kcb->saved_irqflag = kcb->prev_kprobe.saved_irqflag;
 }
 
-static void __kprobes set_current_kprobe(struct kprobe *p)
+static void noinstr set_current_kprobe(struct kprobe *p)
 {
 	__this_cpu_write(current_kprobe, p);
 }
@@ -207,23 +208,23 @@ static void __kprobes set_current_kprobe(struct kprobe *p)
  * simple and avoid nesting exceptions. Interrupts do have to be disabled since
  * the kprobe state is per-CPU and doesn't get migrated.
  */
-static void __kprobes kprobes_save_local_irqflag(struct kprobe_ctlblk *kcb,
-						struct pt_regs *regs)
+static void noinstr kprobes_save_local_irqflag(struct kprobe_ctlblk *kcb,
+					       struct pt_regs *regs)
 {
 	kcb->saved_irqflag = regs->pstate & DAIF_MASK;
 	regs->pstate |= DAIF_MASK;
 }
 
-static void __kprobes kprobes_restore_local_irqflag(struct kprobe_ctlblk *kcb,
-						struct pt_regs *regs)
+static void noinstr kprobes_restore_local_irqflag(struct kprobe_ctlblk *kcb,
+						  struct pt_regs *regs)
 {
 	regs->pstate &= ~DAIF_MASK;
 	regs->pstate |= kcb->saved_irqflag;
 }
 
-static void __kprobes setup_singlestep(struct kprobe *p,
-				       struct pt_regs *regs,
-				       struct kprobe_ctlblk *kcb, int reenter)
+static void noinstr setup_singlestep(struct kprobe *p,
+				     struct pt_regs *regs,
+				     struct kprobe_ctlblk *kcb, int reenter)
 {
 	unsigned long slot;
 
@@ -244,13 +245,15 @@ static void __kprobes setup_singlestep(struct kprobe *p,
 		instruction_pointer_set(regs, slot);
 	} else {
 		/* insn simulation */
+		instrumentation_begin();
 		arch_simulate_insn(p, regs);
+		instrumentation_end();
 	}
 }
 
-static int __kprobes reenter_kprobe(struct kprobe *p,
-				    struct pt_regs *regs,
-				    struct kprobe_ctlblk *kcb)
+static int noinstr reenter_kprobe(struct kprobe *p,
+				  struct pt_regs *regs,
+				  struct kprobe_ctlblk *kcb)
 {
 	switch (kcb->kprobe_status) {
 	case KPROBE_HIT_SSDONE:
@@ -262,23 +265,29 @@ static int __kprobes reenter_kprobe(struct kprobe *p,
 		 * recoverable one-level reentry, so handle it in the same way as
 		 * reentry from KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE.
 		 */
+		instrumentation_begin();
 		kprobes_inc_nmissed_count(p);
+		instrumentation_end();
 		setup_singlestep(p, regs, kcb, 1);
 		break;
 	case KPROBE_REENTER:
+		instrumentation_begin();
 		pr_warn("Failed to recover from reentered kprobes.\n");
 		dump_kprobe(p);
 		BUG();
+		instrumentation_end();
 		break;
 	default:
+		instrumentation_begin();
 		WARN_ON(1);
+		instrumentation_end();
 		return 0;
 	}
 
 	return 1;
 }
 
-static void __kprobes
+static void noinstr
 post_kprobe_handler(struct kprobe *cur, struct kprobe_ctlblk *kcb, struct pt_regs *regs)
 {
 	/* return addr restore if non-branching insn */
-- 
2.47.3

[RFC PATCH 5/9] arm64/kprobes: Invoke pre/post handlers inside instrumentation

[Hongyan Xia]

From: Hongyan Xia <hongyan.xia@transsion.com>

The kprobe pre_handler() and post_handler() are arbitrary instrumentable
code and can themselves trace, fault, or hit other kprobes. They are
the only parts of the kprobe debug exception flow that legitimately
need instrumentation.

Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
---
 arch/arm64/include/asm/kprobes.h   |  9 +++------
 arch/arm64/kernel/probes/kprobes.c | 22 ++++++++++++++++++----
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h
index 35ce2c94040e..a694f7d34f45 100644
--- a/arch/arm64/include/asm/kprobes.h
+++ b/arch/arm64/include/asm/kprobes.h
@@ -48,11 +48,8 @@ void __kprobes *trampoline_probe_handler(struct pt_regs *regs);
 
 #endif /* CONFIG_KPROBES */
 
-int __kprobes kprobe_brk_handler(struct pt_regs *regs,
-				 unsigned long esr);
-int __kprobes kprobe_ss_brk_handler(struct pt_regs *regs,
-				 unsigned long esr);
-int __kprobes kretprobe_brk_handler(struct pt_regs *regs,
-				 unsigned long esr);
+int noinstr kprobe_brk_handler(struct pt_regs *regs, unsigned long esr);
+int noinstr kprobe_ss_brk_handler(struct pt_regs *regs, unsigned long esr);
+int noinstr kretprobe_brk_handler(struct pt_regs *regs, unsigned long esr);
 
 #endif /* _ARM_KPROBES_H */
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 1b12341b2af3..e9fa66fa4217 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -301,8 +301,11 @@ post_kprobe_handler(struct kprobe *cur, struct kprobe_ctlblk *kcb, struct pt_reg
 	}
 	/* call post handler */
 	kcb->kprobe_status = KPROBE_HIT_SSDONE;
+
+	instrumentation_begin();
 	if (cur->post_handler)
 		cur->post_handler(cur, regs, 0);
+	instrumentation_end();
 
 	reset_current_kprobe();
 }
@@ -359,24 +362,28 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr)
 	return 0;
 }
 
-int __kprobes
+int noinstr
 kprobe_brk_handler(struct pt_regs *regs, unsigned long esr)
 {
 	struct kprobe *p, *cur_kprobe;
 	struct kprobe_ctlblk *kcb;
 	unsigned long addr = instruction_pointer(regs);
+	bool handled;
 
 	kcb = get_kprobe_ctlblk();
 	cur_kprobe = kprobe_running();
 
+	instrumentation_begin();
 	p = get_kprobe((kprobe_opcode_t *) addr);
 	if (WARN_ON_ONCE(!p)) {
+		instrumentation_end();
 		/*
 		 * Something went wrong. This BRK used an immediate reserved
 		 * for kprobes, but we couldn't find any corresponding probe.
 		 */
 		return DBG_HOOK_ERROR;
 	}
+	instrumentation_end();
 
 	if (cur_kprobe) {
 		/* Hit a kprobe inside another kprobe */
@@ -387,6 +394,10 @@ kprobe_brk_handler(struct pt_regs *regs, unsigned long esr)
 		set_current_kprobe(p);
 		kcb->kprobe_status = KPROBE_HIT_ACTIVE;
 
+		instrumentation_begin();
+		handled = p->pre_handler && p->pre_handler(p, regs);
+		instrumentation_end();
+
 		/*
 		 * If we have no pre-handler or it returned 0, we
 		 * continue with normal processing.  If we have a
@@ -394,7 +405,7 @@ kprobe_brk_handler(struct pt_regs *regs, unsigned long esr)
 		 * modify the execution path and not need to single-step
 		 * Let's just reset current kprobe and exit.
 		 */
-		if (!p->pre_handler || !p->pre_handler(p, regs))
+		if (!handled)
 			setup_singlestep(p, regs, kcb, 0);
 		else
 			reset_current_kprobe();
@@ -403,7 +414,7 @@ kprobe_brk_handler(struct pt_regs *regs, unsigned long esr)
 	return DBG_HOOK_HANDLED;
 }
 
-int __kprobes
+int noinstr
 kprobe_ss_brk_handler(struct pt_regs *regs, unsigned long esr)
 {
 	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
@@ -422,13 +433,16 @@ kprobe_ss_brk_handler(struct pt_regs *regs, unsigned long esr)
 	return DBG_HOOK_ERROR;
 }
 
-int __kprobes
+int noinstr
 kretprobe_brk_handler(struct pt_regs *regs, unsigned long esr)
 {
 	if (regs->pc != (unsigned long)__kretprobe_trampoline)
 		return DBG_HOOK_ERROR;
 
+	instrumentation_begin();
 	regs->pc = kretprobe_trampoline_handler(regs, (void *)regs->regs[29]);
+	instrumentation_end();
+
 	return DBG_HOOK_HANDLED;
 }
 
-- 
2.47.3

[RFC PATCH 6/9] arm64/kprobes: Make kprobe_fault_handler() noinstr

[Hongyan Xia]

From: Hongyan Xia <hongyan.xia@transsion.com>

kprobe_fault_handler() is reached from do_page_fault() when a fault
happens while a kprobe single-step is armed. Everything it calls (the
save/restore helpers, kprobes_restore_local_irqflag(),
reset_current_kprobe(), register access) is noinstr after the previous
patch, and being called from an instrumentable caller into a noinstr
callee is fine.

Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
---
 arch/arm64/kernel/probes/kprobes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index e9fa66fa4217..4172998d48d9 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -310,7 +310,7 @@ post_kprobe_handler(struct kprobe *cur, struct kprobe_ctlblk *kcb, struct pt_reg
 	reset_current_kprobe();
 }
 
-int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr)
+int noinstr kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr)
 {
 	struct kprobe *cur = kprobe_running();
 	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
-- 
2.47.3

[RFC PATCH 7/9] arm64/kprobes: Drop the KPROBE_HIT_SS reentry special case

[Hongyan Xia]

From: Hongyan Xia <hongyan.xia@transsion.com>

With the debug exception path noinstr from the vectors down to the
kprobe handlers and the only instrumentable code in the flow confined to
instrumentation_begin()/end() windows, a kprobe hit while another probe
is in KPROBE_HIT_SS can no longer happen.

Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
---
 arch/arm64/include/asm/kprobes.h   |  6 ------
 arch/arm64/kernel/probes/kprobes.c | 21 +--------------------
 2 files changed, 1 insertion(+), 26 deletions(-)

diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h
index a694f7d34f45..bff8ba9c1689 100644
--- a/arch/arm64/include/asm/kprobes.h
+++ b/arch/arm64/include/asm/kprobes.h
@@ -26,12 +26,6 @@
 struct prev_kprobe {
 	struct kprobe *kp;
 	unsigned int status;
-
-	/*
-	 * The original DAIF state of the outer kprobe, saved here before
-	 * a nested kprobe overwrites kcb->saved_irqflag during reentry.
-	 */
-	unsigned long saved_irqflag;
 };
 
 /* per-cpu kprobe control block */
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 4172998d48d9..1658b6acd803 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -176,13 +176,6 @@ static void noinstr save_previous_kprobe(struct kprobe_ctlblk *kcb)
 	kcb->prev_kprobe.kp = kprobe_running();
 	kcb->prev_kprobe.status = kcb->kprobe_status;
 
-	/*
-	 * Save the outer kprobe's original DAIF flags before the nested
-	 * kprobe calls kprobes_save_local_irqflag() and overwrites
-	 * kcb->saved_irqflag. Without this, the outer kprobe will restore
-	 * the wrong DAIF state and leave interrupts permanently masked.
-	 */
-	kcb->prev_kprobe.saved_irqflag = kcb->saved_irqflag;
 }
 
 static void noinstr restore_previous_kprobe(struct kprobe_ctlblk *kcb)
@@ -190,12 +183,6 @@ static void noinstr restore_previous_kprobe(struct kprobe_ctlblk *kcb)
 	__this_cpu_write(current_kprobe, kcb->prev_kprobe.kp);
 	kcb->kprobe_status = kcb->prev_kprobe.status;
 
-	/*
-	 * Restore the outer kprobe's saved_irqflag so that when its
-	 * single-step completes, kprobes_restore_local_irqflag() uses
-	 * the correct original DAIF value.
-	 */
-	kcb->saved_irqflag = kcb->prev_kprobe.saved_irqflag;
 }
 
 static void noinstr set_current_kprobe(struct kprobe *p)
@@ -258,18 +245,12 @@ static int noinstr reenter_kprobe(struct kprobe *p,
 	switch (kcb->kprobe_status) {
 	case KPROBE_HIT_SSDONE:
 	case KPROBE_HIT_ACTIVE:
-	case KPROBE_HIT_SS:
-		/*
-		 * A probe can be hit while another kprobe is preparing or
-		 * executing its XOL single-step instruction. This is still a
-		 * recoverable one-level reentry, so handle it in the same way as
-		 * reentry from KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE.
-		 */
 		instrumentation_begin();
 		kprobes_inc_nmissed_count(p);
 		instrumentation_end();
 		setup_singlestep(p, regs, kcb, 1);
 		break;
+	case KPROBE_HIT_SS:
 	case KPROBE_REENTER:
 		instrumentation_begin();
 		pr_warn("Failed to recover from reentered kprobes.\n");
-- 
2.47.3

[RFC PATCH 8/9] arm64/kprobes: Drop the XOL single-step fault PC check

[Hongyan Xia]

From: Hongyan Xia <hongyan.xia@transsion.com>

With the armed single-step window closed (the whole path noinstr, and
every piece of instrumentable code confined to instrumentation windows
that run in KPROBE_HIT_ACTIVE/HIT_SSDONE state), no code other than the
XOL instruction itself can execute while the state is KPROBE_HIT_SS or
KPROBE_REENTER. A page fault taken in those states therefore necessarily
originates from the XOL instruction, and the faulting-PC comparison is
redundant; revert to handling all such faults as single-step faults (as
before commit 879a6754d3d1).

The simulated-kprobe early bail is kept: simulated probes have no XOL
slot and execute (inside an instrumentation window) in debug trap
context, so a fault there can still come from the simulation handlers
and must not be treated as a single-step fault.

Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
---
 arch/arm64/kernel/probes/kprobes.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 1658b6acd803..73bbbcdbbfe5 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -309,18 +309,6 @@ int noinstr kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr)
 	switch (kcb->kprobe_status) {
 	case KPROBE_HIT_SS:
 	case KPROBE_REENTER:
-		/*
-		 * A page fault taken while in KPROBE_HIT_SS or
-		 * KPROBE_REENTER state is only attributable to kprobe
-		 * single-stepping if the faulting PC points to the
-		 * current kprobe's XOL instruction. If the fault occurred
-		 * elsewhere (e.g. in perf or tracing code invoked from the
-		 * debug exception path), leave it for the normal page fault
-		 * handler to process.
-		 */
-		if (instruction_pointer(regs) != (unsigned long)cur->ainsn.xol_insn)
-			break;
-
 		/*
 		 * We are here because the instruction being single
 		 * stepped caused a page fault. We reset the current
-- 
2.47.3

[RFC PATCH 9/9] arm64/debug: Mark debug exception helpers __always_inline

[Hongyan Xia]

From: Hongyan Xia <hongyan.xia@transsion.com>

Static inline should be enough to actually inline functions for most
compilers, but my Clang-19 somehow thinks it's better to outline them.
These tiny helpers then live in normal .text sections instead of
.noinstr sections, violating noinstr.

Mark them __always_inline so the compiler can never outline them.

Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
---
 arch/arm64/include/asm/esr.h       | 2 +-
 arch/arm64/include/asm/percpu.h    | 2 +-
 arch/arm64/include/asm/preempt.h   | 4 ++--
 arch/arm64/include/asm/ptrace.h    | 4 ++--
 arch/arm64/kernel/debug-monitors.c | 2 +-
 include/linux/kprobes.h            | 8 ++++----
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index f816f5d77f1a..a75bfdb7e5fe 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -437,7 +437,7 @@
 #ifndef __ASSEMBLER__
 #include <asm/types.h>
 
-static inline unsigned long esr_brk_comment(unsigned long esr)
+static __always_inline unsigned long esr_brk_comment(unsigned long esr)
 {
 	return esr & ESR_ELx_BRK64_ISS_COMMENT_MASK;
 }
diff --git a/arch/arm64/include/asm/percpu.h b/arch/arm64/include/asm/percpu.h
index b57b2bb00967..d4cae47fde8c 100644
--- a/arch/arm64/include/asm/percpu.h
+++ b/arch/arm64/include/asm/percpu.h
@@ -29,7 +29,7 @@ static inline unsigned long __hyp_my_cpu_offset(void)
 	return read_sysreg(tpidr_el2);
 }
 
-static inline unsigned long __kern_my_cpu_offset(void)
+static __always_inline unsigned long __kern_my_cpu_offset(void)
 {
 	unsigned long off;
 
diff --git a/arch/arm64/include/asm/preempt.h b/arch/arm64/include/asm/preempt.h
index 932ea4b62042..326f221c3f56 100644
--- a/arch/arm64/include/asm/preempt.h
+++ b/arch/arm64/include/asm/preempt.h
@@ -41,14 +41,14 @@ static inline bool test_preempt_need_resched(void)
 	return !current_thread_info()->preempt.need_resched;
 }
 
-static inline void __preempt_count_add(int val)
+static __always_inline void __preempt_count_add(int val)
 {
 	u32 pc = READ_ONCE(current_thread_info()->preempt.count);
 	pc += val;
 	WRITE_ONCE(current_thread_info()->preempt.count, pc);
 }
 
-static inline void __preempt_count_sub(int val)
+static __always_inline void __preempt_count_sub(int val)
 {
 	u32 pc = READ_ONCE(current_thread_info()->preempt.count);
 	pc -= val;
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index 39582511ad72..460726224299 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -336,11 +336,11 @@ static inline unsigned long regs_get_kernel_argument(struct pt_regs *regs,
 struct task_struct;
 int valid_user_regs(struct user_pt_regs *regs, struct task_struct *task);
 
-static inline unsigned long instruction_pointer(struct pt_regs *regs)
+static __always_inline unsigned long instruction_pointer(struct pt_regs *regs)
 {
 	return regs->pc;
 }
-static inline void instruction_pointer_set(struct pt_regs *regs,
+static __always_inline void instruction_pointer_set(struct pt_regs *regs,
 		unsigned long val)
 {
 	regs->pc = val;
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index a970ab6327cd..66cb8151f5df 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -146,7 +146,7 @@ postcore_initcall(debug_monitors_init);
 /*
  * Single step API and exception handling.
  */
-static void set_user_regs_spsr_ss(struct user_pt_regs *regs)
+static __always_inline void set_user_regs_spsr_ss(struct user_pt_regs *regs)
 {
 	regs->pstate |= DBG_SPSR_SS;
 }
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 8c4f3bb24429..5880445ed0f0 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -384,17 +384,17 @@ static inline void kprobe_ftrace_kill(void) {}
 struct kprobe *get_kprobe(void *addr);
 
 /* kprobe_running() will just return the current_kprobe on this CPU */
-static inline struct kprobe *kprobe_running(void)
+static __always_inline struct kprobe *kprobe_running(void)
 {
 	return __this_cpu_read(current_kprobe);
 }
 
-static inline void reset_current_kprobe(void)
+static __always_inline void reset_current_kprobe(void)
 {
 	__this_cpu_write(current_kprobe, NULL);
 }
 
-static inline struct kprobe_ctlblk *get_kprobe_ctlblk(void)
+static __always_inline struct kprobe_ctlblk *get_kprobe_ctlblk(void)
 {
 	return this_cpu_ptr(&kprobe_ctlblk);
 }
@@ -449,7 +449,7 @@ static inline struct kprobe *get_kprobe(void *addr)
 {
 	return NULL;
 }
-static inline struct kprobe *kprobe_running(void)
+static __always_inline struct kprobe *kprobe_running(void)
 {
 	return NULL;
 }
-- 
2.47.3