[PATCH v2 02/11] arm64: debug: call software break handlers statically
Will Deacon
will at kernel.org
Tue May 20 08:35:46 PDT 2025
On Mon, May 12, 2025 at 06:43:17PM +0100, Ada Couprie Diaz wrote:
> Software breakpoints pass an immediate value in ESR ("comment") that can
> be used to call a specialized handler (KGDB, KASAN...).
> We do so in two different ways :
> - During early boot, `early_brk64` statically checks against known
> immediates and calls the corresponding handler,
> - During init, handlers are dynamically registered into a list. When
> called, the generic software breakpoint handler will iterate over
> the list to find the appropriate handler.
>
> The dynamic registration does not provide any benefit here as it is not
> exported and all its uses are within the arm64 tree. It also depends on an
> RCU list, whose safe access currently relies on the non-preemptible state
> of `do_debug_exception`.
>
> Replace the list iteration logic in `call_break_hooks` to call
> the breakpoint handlers statically if they are enabled, like in
> `early_brk64`.
> Expose the handlers in their respective headers to be reachable from
> `arch/arm64/kernel/debug-monitors.c` at link time.
>
> Unify the naming of the software breakpoint handlers to XXX_brk_handler(),
> making it clear they are related and to differentiate from the
> hardware breakpoints.
>
> Signed-off-by: Ada Couprie Diaz <ada.coupriediaz at arm.com>
> ---
> arch/arm64/include/asm/kgdb.h | 3 +
> arch/arm64/include/asm/kprobes.h | 6 ++
> arch/arm64/include/asm/traps.h | 6 ++
> arch/arm64/include/asm/uprobes.h | 2 +
> arch/arm64/kernel/debug-monitors.c | 57 ++++++++++++++----
> arch/arm64/kernel/kgdb.c | 22 ++-----
> arch/arm64/kernel/probes/kprobes.c | 31 ++--------
> arch/arm64/kernel/probes/kprobes_trampoline.S | 2 +-
> arch/arm64/kernel/probes/uprobes.c | 9 +--
> arch/arm64/kernel/traps.c | 59 ++++---------------
> 10 files changed, 84 insertions(+), 113 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kgdb.h b/arch/arm64/include/asm/kgdb.h
> index 21fc85e9d2be..82a76b2102fb 100644
> --- a/arch/arm64/include/asm/kgdb.h
> +++ b/arch/arm64/include/asm/kgdb.h
> @@ -24,6 +24,9 @@ static inline void arch_kgdb_breakpoint(void)
> extern void kgdb_handle_bus_error(void);
> extern int kgdb_fault_expected;
>
> +int kgdb_brk_handler(struct pt_regs *regs, unsigned long esr);
> +int kgdb_compiled_brk_handler(struct pt_regs *regs, unsigned long esr);
> +
> #endif /* !__ASSEMBLY__ */
>
> /*
> diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h
> index be7a3680dadf..b27dd6028e6a 100644
> --- a/arch/arm64/include/asm/kprobes.h
> +++ b/arch/arm64/include/asm/kprobes.h
> @@ -38,6 +38,12 @@ struct kprobe_ctlblk {
> void arch_remove_kprobe(struct kprobe *);
> int kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr);
> void __kretprobe_trampoline(void);
> +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);
> void __kprobes *trampoline_probe_handler(struct pt_regs *regs);
>
> #endif /* CONFIG_KPROBES */
If you add these _after_ the #endif...
> diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
> index 676fa0231935..4ece4a93b872 100644
> --- a/arch/arm64/kernel/debug-monitors.c
> +++ b/arch/arm64/kernel/debug-monitors.c
> @@ -21,8 +21,11 @@
> #include <asm/cputype.h>
> #include <asm/daifflags.h>
> #include <asm/debug-monitors.h>
> +#include <asm/kgdb.h>
> +#include <asm/kprobes.h>
> #include <asm/system_misc.h>
> #include <asm/traps.h>
> +#include <asm/uprobes.h>
>
> /* Determine debug architecture. */
> u8 debug_monitors_arch(void)
> @@ -299,20 +302,50 @@ void unregister_kernel_break_hook(struct break_hook *hook)
>
> static int call_break_hook(struct pt_regs *regs, unsigned long esr)
> {
> - struct break_hook *hook;
> - struct list_head *list;
> -
> - list = user_mode(regs) ? &user_break_hook : &kernel_break_hook;
> -
> - /*
> - * Since brk exception disables interrupt, this function is
> - * entirely not preemptible, and we can use rcu list safely here.
> - */
> - list_for_each_entry_rcu(hook, list, node) {
> - if ((esr_brk_comment(esr) & ~hook->mask) == hook->imm)
> - return hook->fn(regs, esr);
> + if (user_mode(regs)) {
> +#ifdef CONFIG_UPROBES
> + if (esr_brk_comment(esr) == UPROBES_BRK_IMM)
> + return uprobe_brk_handler(regs, esr);
> +#endif
> + return DBG_HOOK_ERROR;
> }
>
> + if (esr_brk_comment(esr) == BUG_BRK_IMM)
> + return bug_brk_handler(regs, esr);
> +
> +#ifdef CONFIG_CFI_CLANG
> + if (esr_is_cfi_brk(esr))
> + return cfi_brk_handler(regs, esr);
> +#endif
> +
> + if (esr_brk_comment(esr) == FAULT_BRK_IMM)
> + return reserved_fault_brk_handler(regs, esr);
> +
> +#ifdef CONFIG_KASAN_SW_TAGS
> + if ((esr_brk_comment(esr) & ~KASAN_BRK_MASK) == KASAN_BRK_IMM)
> + return kasan_brk_handler(regs, esr);
> +#endif
> +#ifdef CONFIG_UBSAN_TRAP
> + if ((esr_brk_comment(esr) & ~UBSAN_BRK_MASK) == UBSAN_BRK_IMM)
> + return ubsan_brk_handler(regs, esr);
> +#endif
> +#ifdef 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);
> +#endif
> +#ifdef CONFIG_KPROBES
> + if (esr_brk_comment(esr) == KPROBES_BRK_IMM)
> + return kprobe_brk_handler(regs, esr);
> + if (esr_brk_comment(esr) == KPROBES_BRK_SS_IMM)
> + return kprobe_ss_brk_handler(regs, esr);
> +#endif
> +#ifdef CONFIG_KRETPROBES
> + if (esr_brk_comment(esr) == KRETPROBES_BRK_IMM)
> + return kretprobe_brk_handler(regs, esr);
> +#endif
... then I think you can use IS_ENABLED() here instead of the #ifdefs.
I didn't check everything, but the diff I was hacking on is below.
Will
--->8
diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h
index b27dd6028e6a..0d41fbbffb06 100644
--- a/arch/arm64/include/asm/kprobes.h
+++ b/arch/arm64/include/asm/kprobes.h
@@ -38,13 +38,14 @@ struct kprobe_ctlblk {
void arch_remove_kprobe(struct kprobe *);
int kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr);
void __kretprobe_trampoline(void);
+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);
-void __kprobes *trampoline_probe_handler(struct pt_regs *regs);
-#endif /* CONFIG_KPROBES */
#endif /* _ARM_KPROBES_H */
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index a5f344e556df..5b199ac70476 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -227,48 +227,50 @@ NOKPROBE_SYMBOL(do_softstep);
static int call_break_hook(struct pt_regs *regs, unsigned long esr)
{
if (user_mode(regs)) {
-#ifdef CONFIG_UPROBES
- if (esr_brk_comment(esr) == UPROBES_BRK_IMM)
+ if (IS_ENABLED(CONFIG_UPROBES) &&
+ esr_brk_comment(esr) == UPROBES_BRK_IMM) {
return uprobe_brk_handler(regs, esr);
-#endif
+ }
+
return DBG_HOOK_ERROR;
}
if (esr_brk_comment(esr) == BUG_BRK_IMM)
return bug_brk_handler(regs, esr);
-#ifdef CONFIG_CFI_CLANG
- if (esr_is_cfi_brk(esr))
+ if (IS_ENABLED(CONFIG_CFI_CLANG) && esr_is_cfi_brk(esr))
return cfi_brk_handler(regs, esr);
-#endif
if (esr_brk_comment(esr) == FAULT_BRK_IMM)
return reserved_fault_brk_handler(regs, esr);
-#ifdef CONFIG_KASAN_SW_TAGS
- if ((esr_brk_comment(esr) & ~KASAN_BRK_MASK) == KASAN_BRK_IMM)
+ if (IS_ENABLED(CONFIG_KASAN_SW_TAGS) &&
+ (esr_brk_comment(esr) & ~KASAN_BRK_MASK) == KASAN_BRK_IMM) {
return kasan_brk_handler(regs, esr);
-#endif
-#ifdef CONFIG_UBSAN_TRAP
- if ((esr_brk_comment(esr) & ~UBSAN_BRK_MASK) == UBSAN_BRK_IMM)
+ }
+
+ if (IS_ENABLED(CONFIG_UBSAN_TRAP) &&
+ (esr_brk_comment(esr) & ~UBSAN_BRK_MASK) == UBSAN_BRK_IMM) {
return ubsan_brk_handler(regs, esr);
-#endif
-#ifdef 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);
-#endif
-#ifdef CONFIG_KPROBES
- if (esr_brk_comment(esr) == KPROBES_BRK_IMM)
- return kprobe_brk_handler(regs, esr);
- if (esr_brk_comment(esr) == KPROBES_BRK_SS_IMM)
- return kprobe_ss_brk_handler(regs, esr);
-#endif
-#ifdef CONFIG_KRETPROBES
- if (esr_brk_comment(esr) == KRETPROBES_BRK_IMM)
+ }
+
+ 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);
+ }
+
+ if (IS_ENABLED(CONFIG_KPROBES)) {
+ if (esr_brk_comment(esr) == KPROBES_BRK_IMM)
+ return kprobe_brk_handler(regs, esr);
+ if (esr_brk_comment(esr) == KPROBES_BRK_SS_IMM)
+ return kprobe_ss_brk_handler(regs, esr);
+ }
+
+ if (IS_ENABLED(CONFIG_KRETPROBES) &&
+ esr_brk_comment(esr) == KRETPROBES_BRK_IMM)
return kretprobe_brk_handler(regs, esr);
-#endif
return DBG_HOOK_ERROR;
}
More information about the linux-arm-kernel
mailing list