[RFC PATCH v2 2/4] arm64/debug: Make the Kprobe functions noinstr

[RFC PATCH v2 2/4] arm64/debug: Make the Kprobe functions noinstr

[Hongyan Xia]

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

The Kprobe debug exception path must be run with extra care. NOKPROBE
isn't sufficient, as other instrumentation like ftrace still opens a can
of worms that is very complex to deal with.

Mark the three main Kprobe entry points noinstr, as well as the debug
exception paths that lead to and exit from these entry points. Note that
noinstr attribute is stronger and can safely replace NOKPROBE and
__kprobe modifiers.

Also mark esr_brk_comment() __always_inline, as clang does not
reliably inline plain static inline functions into noinline (noinstr)
callers such as call_el1_break_hook().

Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
---
 arch/arm64/include/asm/esr.h       | 2 +-
 arch/arm64/include/asm/kprobes.h   | 9 +++------
 arch/arm64/kernel/debug-monitors.c | 6 ++----
 arch/arm64/kernel/probes/kprobes.c | 6 +++---
 4 files changed, 9 insertions(+), 14 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/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/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 29307642f4c9..5cf4fb8ddf83 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -207,7 +207,7 @@ void do_el1_softstep(unsigned long esr, struct pt_regs *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);
@@ -245,7 +245,6 @@ static int call_el1_break_hook(struct pt_regs *regs, unsigned long esr)
 
 	return DBG_HOOK_ERROR;
 }
-NOKPROBE_SYMBOL(call_el1_break_hook);
 
 /*
  * We have already unmasked interrupts and enabled preemption
@@ -261,14 +260,13 @@ 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;
 
 	die("Oops - BRK", regs, esr);
 }
-NOKPROBE_SYMBOL(do_el1_brk64);
 
 #ifdef CONFIG_COMPAT
 void do_bkpt32(unsigned long esr, struct pt_regs *regs)
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 4e0efad5caf2..0e66abf9958e 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -350,7 +350,7 @@ 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;
@@ -394,7 +394,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();
@@ -413,7 +413,7 @@ 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)
-- 
2.47.3

[RFC PATCH v2 3/4] arm64/kprobes: Make the entire Kprobe noinstr

[Hongyan Xia]

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

Convert all the sub-functions called by kprobe_brk_handler,
kprobe_ss_brk_handler and kretprobe_brk_handler noinstr. After this
commit, the entire Kprobe path (anything between Kprobe debug_exception
entry and exit) is now noinstr.

One big item that is missing is the instruction simulation path. We
leave it as future work as the GCS handling needs extra care.

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

Also mark the leaf helpers used inside these functions
__always_inline, as clang does not reliably inline plain static inline
functions into noinline (noinstr) callers.

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

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/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/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 0e66abf9958e..5ec5f4ef9985 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -39,7 +39,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 +170,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 +184,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 +197,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 +207,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;
 
@@ -248,9 +248,9 @@ static void __kprobes setup_singlestep(struct kprobe *p,
 	}
 }
 
-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:
@@ -278,7 +278,7 @@ static int __kprobes reenter_kprobe(struct kprobe *p,
 	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 */
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

Re: [RFC PATCH v2 2/4] arm64/debug: Make the Kprobe functions noinstr

[Masami Hiramatsu]

On Thu, 13 Aug 2026 06:49:42 +0000
Hongyan Xia <hongyan.xia@transsion.com> wrote:

> From: Hongyan Xia <hongyan.xia@transsion.com>
> 
> The Kprobe debug exception path must be run with extra care. NOKPROBE
> isn't sufficient, as other instrumentation like ftrace still opens a can
> of worms that is very complex to deal with.
> 
> Mark the three main Kprobe entry points noinstr, as well as the debug
> exception paths that lead to and exit from these entry points. Note that
> noinstr attribute is stronger and can safely replace NOKPROBE and
> __kprobe modifiers.
> 
> Also mark esr_brk_comment() __always_inline, as clang does not
> reliably inline plain static inline functions into noinline (noinstr)
> callers such as call_el1_break_hook().
> 

Looks good to me.

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you,

> Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
> ---
>  arch/arm64/include/asm/esr.h       | 2 +-
>  arch/arm64/include/asm/kprobes.h   | 9 +++------
>  arch/arm64/kernel/debug-monitors.c | 6 ++----
>  arch/arm64/kernel/probes/kprobes.c | 6 +++---
>  4 files changed, 9 insertions(+), 14 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/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/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
> index 29307642f4c9..5cf4fb8ddf83 100644
> --- a/arch/arm64/kernel/debug-monitors.c
> +++ b/arch/arm64/kernel/debug-monitors.c
> @@ -207,7 +207,7 @@ void do_el1_softstep(unsigned long esr, struct pt_regs *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);
> @@ -245,7 +245,6 @@ static int call_el1_break_hook(struct pt_regs *regs, unsigned long esr)
>  
>  	return DBG_HOOK_ERROR;
>  }
> -NOKPROBE_SYMBOL(call_el1_break_hook);
>  
>  /*
>   * We have already unmasked interrupts and enabled preemption
> @@ -261,14 +260,13 @@ 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;
>  
>  	die("Oops - BRK", regs, esr);
>  }
> -NOKPROBE_SYMBOL(do_el1_brk64);
>  
>  #ifdef CONFIG_COMPAT
>  void do_bkpt32(unsigned long esr, struct pt_regs *regs)
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index 4e0efad5caf2..0e66abf9958e 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -350,7 +350,7 @@ 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;
> @@ -394,7 +394,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();
> @@ -413,7 +413,7 @@ 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)
> -- 
> 2.47.3
> 


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