* [PATCH 00/11] arm64: debug: remove hook registration, split exception entry
@ 2025-04-25 15:36 Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 01/11] arm64: debug: Clean up single_step_handler logic Ada Couprie Diaz
` (10 more replies)
0 siblings, 11 replies; 15+ messages in thread
From: Ada Couprie Diaz @ 2025-04-25 15:36 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Will Deacon, Catalin Marinas
Hi,
This series simplifies the debug exception entry path by removing handler
registration mechanisms for the debug exception handlers, a holdover from
the arm kernel, as well as the break and stepping handlers.
This moves much of the code related to debug exceptions outside of
`mm/fault.c` where it didn't make much sense.
This allows us to split the debug exception entries: going from one common
path per EL for all debug exceptions to a unique one per exception and EL.
The result is a much simpler and fully static exception entry path, which
we tailor to the different exceptions and their constraints.
The series is structured as such :
1 : related clean-up in the signle step handler
2-4 : software breakpoints and single step handlers registration removal
5: preparatory function duplication that gets cleaned-up in patch 11
6-11 : debug exception splitting and handler registration removal
* Patch 2 copies and extends the `early_brk64` break handling for the
normal path, byassing the dynamic registration.
* Patch 3 does something similar for the single stepping handlers.
* Patches 6 through 10 split each individual debug exception from
the unified path by creating specific handlers, adapting them to
their constraints and calling into them, bypassing the dynamically
registered handlers used before.
* Patches 4 and 11 are clean-ups removing the code that has been replaced
and made redundant by the preceding patches.
Testing
===
Testing EL1 debug exceptions can only be properly done with perf.
A simple test of hardware breakpoints, watchpoints, and software stepping
can be achieved by using `perf stat sleep 0.01` and adding hardware events
for `jiffies` and `hrtimer_nanosleep`, which guarantees a
hardware watchpoint and breakpoint.
Inserting a `WARN()` at a convenient place will allow testing both early
and late software breakpoints at EL1, or using KGDB to test without
changing code.
For EL0 debug exceptions, the easiest is to setup a basic program and use
GDB with a list of pre-programmed commands setting hardware and software
breakpoints as well as watchpoints. Setpping will occur naturally when
encountering breakpoints.
All tests maintained behaviour after the patches.
I also tested with KASAN on, with no apparent impact.
See below for some testing examples.
Based on v6.15-rc3.
Thanks,
Ada
Testing examples
===
Perf (for EL1):
~~~
Assuming that `perf` is on your $PATH and building with `kallsyms`
#!/bin/bash
watch_addr=$(sudo cat /proc/kallsyms | grep "D jiffies$" | cut -f1 -d\ )
break_addr=$(sudo cat /proc/kallsyms | grep "clock_nanosleep$" | cut -f1 -d\ )
cmd="sleep 0.01"
sudo perf stat -a -e mem:0x${watch_addr}/8:w -e mem:0x${break_addr}:x ${cmd}
NB: This does /not/ test EL1 BRKs.
GDB commands (for EL0):
~~~
The following C example, compiled with `-g -O0`
int main() {
int add = 0xAA;
int target = 0;
target += add;
#ifdef COMPAT
__asm__("BKPT");
#else
__asm__("BRK 1");
#endif
return target;
}
Combined with the following GDB command-list
start
hbreak 3
watch target
commands 2
continue
end
commands 3
continue
end
continue
jump 11
continue
quit
Executed as such : `gdb -x ${COMMAND_LIST_FILE} ./a.out`
should go through the whole program, return 0252/170/0xAA, and
exercise all EL0 debug exception entries.
By using a cross-compiler and passing and additional `-DCOMPAT` argument
during compilation, the `BKPT32` path can also be tested.
NOTE: `BKPT` *will* make GDB loop infinitely, that is expected. Sending
SIGINT to GDB will break the loop and the execution should complete.
Ada Couprie Diaz (11):
arm64: debug: Clean up single_step_handler logic
arm64: debug: call software break handlers statically
arm64: debug: call step handlers statically
arm64: debug: remove break/step handler registration infrastructure
arm64: entry: Add entry and exit functions for debug exceptions
arm64: debug: split hardware breakpoint exeception entry
arm64: debug: split single stepping exception entry
arm64: debug: split hardware watchpoint exception entry
arm64: debug: split brk64 exception entry
arm64: debug: split bkpt32 exception entry
arm64: debug: remove debug exception registration infrastructure
arch/arm64/include/asm/debug-monitors.h | 26 ---
arch/arm64/include/asm/exception.h | 6 +-
arch/arm64/include/asm/kgdb.h | 4 +
arch/arm64/include/asm/kprobes.h | 6 +
arch/arm64/include/asm/system_misc.h | 4 -
arch/arm64/include/asm/traps.h | 6 +
arch/arm64/include/asm/uprobes.h | 3 +
arch/arm64/kernel/debug-monitors.c | 198 ++++++++----------
arch/arm64/kernel/entry-common.c | 129 +++++++++++-
arch/arm64/kernel/hw_breakpoint.c | 34 ++-
arch/arm64/kernel/kgdb.c | 39 +---
arch/arm64/kernel/probes/kprobes.c | 31 +--
arch/arm64/kernel/probes/kprobes_trampoline.S | 2 +-
arch/arm64/kernel/probes/uprobes.c | 18 +-
arch/arm64/kernel/traps.c | 77 +------
arch/arm64/mm/fault.c | 75 -------
16 files changed, 285 insertions(+), 373 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 01/11] arm64: debug: Clean up single_step_handler logic
2025-04-25 15:36 [PATCH 00/11] arm64: debug: remove hook registration, split exception entry Ada Couprie Diaz
@ 2025-04-25 15:36 ` Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 02/11] arm64: debug: call software break handlers statically Ada Couprie Diaz
` (9 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Ada Couprie Diaz @ 2025-04-25 15:36 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Will Deacon, Catalin Marinas
Remove the unnecessary boolean which always checks if the handler was found
and return early instead.
Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
---
arch/arm64/kernel/debug-monitors.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 58f047de3e1c..676fa0231935 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -241,8 +241,6 @@ static void send_user_sigtrap(int si_code)
static int single_step_handler(unsigned long unused, unsigned long esr,
struct pt_regs *regs)
{
- bool handler_found = false;
-
/*
* If we are stepping a pending breakpoint, call the hw_breakpoint
* handler first.
@@ -250,10 +248,10 @@ static int single_step_handler(unsigned long unused, unsigned long esr,
if (!reinstall_suspended_bps(regs))
return 0;
- if (!handler_found && call_step_hook(regs, esr) == DBG_HOOK_HANDLED)
- handler_found = true;
+ if (call_step_hook(regs, esr) == DBG_HOOK_HANDLED)
+ return 0;
- if (!handler_found && user_mode(regs)) {
+ if (user_mode(regs)) {
send_user_sigtrap(TRAP_TRACE);
/*
@@ -263,7 +261,7 @@ static int single_step_handler(unsigned long unused, unsigned long esr,
* to the active-not-pending state).
*/
user_rewind_single_step(current);
- } else if (!handler_found) {
+ } else {
pr_warn("Unexpected kernel single-step exception at EL1\n");
/*
* Re-enable stepping since we know that we will be
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 02/11] arm64: debug: call software break handlers statically
2025-04-25 15:36 [PATCH 00/11] arm64: debug: remove hook registration, split exception entry Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 01/11] arm64: debug: Clean up single_step_handler logic Ada Couprie Diaz
@ 2025-04-25 15:36 ` Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 03/11] arm64: debug: call step " Ada Couprie Diaz
` (8 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Ada Couprie Diaz @ 2025-04-25 15:36 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Will Deacon, Catalin Marinas
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@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 */
diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
index 82cf1f879c61..e3e8944a71c3 100644
--- a/arch/arm64/include/asm/traps.h
+++ b/arch/arm64/include/asm/traps.h
@@ -29,6 +29,12 @@ void arm64_force_sig_fault_pkey(unsigned long far, const char *str, int pkey);
void arm64_force_sig_mceerr(int code, unsigned long far, short lsb, const char *str);
void arm64_force_sig_ptrace_errno_trap(int errno, unsigned long far, const char *str);
+int bug_brk_handler(struct pt_regs *regs, unsigned long esr);
+int cfi_brk_handler(struct pt_regs *regs, unsigned long esr);
+int reserved_fault_brk_handler(struct pt_regs *regs, unsigned long esr);
+int kasan_brk_handler(struct pt_regs *regs, unsigned long esr);
+int ubsan_brk_handler(struct pt_regs *regs, unsigned long esr);
+
int early_brk64(unsigned long addr, unsigned long esr, struct pt_regs *regs);
/*
diff --git a/arch/arm64/include/asm/uprobes.h b/arch/arm64/include/asm/uprobes.h
index 014b02897f8e..3659a79a9f32 100644
--- a/arch/arm64/include/asm/uprobes.h
+++ b/arch/arm64/include/asm/uprobes.h
@@ -28,4 +28,6 @@ struct arch_uprobe {
bool simulate;
};
+int uprobe_brk_handler(struct pt_regs *regs, unsigned long esr);
+
#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
+
return DBG_HOOK_ERROR;
}
NOKPROBE_SYMBOL(call_break_hook);
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index f3c4d3a8a20f..b5a3b9c85a65 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -234,21 +234,21 @@ int kgdb_arch_handle_exception(int exception_vector, int signo,
return err;
}
-static int kgdb_brk_fn(struct pt_regs *regs, unsigned long esr)
+int kgdb_brk_handler(struct pt_regs *regs, unsigned long esr)
{
kgdb_handle_exception(1, SIGTRAP, 0, regs);
return DBG_HOOK_HANDLED;
}
-NOKPROBE_SYMBOL(kgdb_brk_fn)
+NOKPROBE_SYMBOL(kgdb_brk_handler)
-static int kgdb_compiled_brk_fn(struct pt_regs *regs, unsigned long esr)
+int kgdb_compiled_brk_handler(struct pt_regs *regs, unsigned long esr)
{
compiled_break = 1;
kgdb_handle_exception(1, SIGTRAP, 0, regs);
return DBG_HOOK_HANDLED;
}
-NOKPROBE_SYMBOL(kgdb_compiled_brk_fn);
+NOKPROBE_SYMBOL(kgdb_compiled_brk_handler);
static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned long esr)
{
@@ -260,16 +260,6 @@ static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned long esr)
}
NOKPROBE_SYMBOL(kgdb_step_brk_fn);
-static struct break_hook kgdb_brkpt_hook = {
- .fn = kgdb_brk_fn,
- .imm = KGDB_DYN_DBG_BRK_IMM,
-};
-
-static struct break_hook kgdb_compiled_brkpt_hook = {
- .fn = kgdb_compiled_brk_fn,
- .imm = KGDB_COMPILED_DBG_BRK_IMM,
-};
-
static struct step_hook kgdb_step_hook = {
.fn = kgdb_step_brk_fn
};
@@ -316,8 +306,6 @@ int kgdb_arch_init(void)
if (ret != 0)
return ret;
- register_kernel_break_hook(&kgdb_brkpt_hook);
- register_kernel_break_hook(&kgdb_compiled_brkpt_hook);
register_kernel_step_hook(&kgdb_step_hook);
return 0;
}
@@ -329,8 +317,6 @@ int kgdb_arch_init(void)
*/
void kgdb_arch_exit(void)
{
- unregister_kernel_break_hook(&kgdb_brkpt_hook);
- unregister_kernel_break_hook(&kgdb_compiled_brkpt_hook);
unregister_kernel_step_hook(&kgdb_step_hook);
unregister_die_notifier(&kgdb_notifier);
}
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index d9e462eafb95..0c5d408afd95 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -292,8 +292,8 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr)
return 0;
}
-static int __kprobes
-kprobe_breakpoint_handler(struct pt_regs *regs, unsigned long esr)
+int __kprobes
+kprobe_brk_handler(struct pt_regs *regs, unsigned long esr)
{
struct kprobe *p, *cur_kprobe;
struct kprobe_ctlblk *kcb;
@@ -336,13 +336,8 @@ kprobe_breakpoint_handler(struct pt_regs *regs, unsigned long esr)
return DBG_HOOK_HANDLED;
}
-static struct break_hook kprobes_break_hook = {
- .imm = KPROBES_BRK_IMM,
- .fn = kprobe_breakpoint_handler,
-};
-
-static int __kprobes
-kprobe_breakpoint_ss_handler(struct pt_regs *regs, unsigned long esr)
+int __kprobes
+kprobe_ss_brk_handler(struct pt_regs *regs, unsigned long esr)
{
struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
unsigned long addr = instruction_pointer(regs);
@@ -360,13 +355,8 @@ kprobe_breakpoint_ss_handler(struct pt_regs *regs, unsigned long esr)
return DBG_HOOK_ERROR;
}
-static struct break_hook kprobes_break_ss_hook = {
- .imm = KPROBES_BRK_SS_IMM,
- .fn = kprobe_breakpoint_ss_handler,
-};
-
-static int __kprobes
-kretprobe_breakpoint_handler(struct pt_regs *regs, unsigned long esr)
+int __kprobes
+kretprobe_brk_handler(struct pt_regs *regs, unsigned long esr)
{
if (regs->pc != (unsigned long)__kretprobe_trampoline)
return DBG_HOOK_ERROR;
@@ -375,11 +365,6 @@ kretprobe_breakpoint_handler(struct pt_regs *regs, unsigned long esr)
return DBG_HOOK_HANDLED;
}
-static struct break_hook kretprobes_break_hook = {
- .imm = KRETPROBES_BRK_IMM,
- .fn = kretprobe_breakpoint_handler,
-};
-
/*
* Provide a blacklist of symbols identifying ranges which cannot be kprobed.
* This blacklist is exposed to userspace via debugfs (kprobes/blacklist).
@@ -422,9 +407,5 @@ int __kprobes arch_trampoline_kprobe(struct kprobe *p)
int __init arch_init_kprobes(void)
{
- register_kernel_break_hook(&kprobes_break_hook);
- register_kernel_break_hook(&kprobes_break_ss_hook);
- register_kernel_break_hook(&kretprobes_break_hook);
-
return 0;
}
diff --git a/arch/arm64/kernel/probes/kprobes_trampoline.S b/arch/arm64/kernel/probes/kprobes_trampoline.S
index a362f3dbb3d1..b60739d3983f 100644
--- a/arch/arm64/kernel/probes/kprobes_trampoline.S
+++ b/arch/arm64/kernel/probes/kprobes_trampoline.S
@@ -12,7 +12,7 @@
SYM_CODE_START(__kretprobe_trampoline)
/*
* Trigger a breakpoint exception. The PC will be adjusted by
- * kretprobe_breakpoint_handler(), and no subsequent instructions will
+ * kretprobe_brk_handler(), and no subsequent instructions will
* be executed from the trampoline.
*/
brk #KRETPROBES_BRK_IMM
diff --git a/arch/arm64/kernel/probes/uprobes.c b/arch/arm64/kernel/probes/uprobes.c
index cb3d05af36e3..ad68b4a5974d 100644
--- a/arch/arm64/kernel/probes/uprobes.c
+++ b/arch/arm64/kernel/probes/uprobes.c
@@ -173,7 +173,7 @@ int arch_uprobe_exception_notify(struct notifier_block *self,
return NOTIFY_DONE;
}
-static int uprobe_breakpoint_handler(struct pt_regs *regs,
+int uprobe_brk_handler(struct pt_regs *regs,
unsigned long esr)
{
if (uprobe_pre_sstep_notifier(regs))
@@ -194,12 +194,6 @@ static int uprobe_single_step_handler(struct pt_regs *regs,
return DBG_HOOK_ERROR;
}
-/* uprobe breakpoint handler hook */
-static struct break_hook uprobes_break_hook = {
- .imm = UPROBES_BRK_IMM,
- .fn = uprobe_breakpoint_handler,
-};
-
/* uprobe single step handler hook */
static struct step_hook uprobes_step_hook = {
.fn = uprobe_single_step_handler,
@@ -207,7 +201,6 @@ static struct step_hook uprobes_step_hook = {
static int __init arch_init_uprobes(void)
{
- register_user_break_hook(&uprobes_break_hook);
register_user_step_hook(&uprobes_step_hook);
return 0;
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 529cff825531..50d7a6a75f45 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -987,7 +987,7 @@ void do_serror(struct pt_regs *regs, unsigned long esr)
int is_valid_bugaddr(unsigned long addr)
{
/*
- * bug_handler() only called for BRK #BUG_BRK_IMM.
+ * bug_brk_handler() only called for BRK #BUG_BRK_IMM.
* So the answer is trivial -- any spurious instances with no
* bug table entry will be rejected by report_bug() and passed
* back to the debug-monitors code and handled as a fatal
@@ -997,7 +997,7 @@ int is_valid_bugaddr(unsigned long addr)
}
#endif
-static int bug_handler(struct pt_regs *regs, unsigned long esr)
+int bug_brk_handler(struct pt_regs *regs, unsigned long esr)
{
switch (report_bug(regs->pc, regs)) {
case BUG_TRAP_TYPE_BUG:
@@ -1017,13 +1017,8 @@ static int bug_handler(struct pt_regs *regs, unsigned long esr)
return DBG_HOOK_HANDLED;
}
-static struct break_hook bug_break_hook = {
- .fn = bug_handler,
- .imm = BUG_BRK_IMM,
-};
-
#ifdef CONFIG_CFI_CLANG
-static int cfi_handler(struct pt_regs *regs, unsigned long esr)
+int cfi_brk_handler(struct pt_regs *regs, unsigned long esr)
{
unsigned long target;
u32 type;
@@ -1046,15 +1041,9 @@ static int cfi_handler(struct pt_regs *regs, unsigned long esr)
arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE);
return DBG_HOOK_HANDLED;
}
-
-static struct break_hook cfi_break_hook = {
- .fn = cfi_handler,
- .imm = CFI_BRK_IMM_BASE,
- .mask = CFI_BRK_IMM_MASK,
-};
#endif /* CONFIG_CFI_CLANG */
-static int reserved_fault_handler(struct pt_regs *regs, unsigned long esr)
+int reserved_fault_brk_handler(struct pt_regs *regs, unsigned long esr)
{
pr_err("%s generated an invalid instruction at %pS!\n",
"Kernel text patching",
@@ -1064,11 +1053,6 @@ static int reserved_fault_handler(struct pt_regs *regs, unsigned long esr)
return DBG_HOOK_ERROR;
}
-static struct break_hook fault_break_hook = {
- .fn = reserved_fault_handler,
- .imm = FAULT_BRK_IMM,
-};
-
#ifdef CONFIG_KASAN_SW_TAGS
#define KASAN_ESR_RECOVER 0x20
@@ -1076,7 +1060,7 @@ static struct break_hook fault_break_hook = {
#define KASAN_ESR_SIZE_MASK 0x0f
#define KASAN_ESR_SIZE(esr) (1 << ((esr) & KASAN_ESR_SIZE_MASK))
-static int kasan_handler(struct pt_regs *regs, unsigned long esr)
+int kasan_brk_handler(struct pt_regs *regs, unsigned long esr)
{
bool recover = esr & KASAN_ESR_RECOVER;
bool write = esr & KASAN_ESR_WRITE;
@@ -1107,26 +1091,14 @@ static int kasan_handler(struct pt_regs *regs, unsigned long esr)
arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE);
return DBG_HOOK_HANDLED;
}
-
-static struct break_hook kasan_break_hook = {
- .fn = kasan_handler,
- .imm = KASAN_BRK_IMM,
- .mask = KASAN_BRK_MASK,
-};
#endif
#ifdef CONFIG_UBSAN_TRAP
-static int ubsan_handler(struct pt_regs *regs, unsigned long esr)
+int ubsan_brk_handler(struct pt_regs *regs, unsigned long esr)
{
die(report_ubsan_failure(regs, esr & UBSAN_BRK_MASK), regs, esr);
return DBG_HOOK_HANDLED;
}
-
-static struct break_hook ubsan_break_hook = {
- .fn = ubsan_handler,
- .imm = UBSAN_BRK_IMM,
- .mask = UBSAN_BRK_MASK,
-};
#endif
/*
@@ -1138,31 +1110,20 @@ int __init early_brk64(unsigned long addr, unsigned long esr,
{
#ifdef CONFIG_CFI_CLANG
if (esr_is_cfi_brk(esr))
- return cfi_handler(regs, esr) != DBG_HOOK_HANDLED;
+ return cfi_brk_handler(regs, esr) != DBG_HOOK_HANDLED;
#endif
#ifdef CONFIG_KASAN_SW_TAGS
if ((esr_brk_comment(esr) & ~KASAN_BRK_MASK) == KASAN_BRK_IMM)
- return kasan_handler(regs, esr) != DBG_HOOK_HANDLED;
+ return kasan_brk_handler(regs, esr) != DBG_HOOK_HANDLED;
#endif
#ifdef CONFIG_UBSAN_TRAP
if ((esr_brk_comment(esr) & ~UBSAN_BRK_MASK) == UBSAN_BRK_IMM)
- return ubsan_handler(regs, esr) != DBG_HOOK_HANDLED;
+ return ubsan_brk_handler(regs, esr) != DBG_HOOK_HANDLED;
#endif
- return bug_handler(regs, esr) != DBG_HOOK_HANDLED;
+ return bug_brk_handler(regs, esr) != DBG_HOOK_HANDLED;
}
void __init trap_init(void)
{
- register_kernel_break_hook(&bug_break_hook);
-#ifdef CONFIG_CFI_CLANG
- register_kernel_break_hook(&cfi_break_hook);
-#endif
- register_kernel_break_hook(&fault_break_hook);
-#ifdef CONFIG_KASAN_SW_TAGS
- register_kernel_break_hook(&kasan_break_hook);
-#endif
-#ifdef CONFIG_UBSAN_TRAP
- register_kernel_break_hook(&ubsan_break_hook);
-#endif
debug_traps_init();
}
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 03/11] arm64: debug: call step handlers statically
2025-04-25 15:36 [PATCH 00/11] arm64: debug: remove hook registration, split exception entry Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 01/11] arm64: debug: Clean up single_step_handler logic Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 02/11] arm64: debug: call software break handlers statically Ada Couprie Diaz
@ 2025-04-25 15:36 ` Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 04/11] arm64: debug: remove break/step handler registration infrastructure Ada Couprie Diaz
` (7 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Ada Couprie Diaz @ 2025-04-25 15:36 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Will Deacon, Catalin Marinas
Software stepping checks for the correct handler by iterating over a list
of dynamically registered handlers and calling all of them until one
handles the exception.
This is the only generic way to handle software stepping handlers in arm64
as the exception does not provide an immediate that could be checked,
contrary to software breakpoints.
However, the registration mechanism is not exported and has only
two current users : the KGDB stepping handler, and the uprobe single step
handler.
Given that one comes from user mode and the other from kernel mode, call
the appropriate one by checking the source EL of the exception, if it
is enabled.
Unify the naming of the handler to XXX_singlestep_handler(), making it
clear they are related.
Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
---
arch/arm64/include/asm/kgdb.h | 1 +
arch/arm64/include/asm/uprobes.h | 1 +
arch/arm64/kernel/debug-monitors.c | 32 +++++++++++++-----------------
arch/arm64/kernel/kgdb.c | 17 +++-------------
arch/arm64/kernel/probes/uprobes.c | 9 +--------
5 files changed, 20 insertions(+), 40 deletions(-)
diff --git a/arch/arm64/include/asm/kgdb.h b/arch/arm64/include/asm/kgdb.h
index 82a76b2102fb..fd287ec38bb7 100644
--- a/arch/arm64/include/asm/kgdb.h
+++ b/arch/arm64/include/asm/kgdb.h
@@ -26,6 +26,7 @@ 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);
+int kgdb_singlestep_handler(struct pt_regs *regs, unsigned long esr);
#endif /* !__ASSEMBLY__ */
diff --git a/arch/arm64/include/asm/uprobes.h b/arch/arm64/include/asm/uprobes.h
index 3659a79a9f32..e44bbef40eca 100644
--- a/arch/arm64/include/asm/uprobes.h
+++ b/arch/arm64/include/asm/uprobes.h
@@ -29,5 +29,6 @@ struct arch_uprobe {
};
int uprobe_brk_handler(struct pt_regs *regs, unsigned long esr);
+int uprobe_singlestep_handler(struct pt_regs *regs, unsigned long esr);
#endif
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 4ece4a93b872..81b813e16842 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -200,30 +200,26 @@ void unregister_kernel_step_hook(struct step_hook *hook)
}
/*
- * Call registered single step handlers
+ * Call single step handlers
* There is no Syndrome info to check for determining the handler.
- * So we call all the registered handlers, until the right handler is
- * found which returns zero.
+ * However, there is only one possible handler for user and kernel modes, so
+ * check and call the appropriate one if it is enabled.
*/
static int call_step_hook(struct pt_regs *regs, unsigned long esr)
{
- struct step_hook *hook;
- struct list_head *list;
- int retval = DBG_HOOK_ERROR;
-
- list = user_mode(regs) ? &user_step_hook : &kernel_step_hook;
-
- /*
- * Since single-step 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) {
- retval = hook->fn(regs, esr);
- if (retval == DBG_HOOK_HANDLED)
- break;
+ if (user_mode(regs)) {
+#if CONFIG_UPROBES
+ return uprobe_singlestep_handler(regs, esr);
+#else
+ return DBG_HOOK_ERROR;
+#endif
}
- return retval;
+#ifdef CONFIG_KGDB
+ return kgdb_singlestep_handler(regs, esr);
+#else
+ return DBG_HOOK_ERROR;
+#endif
}
NOKPROBE_SYMBOL(call_step_hook);
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index b5a3b9c85a65..8f6ce2ea005c 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -250,7 +250,7 @@ int kgdb_compiled_brk_handler(struct pt_regs *regs, unsigned long esr)
}
NOKPROBE_SYMBOL(kgdb_compiled_brk_handler);
-static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned long esr)
+int kgdb_singlestep_handler(struct pt_regs *regs, unsigned long esr)
{
if (!kgdb_single_step)
return DBG_HOOK_ERROR;
@@ -258,11 +258,7 @@ static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned long esr)
kgdb_handle_exception(0, SIGTRAP, 0, regs);
return DBG_HOOK_HANDLED;
}
-NOKPROBE_SYMBOL(kgdb_step_brk_fn);
-
-static struct step_hook kgdb_step_hook = {
- .fn = kgdb_step_brk_fn
-};
+NOKPROBE_SYMBOL(kgdb_singlestep_handler);
static int __kgdb_notify(struct die_args *args, unsigned long cmd)
{
@@ -301,13 +297,7 @@ static struct notifier_block kgdb_notifier = {
*/
int kgdb_arch_init(void)
{
- int ret = register_die_notifier(&kgdb_notifier);
-
- if (ret != 0)
- return ret;
-
- register_kernel_step_hook(&kgdb_step_hook);
- return 0;
+ return register_die_notifier(&kgdb_notifier);
}
/*
@@ -317,7 +307,6 @@ int kgdb_arch_init(void)
*/
void kgdb_arch_exit(void)
{
- unregister_kernel_step_hook(&kgdb_step_hook);
unregister_die_notifier(&kgdb_notifier);
}
diff --git a/arch/arm64/kernel/probes/uprobes.c b/arch/arm64/kernel/probes/uprobes.c
index ad68b4a5974d..fefc990860bc 100644
--- a/arch/arm64/kernel/probes/uprobes.c
+++ b/arch/arm64/kernel/probes/uprobes.c
@@ -182,7 +182,7 @@ int uprobe_brk_handler(struct pt_regs *regs,
return DBG_HOOK_ERROR;
}
-static int uprobe_single_step_handler(struct pt_regs *regs,
+int uprobe_singlestep_handler(struct pt_regs *regs,
unsigned long esr)
{
struct uprobe_task *utask = current->utask;
@@ -194,15 +194,8 @@ static int uprobe_single_step_handler(struct pt_regs *regs,
return DBG_HOOK_ERROR;
}
-/* uprobe single step handler hook */
-static struct step_hook uprobes_step_hook = {
- .fn = uprobe_single_step_handler,
-};
-
static int __init arch_init_uprobes(void)
{
- register_user_step_hook(&uprobes_step_hook);
-
return 0;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 04/11] arm64: debug: remove break/step handler registration infrastructure
2025-04-25 15:36 [PATCH 00/11] arm64: debug: remove hook registration, split exception entry Ada Couprie Diaz
` (2 preceding siblings ...)
2025-04-25 15:36 ` [PATCH 03/11] arm64: debug: call step " Ada Couprie Diaz
@ 2025-04-25 15:36 ` Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 05/11] arm64: entry: Add entry and exit functions for debug exceptions Ada Couprie Diaz
` (6 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Ada Couprie Diaz @ 2025-04-25 15:36 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Will Deacon, Catalin Marinas
Remove all infrastructure for the dynamic registration previously used by
software breakpoints and stepping handlers.
Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
---
arch/arm64/include/asm/debug-monitors.h | 24 ----------
arch/arm64/kernel/debug-monitors.c | 63 -------------------------
2 files changed, 87 deletions(-)
diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index 8f6ba31b8658..9aabf65de693 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -62,30 +62,6 @@ struct task_struct;
#define DBG_HOOK_HANDLED 0
#define DBG_HOOK_ERROR 1
-struct step_hook {
- struct list_head node;
- int (*fn)(struct pt_regs *regs, unsigned long esr);
-};
-
-void register_user_step_hook(struct step_hook *hook);
-void unregister_user_step_hook(struct step_hook *hook);
-
-void register_kernel_step_hook(struct step_hook *hook);
-void unregister_kernel_step_hook(struct step_hook *hook);
-
-struct break_hook {
- struct list_head node;
- int (*fn)(struct pt_regs *regs, unsigned long esr);
- u16 imm;
- u16 mask; /* These bits are ignored when comparing with imm */
-};
-
-void register_user_break_hook(struct break_hook *hook);
-void unregister_user_break_hook(struct break_hook *hook);
-
-void register_kernel_break_hook(struct break_hook *hook);
-void unregister_kernel_break_hook(struct break_hook *hook);
-
u8 debug_monitors_arch(void);
enum dbg_active_el {
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 81b813e16842..55bca019ef5c 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -159,46 +159,6 @@ NOKPROBE_SYMBOL(clear_user_regs_spsr_ss);
#define set_regs_spsr_ss(r) set_user_regs_spsr_ss(&(r)->user_regs)
#define clear_regs_spsr_ss(r) clear_user_regs_spsr_ss(&(r)->user_regs)
-static DEFINE_SPINLOCK(debug_hook_lock);
-static LIST_HEAD(user_step_hook);
-static LIST_HEAD(kernel_step_hook);
-
-static void register_debug_hook(struct list_head *node, struct list_head *list)
-{
- spin_lock(&debug_hook_lock);
- list_add_rcu(node, list);
- spin_unlock(&debug_hook_lock);
-
-}
-
-static void unregister_debug_hook(struct list_head *node)
-{
- spin_lock(&debug_hook_lock);
- list_del_rcu(node);
- spin_unlock(&debug_hook_lock);
- synchronize_rcu();
-}
-
-void register_user_step_hook(struct step_hook *hook)
-{
- register_debug_hook(&hook->node, &user_step_hook);
-}
-
-void unregister_user_step_hook(struct step_hook *hook)
-{
- unregister_debug_hook(&hook->node);
-}
-
-void register_kernel_step_hook(struct step_hook *hook)
-{
- register_debug_hook(&hook->node, &kernel_step_hook);
-}
-
-void unregister_kernel_step_hook(struct step_hook *hook)
-{
- unregister_debug_hook(&hook->node);
-}
-
/*
* Call single step handlers
* There is no Syndrome info to check for determining the handler.
@@ -273,29 +233,6 @@ static int single_step_handler(unsigned long unused, unsigned long esr,
}
NOKPROBE_SYMBOL(single_step_handler);
-static LIST_HEAD(user_break_hook);
-static LIST_HEAD(kernel_break_hook);
-
-void register_user_break_hook(struct break_hook *hook)
-{
- register_debug_hook(&hook->node, &user_break_hook);
-}
-
-void unregister_user_break_hook(struct break_hook *hook)
-{
- unregister_debug_hook(&hook->node);
-}
-
-void register_kernel_break_hook(struct break_hook *hook)
-{
- register_debug_hook(&hook->node, &kernel_break_hook);
-}
-
-void unregister_kernel_break_hook(struct break_hook *hook)
-{
- unregister_debug_hook(&hook->node);
-}
-
static int call_break_hook(struct pt_regs *regs, unsigned long esr)
{
if (user_mode(regs)) {
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 05/11] arm64: entry: Add entry and exit functions for debug exceptions
2025-04-25 15:36 [PATCH 00/11] arm64: debug: remove hook registration, split exception entry Ada Couprie Diaz
` (3 preceding siblings ...)
2025-04-25 15:36 ` [PATCH 04/11] arm64: debug: remove break/step handler registration infrastructure Ada Couprie Diaz
@ 2025-04-25 15:36 ` Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 06/11] arm64: debug: split hardware breakpoint exeception entry Ada Couprie Diaz
` (5 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Ada Couprie Diaz @ 2025-04-25 15:36 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Will Deacon, Catalin Marinas
Duplicate the `debug_exception_entry()` and `debug_exception_exit()`
functions from mm/fault.c, as they are needed to split
the debug exceptions entry paths from the current unified one.
The original will be removed in a cleanup patch.
Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
---
arch/arm64/kernel/entry-common.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index b260ddc4d3e9..92d78b329e67 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -418,6 +418,28 @@ static inline void fp_user_discard(void)
}
}
+/*
+ * In debug exception context, we explicitly disable preemption despite
+ * having interrupts disabled.
+ * This serves two purposes: it makes it much less likely that we would
+ * 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)
+{
+ preempt_disable();
+
+ /* This code is a bit fragile. Test it. */
+ RCU_LOCKDEP_WARN(!rcu_is_watching(), "exception_enter didn't work");
+}
+NOKPROBE_SYMBOL(debug_exception_enter);
+
+static void debug_exception_exit(struct pt_regs *regs)
+{
+ preempt_enable_no_resched();
+}
+NOKPROBE_SYMBOL(debug_exception_exit);
+
UNHANDLED(el1t, 64, sync)
UNHANDLED(el1t, 64, irq)
UNHANDLED(el1t, 64, fiq)
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 06/11] arm64: debug: split hardware breakpoint exeception entry
2025-04-25 15:36 [PATCH 00/11] arm64: debug: remove hook registration, split exception entry Ada Couprie Diaz
` (4 preceding siblings ...)
2025-04-25 15:36 ` [PATCH 05/11] arm64: entry: Add entry and exit functions for debug exceptions Ada Couprie Diaz
@ 2025-04-25 15:36 ` Ada Couprie Diaz
2025-04-25 15:54 ` Ada Couprie Diaz
2025-05-01 11:15 ` Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 07/11] arm64: debug: split single stepping exception entry Ada Couprie Diaz
` (4 subsequent siblings)
10 siblings, 2 replies; 15+ messages in thread
From: Ada Couprie Diaz @ 2025-04-25 15:36 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Will Deacon, Catalin Marinas
Currently all debug exceptions share common entry code and are routed
to `do_debug_exception()`, which calls dynamically-registered
handlers for each specific debug exception. This is unfortunate as
different debug exceptions have different entry handling requirements,
and it would be better to handle these distinct requirements earlier.
Hardware breakpoints exceptions are generated by the hardware after user
configuration. As such, they can be exploited when training branch
predictors outisde of the userspace VA range: they still need to call
`arm64_apply_bp_hardening()` if needed to mitigate against this attack.
However, they do not need to handle the Cortex-A76 erratum #1463225 as
it only applies to single stepping exceptions.
It does not set an address in FAR_EL1 either, only the hardware
watchpoint does.
Split the hardware breakpoint exception entry, adjust
the function signature, and handling of the Cortex-A76 erratum to fit
the behaviour of the exception.
Duplicate the debug_exception_ entry and exit functions from mm/fault.c,
as they are needed in the split paths for exceptions other than BKPT32.
The original will be removed in a cleanup patch.
Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
---
arch/arm64/include/asm/exception.h | 1 +
arch/arm64/kernel/entry-common.c | 25 +++++++++++++++++++++++++
arch/arm64/kernel/hw_breakpoint.c | 20 ++++++++++++++++----
3 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index d48fc16584cd..c593fe639697 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -61,6 +61,7 @@ void do_el0_gcs(struct pt_regs *regs, unsigned long esr);
void do_el1_gcs(struct pt_regs *regs, unsigned long esr);
void do_debug_exception(unsigned long addr_if_watchpoint, unsigned long esr,
struct pt_regs *regs);
+void do_breakpoint(unsigned long esr, struct pt_regs *regs);
void do_fpsimd_acc(unsigned long esr, struct pt_regs *regs);
void do_sve_acc(unsigned long esr, struct pt_regs *regs);
void do_sme_acc(unsigned long esr, struct pt_regs *regs);
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 92d78b329e67..c8139c87caa6 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -503,6 +503,15 @@ static void noinstr el1_mops(struct pt_regs *regs, unsigned long esr)
exit_to_kernel_mode(regs);
}
+static void noinstr el1_breakpt(struct pt_regs *regs, unsigned long esr)
+{
+ arm64_enter_el1_dbg(regs);
+ debug_exception_enter(regs);
+ do_breakpoint(esr, regs);
+ debug_exception_exit(regs);
+ arm64_exit_el1_dbg(regs);
+}
+
static void noinstr el1_dbg(struct pt_regs *regs, unsigned long esr)
{
unsigned long far = read_sysreg(far_el1);
@@ -552,6 +561,8 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
el1_mops(regs, esr);
break;
case ESR_ELx_EC_BREAKPT_CUR:
+ el1_breakpt(regs, esr);
+ break;
case ESR_ELx_EC_SOFTSTP_CUR:
case ESR_ELx_EC_WATCHPT_CUR:
case ESR_ELx_EC_BRK64:
@@ -746,6 +757,16 @@ static void noinstr el0_inv(struct pt_regs *regs, unsigned long esr)
exit_to_user_mode(regs);
}
+static void noinstr el0_breakpt(struct pt_regs *regs, unsigned long esr)
+{
+ enter_from_user_mode(regs);
+ debug_exception_enter(regs);
+ do_breakpoint(esr, regs);
+ debug_exception_exit(regs);
+ local_daif_restore(DAIF_PROCCTX);
+ exit_to_user_mode(regs);
+}
+
static void noinstr el0_dbg(struct pt_regs *regs, unsigned long esr)
{
/* Only watchpoints write FAR_EL1, otherwise its UNKNOWN */
@@ -824,6 +845,8 @@ asmlinkage void noinstr el0t_64_sync_handler(struct pt_regs *regs)
el0_gcs(regs, esr);
break;
case ESR_ELx_EC_BREAKPT_LOW:
+ el0_breakpt(regs, esr);
+ break;
case ESR_ELx_EC_SOFTSTP_LOW:
case ESR_ELx_EC_WATCHPT_LOW:
case ESR_ELx_EC_BRK64:
@@ -944,6 +967,8 @@ asmlinkage void noinstr el0t_32_sync_handler(struct pt_regs *regs)
el0_cp15(regs, esr);
break;
case ESR_ELx_EC_BREAKPT_LOW:
+ el0_breakpt(regs, esr);
+ break;
case ESR_ELx_EC_SOFTSTP_LOW:
case ESR_ELx_EC_WATCHPT_LOW:
case ESR_ELx_EC_BKPT32:
diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index 722ac45f9f7b..8a5b6277bca8 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c
@@ -22,6 +22,7 @@
#include <asm/current.h>
#include <asm/debug-monitors.h>
#include <asm/esr.h>
+#include <asm/exception.h>
#include <asm/hw_breakpoint.h>
#include <asm/traps.h>
#include <asm/cputype.h>
@@ -618,8 +619,7 @@ NOKPROBE_SYMBOL(toggle_bp_registers);
/*
* Debug exception handlers.
*/
-static int breakpoint_handler(unsigned long unused, unsigned long esr,
- struct pt_regs *regs)
+static int breakpoint_handler(unsigned long esr, struct pt_regs *regs)
{
int i, step = 0, *kernel_step;
u32 ctrl_reg;
@@ -695,6 +695,20 @@ static int breakpoint_handler(unsigned long unused, unsigned long esr,
}
NOKPROBE_SYMBOL(breakpoint_handler);
+void do_breakpoint(unsigned long esr, struct pt_regs *regs)
+{
+ unsigned long pc = regs->pc;
+
+ if (user_mode(regs) && !is_ttbr0_addr(pc))
+ arm64_apply_bp_hardening();
+
+ if (breakpoint_handler(esr, regs)) {
+ arm64_notify_die("hw-breakpoint handler", regs, SIGTRAP, TRAP_HWBKPT,
+ pc, esr);
+ }
+}
+NOKPROBE_SYMBOL(do_breakpoint);
+
/*
* Arm64 hardware does not always report a watchpoint hit address that matches
* one of the watchpoints set. It can also report an address "near" the
@@ -988,8 +1002,6 @@ static int __init arch_hw_breakpoint_init(void)
core_num_brps, core_num_wrps);
/* Register debug fault handlers. */
- hook_debug_fault_code(DBG_ESR_EVT_HWBP, breakpoint_handler, SIGTRAP,
- TRAP_HWBKPT, "hw-breakpoint handler");
hook_debug_fault_code(DBG_ESR_EVT_HWWP, watchpoint_handler, SIGTRAP,
TRAP_HWBKPT, "hw-watchpoint handler");
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 07/11] arm64: debug: split single stepping exception entry
2025-04-25 15:36 [PATCH 00/11] arm64: debug: remove hook registration, split exception entry Ada Couprie Diaz
` (5 preceding siblings ...)
2025-04-25 15:36 ` [PATCH 06/11] arm64: debug: split hardware breakpoint exeception entry Ada Couprie Diaz
@ 2025-04-25 15:36 ` Ada Couprie Diaz
2025-05-01 11:36 ` Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 08/11] arm64: debug: split hardware watchpoint " Ada Couprie Diaz
` (3 subsequent siblings)
10 siblings, 1 reply; 15+ messages in thread
From: Ada Couprie Diaz @ 2025-04-25 15:36 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Will Deacon, Catalin Marinas
Currently all debug exceptions share common entry code and are routed
to `do_debug_exception()`, which calls dynamically-registered
handlers for each specific debug exception. This is unfortunate as
different debug exceptions have different entry handling requirements,
and it would be better to handle these distinct requirements earlier.
The single stepping exception has the most constraints : it can be
exploited to train branch predictors and it needs special handling at EL1
for the Cortex-A76 erratum #1463225. We need to conserve all those
mitigations.
However, it does not write an address at FAR_EL1, as only hardware
watchpoints do so.
Split the single stepping exception entry, adjust the function signature
but keep the security mitigation and erratum handling.
Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
---
arch/arm64/include/asm/exception.h | 1 +
arch/arm64/kernel/debug-monitors.c | 20 ++++++++++++++++----
arch/arm64/kernel/entry-common.c | 27 +++++++++++++++++++++++++++
3 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index c593fe639697..cbcd832bf58e 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -62,6 +62,7 @@ void do_el1_gcs(struct pt_regs *regs, unsigned long esr);
void do_debug_exception(unsigned long addr_if_watchpoint, unsigned long esr,
struct pt_regs *regs);
void do_breakpoint(unsigned long esr, struct pt_regs *regs);
+void do_softstep(unsigned long esr, struct pt_regs *regs);
void do_fpsimd_acc(unsigned long esr, struct pt_regs *regs);
void do_sve_acc(unsigned long esr, struct pt_regs *regs);
void do_sme_acc(unsigned long esr, struct pt_regs *regs);
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 55bca019ef5c..e02724947b68 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -21,6 +21,7 @@
#include <asm/cputype.h>
#include <asm/daifflags.h>
#include <asm/debug-monitors.h>
+#include <asm/exception.h>
#include <asm/kgdb.h>
#include <asm/kprobes.h>
#include <asm/system_misc.h>
@@ -197,8 +198,7 @@ static void send_user_sigtrap(int si_code)
"User debug trap");
}
-static int single_step_handler(unsigned long unused, unsigned long esr,
- struct pt_regs *regs)
+static int single_step_handler(unsigned long esr, struct pt_regs *regs)
{
/*
* If we are stepping a pending breakpoint, call the hw_breakpoint
@@ -233,6 +233,20 @@ static int single_step_handler(unsigned long unused, unsigned long esr,
}
NOKPROBE_SYMBOL(single_step_handler);
+void do_softstep(unsigned long esr, struct pt_regs *regs)
+{
+ unsigned long pc = instruction_pointer(regs);
+
+ if (user_mode(regs) && !is_ttbr0_addr(pc))
+ arm64_apply_bp_hardening();
+
+ if (single_step_handler(esr, regs)) {
+ arm64_notify_die("single-step handler", regs, SIGTRAP, TRAP_TRACE, pc,
+ esr);
+ }
+}
+NOKPROBE_SYMBOL(do_softstep);
+
static int call_break_hook(struct pt_regs *regs, unsigned long esr)
{
if (user_mode(regs)) {
@@ -341,8 +355,6 @@ NOKPROBE_SYMBOL(aarch32_break_handler);
void __init debug_traps_init(void)
{
- hook_debug_fault_code(DBG_ESR_EVT_HWSS, single_step_handler, SIGTRAP,
- TRAP_TRACE, "single-step handler");
hook_debug_fault_code(DBG_ESR_EVT_BRK, brk_handler, SIGTRAP,
TRAP_BRKPT, "BRK handler");
}
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index c8139c87caa6..b0da8637fc64 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -512,6 +512,17 @@ static void noinstr el1_breakpt(struct pt_regs *regs, unsigned long esr)
arm64_exit_el1_dbg(regs);
}
+static void noinstr el1_softstp(struct pt_regs *regs, unsigned long esr)
+{
+ arm64_enter_el1_dbg(regs);
+ if (!cortex_a76_erratum_1463225_debug_handler(regs)) {
+ debug_exception_enter(regs);
+ do_softstep(esr, regs);
+ debug_exception_exit(regs);
+ }
+ arm64_exit_el1_dbg(regs);
+}
+
static void noinstr el1_dbg(struct pt_regs *regs, unsigned long esr)
{
unsigned long far = read_sysreg(far_el1);
@@ -564,6 +575,8 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
el1_breakpt(regs, esr);
break;
case ESR_ELx_EC_SOFTSTP_CUR:
+ el1_softstp(regs, esr);
+ break;
case ESR_ELx_EC_WATCHPT_CUR:
case ESR_ELx_EC_BRK64:
el1_dbg(regs, esr);
@@ -767,6 +780,16 @@ static void noinstr el0_breakpt(struct pt_regs *regs, unsigned long esr)
exit_to_user_mode(regs);
}
+static void noinstr el0_softstp(struct pt_regs *regs, unsigned long esr)
+{
+ enter_from_user_mode(regs);
+ debug_exception_enter(regs);
+ do_softstep(esr, regs);
+ debug_exception_exit(regs);
+ local_daif_restore(DAIF_PROCCTX);
+ exit_to_user_mode(regs);
+}
+
static void noinstr el0_dbg(struct pt_regs *regs, unsigned long esr)
{
/* Only watchpoints write FAR_EL1, otherwise its UNKNOWN */
@@ -848,6 +871,8 @@ asmlinkage void noinstr el0t_64_sync_handler(struct pt_regs *regs)
el0_breakpt(regs, esr);
break;
case ESR_ELx_EC_SOFTSTP_LOW:
+ el0_softstp(regs, esr);
+ break;
case ESR_ELx_EC_WATCHPT_LOW:
case ESR_ELx_EC_BRK64:
el0_dbg(regs, esr);
@@ -970,6 +995,8 @@ asmlinkage void noinstr el0t_32_sync_handler(struct pt_regs *regs)
el0_breakpt(regs, esr);
break;
case ESR_ELx_EC_SOFTSTP_LOW:
+ el0_softstp(regs, esr);
+ break;
case ESR_ELx_EC_WATCHPT_LOW:
case ESR_ELx_EC_BKPT32:
el0_dbg(regs, esr);
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 08/11] arm64: debug: split hardware watchpoint exception entry
2025-04-25 15:36 [PATCH 00/11] arm64: debug: remove hook registration, split exception entry Ada Couprie Diaz
` (6 preceding siblings ...)
2025-04-25 15:36 ` [PATCH 07/11] arm64: debug: split single stepping exception entry Ada Couprie Diaz
@ 2025-04-25 15:36 ` Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 09/11] arm64: debug: split brk64 " Ada Couprie Diaz
` (2 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Ada Couprie Diaz @ 2025-04-25 15:36 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Will Deacon, Catalin Marinas
Currently all debug exceptions share common entry code and are routed
to `do_debug_exception()`, which calls dynamically-registered
handlers for each specific debug exception. This is unfortunate as
different debug exceptions have different entry handling requirements,
and it would be better to handle these distinct requirements earlier.
Hardware watchpoints are the only debug exceptions that will write
FAR_EL1, so we need to preserve it and pass it down.
However, they cannot be used to maliciously train branch predictors, so
we can omit calling `arm64_bp_hardening()`, nor do they need to handle
the Cortex-A76 erratum #1463225, as it only applies to single stepping
exceptions.
Split the hardware watchpoint exception entry and adjust the behaviour
to match the lack of needed mitigations.
Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
---
arch/arm64/include/asm/exception.h | 2 ++
arch/arm64/kernel/entry-common.c | 31 +++++++++++++++++++++++++++++-
arch/arm64/kernel/hw_breakpoint.c | 14 ++++++++++----
3 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index cbcd832bf58e..eb465c375d34 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -63,6 +63,8 @@ void do_debug_exception(unsigned long addr_if_watchpoint, unsigned long esr,
struct pt_regs *regs);
void do_breakpoint(unsigned long esr, struct pt_regs *regs);
void do_softstep(unsigned long esr, struct pt_regs *regs);
+void do_watchpoint(unsigned long addr, unsigned long esr,
+ struct pt_regs *regs);
void do_fpsimd_acc(unsigned long esr, struct pt_regs *regs);
void do_sve_acc(unsigned long esr, struct pt_regs *regs);
void do_sme_acc(unsigned long esr, struct pt_regs *regs);
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index b0da8637fc64..8801c3aa846f 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -523,10 +523,20 @@ static void noinstr el1_softstp(struct pt_regs *regs, unsigned long esr)
arm64_exit_el1_dbg(regs);
}
-static void noinstr el1_dbg(struct pt_regs *regs, unsigned long esr)
+static void noinstr el1_watchpt(struct pt_regs *regs, unsigned long esr)
{
+ /* Only watchpoints write FAR_EL1 */
unsigned long far = read_sysreg(far_el1);
+ arm64_enter_el1_dbg(regs);
+ debug_exception_enter(regs);
+ do_watchpoint(far, esr, regs);
+ debug_exception_exit(regs);
+ arm64_exit_el1_dbg(regs);
+}
+
+static void noinstr el1_dbg(struct pt_regs *regs, unsigned long esr)
+{
arm64_enter_el1_dbg(regs);
if (!cortex_a76_erratum_1463225_debug_handler(regs))
do_debug_exception(far, esr, regs);
@@ -578,6 +588,8 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
el1_softstp(regs, esr);
break;
case ESR_ELx_EC_WATCHPT_CUR:
+ el1_watchpt(regs, esr);
+ break;
case ESR_ELx_EC_BRK64:
el1_dbg(regs, esr);
break;
@@ -790,6 +802,19 @@ static void noinstr el0_softstp(struct pt_regs *regs, unsigned long esr)
exit_to_user_mode(regs);
}
+static void noinstr el0_watchpt(struct pt_regs *regs, unsigned long esr)
+{
+ /* Only watchpoints write FAR_EL1 */
+ unsigned long far = read_sysreg(far_el1);
+
+ enter_from_user_mode(regs);
+ debug_exception_enter(regs);
+ do_watchpoint(far, esr, regs);
+ debug_exception_exit(regs);
+ local_daif_restore(DAIF_PROCCTX);
+ exit_to_user_mode(regs);
+}
+
static void noinstr el0_dbg(struct pt_regs *regs, unsigned long esr)
{
/* Only watchpoints write FAR_EL1, otherwise its UNKNOWN */
@@ -874,6 +899,8 @@ asmlinkage void noinstr el0t_64_sync_handler(struct pt_regs *regs)
el0_softstp(regs, esr);
break;
case ESR_ELx_EC_WATCHPT_LOW:
+ el0_watchpt(regs, esr);
+ break;
case ESR_ELx_EC_BRK64:
el0_dbg(regs, esr);
break;
@@ -998,6 +1025,8 @@ asmlinkage void noinstr el0t_32_sync_handler(struct pt_regs *regs)
el0_softstp(regs, esr);
break;
case ESR_ELx_EC_WATCHPT_LOW:
+ el0_watchpt(regs, esr);
+ break;
case ESR_ELx_EC_BKPT32:
el0_dbg(regs, esr);
break;
diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index 8a5b6277bca8..d6b51e762562 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c
@@ -860,6 +860,16 @@ static int watchpoint_handler(unsigned long addr, unsigned long esr,
}
NOKPROBE_SYMBOL(watchpoint_handler);
+void do_watchpoint(unsigned long addr, unsigned long esr,
+ struct pt_regs *regs)
+{
+ if (watchpoint_handler(addr, esr, regs)) {
+ arm64_notify_die("hw-watchpoint handler", regs, SIGTRAP, TRAP_HWBKPT,
+ regs->pc, esr);
+ }
+}
+NOKPROBE_SYMBOL(do_watchpoint);
+
/*
* Handle single-step exception.
*/
@@ -1001,10 +1011,6 @@ static int __init arch_hw_breakpoint_init(void)
pr_info("found %d breakpoint and %d watchpoint registers.\n",
core_num_brps, core_num_wrps);
- /* Register debug fault handlers. */
- hook_debug_fault_code(DBG_ESR_EVT_HWWP, watchpoint_handler, SIGTRAP,
- TRAP_HWBKPT, "hw-watchpoint handler");
-
/*
* Reset the breakpoint resources. We assume that a halting
* debugger will leave the world in a nice state for us.
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 09/11] arm64: debug: split brk64 exception entry
2025-04-25 15:36 [PATCH 00/11] arm64: debug: remove hook registration, split exception entry Ada Couprie Diaz
` (7 preceding siblings ...)
2025-04-25 15:36 ` [PATCH 08/11] arm64: debug: split hardware watchpoint " Ada Couprie Diaz
@ 2025-04-25 15:36 ` Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 10/11] arm64: debug: split bkpt32 " Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 11/11] arm64: debug: remove debug exception registration infrastructure Ada Couprie Diaz
10 siblings, 0 replies; 15+ messages in thread
From: Ada Couprie Diaz @ 2025-04-25 15:36 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Will Deacon, Catalin Marinas
Currently all debug exceptions share common entry code and are routed
to `do_debug_exception()`, which calls dynamically-registered
handlers for each specific debug exception. This is unfortunate as
different debug exceptions have different entry handling requirements,
and it would be better to handle these distinct requirements earlier.
The BRK64 instruction can only be triggered by a BRK instruction. Thus,
we know that the PC is a legitimate address and isn't being used to train
a branch predictor with a bogus address : we don't need to call
`arm64_apply_bp_hardening()`.
We do not need to handle the Cortex-A76 erratum #1463225 either, as it
only relevant for single stepping at EL1.
BRK64 does not write FAR_EL1 either, as only hardware watchpoints do so.
Split the BRK64 exception entry, adjust the function signature, and its
behaviour to match the lack of needed mitigations.
Given that the break hook registration is handled statically in
`call_break_hook` since
(arm64: debug: call software break handlers statically)
and that we now bypass the exception handler registration, this change
renders `early_brk64` redundant : its functionality is now handled through
the post-init path.
This also removes the last usage of `el1_dbg()`.
Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
---
arch/arm64/include/asm/exception.h | 1 +
arch/arm64/kernel/debug-monitors.c | 16 ++++++++++------
arch/arm64/kernel/entry-common.c | 21 ++++++++++++++++-----
3 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index eb465c375d34..af9b521e35f9 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -65,6 +65,7 @@ void do_breakpoint(unsigned long esr, struct pt_regs *regs);
void do_softstep(unsigned long esr, struct pt_regs *regs);
void do_watchpoint(unsigned long addr, unsigned long esr,
struct pt_regs *regs);
+void do_brk64(unsigned long esr, struct pt_regs *regs);
void do_fpsimd_acc(unsigned long esr, struct pt_regs *regs);
void do_sve_acc(unsigned long esr, struct pt_regs *regs);
void do_sme_acc(unsigned long esr, struct pt_regs *regs);
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index e02724947b68..2209aff73d51 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -297,8 +297,7 @@ static int call_break_hook(struct pt_regs *regs, unsigned long esr)
}
NOKPROBE_SYMBOL(call_break_hook);
-static int brk_handler(unsigned long unused, unsigned long esr,
- struct pt_regs *regs)
+static int brk_handler(unsigned long esr, struct pt_regs *regs)
{
if (call_break_hook(regs, esr) == DBG_HOOK_HANDLED)
return 0;
@@ -314,6 +313,14 @@ static int brk_handler(unsigned long unused, unsigned long esr,
}
NOKPROBE_SYMBOL(brk_handler);
+void do_brk64(unsigned long esr, struct pt_regs *regs)
+{
+ if (brk_handler(esr, regs))
+ arm64_notify_die("BRK handler", regs, SIGTRAP, TRAP_BRKPT, regs->pc,
+ esr);
+}
+NOKPROBE_SYMBOL(do_brk64);
+
int aarch32_break_handler(struct pt_regs *regs)
{
u32 arm_instr;
@@ -354,10 +361,7 @@ int aarch32_break_handler(struct pt_regs *regs)
NOKPROBE_SYMBOL(aarch32_break_handler);
void __init debug_traps_init(void)
-{
- hook_debug_fault_code(DBG_ESR_EVT_BRK, brk_handler, SIGTRAP,
- TRAP_BRKPT, "BRK handler");
-}
+{}
/* Re-enable single step for syscall restarting. */
void user_rewind_single_step(struct task_struct *task)
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 8801c3aa846f..8f257cd28d53 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -535,11 +535,12 @@ static void noinstr el1_watchpt(struct pt_regs *regs, unsigned long esr)
arm64_exit_el1_dbg(regs);
}
-static void noinstr el1_dbg(struct pt_regs *regs, unsigned long esr)
+static void noinstr el1_brk64(struct pt_regs *regs, unsigned long esr)
{
arm64_enter_el1_dbg(regs);
- if (!cortex_a76_erratum_1463225_debug_handler(regs))
- do_debug_exception(far, esr, regs);
+ debug_exception_enter(regs);
+ do_brk64(esr, regs);
+ debug_exception_exit(regs);
arm64_exit_el1_dbg(regs);
}
@@ -591,7 +592,7 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
el1_watchpt(regs, esr);
break;
case ESR_ELx_EC_BRK64:
- el1_dbg(regs, esr);
+ el1_brk64(regs, esr);
break;
case ESR_ELx_EC_FPAC:
el1_fpac(regs, esr);
@@ -815,6 +816,16 @@ static void noinstr el0_watchpt(struct pt_regs *regs, unsigned long esr)
exit_to_user_mode(regs);
}
+static void noinstr el0_brk64(struct pt_regs *regs, unsigned long esr)
+{
+ enter_from_user_mode(regs);
+ debug_exception_enter(regs);
+ do_brk64(esr, regs);
+ debug_exception_exit(regs);
+ local_daif_restore(DAIF_PROCCTX);
+ exit_to_user_mode(regs);
+}
+
static void noinstr el0_dbg(struct pt_regs *regs, unsigned long esr)
{
/* Only watchpoints write FAR_EL1, otherwise its UNKNOWN */
@@ -902,7 +913,7 @@ asmlinkage void noinstr el0t_64_sync_handler(struct pt_regs *regs)
el0_watchpt(regs, esr);
break;
case ESR_ELx_EC_BRK64:
- el0_dbg(regs, esr);
+ el0_brk64(regs, esr);
break;
case ESR_ELx_EC_FPAC:
el0_fpac(regs, esr);
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 10/11] arm64: debug: split bkpt32 exception entry
2025-04-25 15:36 [PATCH 00/11] arm64: debug: remove hook registration, split exception entry Ada Couprie Diaz
` (8 preceding siblings ...)
2025-04-25 15:36 ` [PATCH 09/11] arm64: debug: split brk64 " Ada Couprie Diaz
@ 2025-04-25 15:36 ` Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 11/11] arm64: debug: remove debug exception registration infrastructure Ada Couprie Diaz
10 siblings, 0 replies; 15+ messages in thread
From: Ada Couprie Diaz @ 2025-04-25 15:36 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Will Deacon, Catalin Marinas
Currently all debug exceptions share common entry code and are routed
to `do_debug_exception()`, which calls dynamically-registered
handlers for each specific debug exception. This is unfortunate as
different debug exceptions have different entry handling requirements,
and it would be better to handle these distinct requirements earlier.
The BKPT32 exception can only be triggered by a BKPT instruction. Thus,
we know that the PC is a legitimate address and isn't being used to train
a branch predictor with a bogus address : we don't need to call
`arm64_apply_bp_hardening()`.
The handler for this exception only pends a signal and doesn't depend
on any per-CPU state : we don't need to inhibit preemption, nor do we
need to keep the DAIF exceptions masked, so we can unmask them earlier.
Split the BKPT32 exception entry and adjust function signatures and its
behaviour to match its relaxed constraints compared to other
debug exceptions.
This replaces the last usage of `el0_dbg()`, so remove it.
Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
---
arch/arm64/include/asm/exception.h | 1 +
arch/arm64/kernel/debug-monitors.c | 9 +++++++++
arch/arm64/kernel/entry-common.c | 21 +++++++++------------
3 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index af9b521e35f9..4ca43d4eaf98 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -66,6 +66,7 @@ void do_softstep(unsigned long esr, struct pt_regs *regs);
void do_watchpoint(unsigned long addr, unsigned long esr,
struct pt_regs *regs);
void do_brk64(unsigned long esr, struct pt_regs *regs);
+void do_bkpt32(unsigned long esr, struct pt_regs *regs);
void do_fpsimd_acc(unsigned long esr, struct pt_regs *regs);
void do_sve_acc(unsigned long esr, struct pt_regs *regs);
void do_sme_acc(unsigned long esr, struct pt_regs *regs);
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 2209aff73d51..7e4e85a212f4 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -321,6 +321,15 @@ void do_brk64(unsigned long esr, struct pt_regs *regs)
}
NOKPROBE_SYMBOL(do_brk64);
+#ifdef CONFIG_COMPAT
+/* BRKPT exception always traps to be handled. */
+void do_bkpt32(unsigned long esr, struct pt_regs *regs)
+{
+ arm64_notify_die("aarch32 BKPT", regs, SIGTRAP, TRAP_BRKPT, regs->pc, esr);
+}
+NOKPROBE_SYMBOL(do_bkpt32);
+#endif /* CONFIG_COMPAT */
+
int aarch32_break_handler(struct pt_regs *regs)
{
u32 arm_instr;
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 8f257cd28d53..20aeb2eb9239 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -826,17 +826,6 @@ static void noinstr el0_brk64(struct pt_regs *regs, unsigned long esr)
exit_to_user_mode(regs);
}
-static void noinstr el0_dbg(struct pt_regs *regs, unsigned long esr)
-{
- /* Only watchpoints write FAR_EL1, otherwise its UNKNOWN */
- unsigned long far = read_sysreg(far_el1);
-
- enter_from_user_mode(regs);
- do_debug_exception(far, esr, regs);
- local_daif_restore(DAIF_PROCCTX);
- exit_to_user_mode(regs);
-}
-
static void noinstr el0_svc(struct pt_regs *regs)
{
enter_from_user_mode(regs);
@@ -996,6 +985,14 @@ static void noinstr el0_svc_compat(struct pt_regs *regs)
exit_to_user_mode(regs);
}
+static void noinstr el0_bkpt32(struct pt_regs *regs, unsigned long esr)
+{
+ enter_from_user_mode(regs);
+ local_daif_restore(DAIF_PROCCTX);
+ do_bkpt32(esr, regs);
+ exit_to_user_mode(regs);
+}
+
asmlinkage void noinstr el0t_32_sync_handler(struct pt_regs *regs)
{
unsigned long esr = read_sysreg(esr_el1);
@@ -1039,7 +1036,7 @@ asmlinkage void noinstr el0t_32_sync_handler(struct pt_regs *regs)
el0_watchpt(regs, esr);
break;
case ESR_ELx_EC_BKPT32:
- el0_dbg(regs, esr);
+ el0_bkpt32(regs, esr);
break;
default:
el0_inv(regs, esr);
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 11/11] arm64: debug: remove debug exception registration infrastructure
2025-04-25 15:36 [PATCH 00/11] arm64: debug: remove hook registration, split exception entry Ada Couprie Diaz
` (9 preceding siblings ...)
2025-04-25 15:36 ` [PATCH 10/11] arm64: debug: split bkpt32 " Ada Couprie Diaz
@ 2025-04-25 15:36 ` Ada Couprie Diaz
10 siblings, 0 replies; 15+ messages in thread
From: Ada Couprie Diaz @ 2025-04-25 15:36 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Will Deacon, Catalin Marinas
Now that debug exceptions are handled individually and without the need
for dynamic registration, remove the unused registration infrastructure.
Remove `early_brk64` as it has been made redundant by
(arm64: debug: split brk64 exception entry) and is not used anymore.
Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
---
arch/arm64/include/asm/debug-monitors.h | 2 -
arch/arm64/include/asm/exception.h | 2 -
arch/arm64/include/asm/system_misc.h | 4 --
arch/arm64/kernel/debug-monitors.c | 3 -
arch/arm64/kernel/traps.c | 26 +--------
arch/arm64/mm/fault.c | 75 -------------------------
6 files changed, 1 insertion(+), 111 deletions(-)
diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index 9aabf65de693..fedcf78fb04d 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -94,7 +94,5 @@ static inline int reinstall_suspended_bps(struct pt_regs *regs)
int aarch32_break_handler(struct pt_regs *regs);
-void debug_traps_init(void);
-
#endif /* __ASSEMBLY */
#endif /* __ASM_DEBUG_MONITORS_H */
diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 4ca43d4eaf98..f7f772d75860 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -59,8 +59,6 @@ void do_el0_bti(struct pt_regs *regs);
void do_el1_bti(struct pt_regs *regs, unsigned long esr);
void do_el0_gcs(struct pt_regs *regs, unsigned long esr);
void do_el1_gcs(struct pt_regs *regs, unsigned long esr);
-void do_debug_exception(unsigned long addr_if_watchpoint, unsigned long esr,
- struct pt_regs *regs);
void do_breakpoint(unsigned long esr, struct pt_regs *regs);
void do_softstep(unsigned long esr, struct pt_regs *regs);
void do_watchpoint(unsigned long addr, unsigned long esr,
diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h
index c34344256762..344b1c1a4bbb 100644
--- a/arch/arm64/include/asm/system_misc.h
+++ b/arch/arm64/include/asm/system_misc.h
@@ -25,10 +25,6 @@ void arm64_notify_die(const char *str, struct pt_regs *regs,
int signo, int sicode, unsigned long far,
unsigned long err);
-void hook_debug_fault_code(int nr, int (*fn)(unsigned long, unsigned long,
- struct pt_regs *),
- int sig, int code, const char *name);
-
struct mm_struct;
extern void __show_regs(struct pt_regs *);
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index 7e4e85a212f4..bcbd9a3ff823 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -369,9 +369,6 @@ int aarch32_break_handler(struct pt_regs *regs)
}
NOKPROBE_SYMBOL(aarch32_break_handler);
-void __init debug_traps_init(void)
-{}
-
/* Re-enable single step for syscall restarting. */
void user_rewind_single_step(struct task_struct *task)
{
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 50d7a6a75f45..ce20f46e08cb 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -1101,29 +1101,5 @@ int ubsan_brk_handler(struct pt_regs *regs, unsigned long esr)
}
#endif
-/*
- * Initial handler for AArch64 BRK exceptions
- * This handler only used until debug_traps_init().
- */
-int __init early_brk64(unsigned long addr, unsigned long esr,
- struct pt_regs *regs)
-{
-#ifdef CONFIG_CFI_CLANG
- if (esr_is_cfi_brk(esr))
- return cfi_brk_handler(regs, esr) != DBG_HOOK_HANDLED;
-#endif
-#ifdef CONFIG_KASAN_SW_TAGS
- if ((esr_brk_comment(esr) & ~KASAN_BRK_MASK) == KASAN_BRK_IMM)
- return kasan_brk_handler(regs, esr) != DBG_HOOK_HANDLED;
-#endif
-#ifdef CONFIG_UBSAN_TRAP
- if ((esr_brk_comment(esr) & ~UBSAN_BRK_MASK) == UBSAN_BRK_IMM)
- return ubsan_brk_handler(regs, esr) != DBG_HOOK_HANDLED;
-#endif
- return bug_brk_handler(regs, esr) != DBG_HOOK_HANDLED;
-}
-
void __init trap_init(void)
-{
- debug_traps_init();
-}
+{}
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index ec0a337891dd..bc7f554dea9f 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -53,18 +53,12 @@ struct fault_info {
};
static const struct fault_info fault_info[];
-static struct fault_info debug_fault_info[];
static inline const struct fault_info *esr_to_fault_info(unsigned long esr)
{
return fault_info + (esr & ESR_ELx_FSC);
}
-static inline const struct fault_info *esr_to_debug_fault_info(unsigned long esr)
-{
- return debug_fault_info + DBG_ESR_EVT(esr);
-}
-
static void data_abort_decode(unsigned long esr)
{
unsigned long iss2 = ESR_ELx_ISS2(esr);
@@ -938,75 +932,6 @@ void do_sp_pc_abort(unsigned long addr, unsigned long esr, struct pt_regs *regs)
}
NOKPROBE_SYMBOL(do_sp_pc_abort);
-/*
- * __refdata because early_brk64 is __init, but the reference to it is
- * clobbered at arch_initcall time.
- * See traps.c and debug-monitors.c:debug_traps_init().
- */
-static struct fault_info __refdata debug_fault_info[] = {
- { do_bad, SIGTRAP, TRAP_HWBKPT, "hardware breakpoint" },
- { do_bad, SIGTRAP, TRAP_HWBKPT, "hardware single-step" },
- { do_bad, SIGTRAP, TRAP_HWBKPT, "hardware watchpoint" },
- { do_bad, SIGKILL, SI_KERNEL, "unknown 3" },
- { do_bad, SIGTRAP, TRAP_BRKPT, "aarch32 BKPT" },
- { do_bad, SIGKILL, SI_KERNEL, "aarch32 vector catch" },
- { early_brk64, SIGTRAP, TRAP_BRKPT, "aarch64 BRK" },
- { do_bad, SIGKILL, SI_KERNEL, "unknown 7" },
-};
-
-void __init hook_debug_fault_code(int nr,
- int (*fn)(unsigned long, unsigned long, struct pt_regs *),
- int sig, int code, const char *name)
-{
- BUG_ON(nr < 0 || nr >= ARRAY_SIZE(debug_fault_info));
-
- debug_fault_info[nr].fn = fn;
- debug_fault_info[nr].sig = sig;
- debug_fault_info[nr].code = code;
- debug_fault_info[nr].name = name;
-}
-
-/*
- * In debug exception context, we explicitly disable preemption despite
- * having interrupts disabled.
- * This serves two purposes: it makes it much less likely that we would
- * 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)
-{
- preempt_disable();
-
- /* This code is a bit fragile. Test it. */
- RCU_LOCKDEP_WARN(!rcu_is_watching(), "exception_enter didn't work");
-}
-NOKPROBE_SYMBOL(debug_exception_enter);
-
-static void debug_exception_exit(struct pt_regs *regs)
-{
- preempt_enable_no_resched();
-}
-NOKPROBE_SYMBOL(debug_exception_exit);
-
-void do_debug_exception(unsigned long addr_if_watchpoint, unsigned long esr,
- struct pt_regs *regs)
-{
- const struct fault_info *inf = esr_to_debug_fault_info(esr);
- unsigned long pc = instruction_pointer(regs);
-
- debug_exception_enter(regs);
-
- if (user_mode(regs) && !is_ttbr0_addr(pc))
- arm64_apply_bp_hardening();
-
- if (inf->fn(addr_if_watchpoint, esr, regs)) {
- arm64_notify_die(inf->name, regs, inf->sig, inf->code, pc, esr);
- }
-
- debug_exception_exit(regs);
-}
-NOKPROBE_SYMBOL(do_debug_exception);
-
/*
* Used during anonymous page fault handling.
*/
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 06/11] arm64: debug: split hardware breakpoint exeception entry
2025-04-25 15:36 ` [PATCH 06/11] arm64: debug: split hardware breakpoint exeception entry Ada Couprie Diaz
@ 2025-04-25 15:54 ` Ada Couprie Diaz
2025-05-01 11:15 ` Ada Couprie Diaz
1 sibling, 0 replies; 15+ messages in thread
From: Ada Couprie Diaz @ 2025-04-25 15:54 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Catalin Marinas, Mark Rutland, Will Deacon
On 25/04/2025 16:36, Ada Couprie Diaz wrote:
> [...]
>
> Duplicate the debug_exception_ entry and exit functions from mm/fault.c,
> as they are needed in the split paths for exceptions other than BKPT32.
> The original will be removed in a cleanup patch.
This is a mistake as those functions are now added to
`kernel/entry-common.c` in Patch 05, sorry.
This paragraph should not be present.
Thanks,
Ada
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 06/11] arm64: debug: split hardware breakpoint exeception entry
2025-04-25 15:36 ` [PATCH 06/11] arm64: debug: split hardware breakpoint exeception entry Ada Couprie Diaz
2025-04-25 15:54 ` Ada Couprie Diaz
@ 2025-05-01 11:15 ` Ada Couprie Diaz
1 sibling, 0 replies; 15+ messages in thread
From: Ada Couprie Diaz @ 2025-05-01 11:15 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Catalin Marinas, Will Deacon
On 25/04/2025 16:36, Ada Couprie Diaz wrote:
> +void do_breakpoint(unsigned long esr, struct pt_regs *regs)
> +{
> + unsigned long pc = regs->pc;
> +
> + if (user_mode(regs) && !is_ttbr0_addr(pc))
> + arm64_apply_bp_hardening();
After some thinking around the single stepping case, I will be moving
the `arm64_apply_bp_hardening()` call earlier, in `el0_breakpt()`,
similarly to what is done in `el0_pc()` ; likewise for the other
relevant patches in the series.
That way, it can be in `noinstr` code, allow enabling preemption in
`entry-common.c` for the debug exceptions it makes sense for, and
removes the always-false check from the EL1 path.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 07/11] arm64: debug: split single stepping exception entry
2025-04-25 15:36 ` [PATCH 07/11] arm64: debug: split single stepping exception entry Ada Couprie Diaz
@ 2025-05-01 11:36 ` Ada Couprie Diaz
0 siblings, 0 replies; 15+ messages in thread
From: Ada Couprie Diaz @ 2025-05-01 11:36 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Mark Rutland, Catalin Marinas, Will Deacon
On 25/04/2025 16:36, Ada Couprie Diaz wrote:
> +static void noinstr el0_softstp(struct pt_regs *regs, unsigned long esr)
> +{
> + enter_from_user_mode(regs);
> + debug_exception_enter(regs);
> + do_softstep(esr, regs);
I spent some time digging into the preemptibility of this, specifically
to be able to fix the reported issue with PREEMPT_RT [1].
From EL0, the only possible path for the single stepping exception is
to call `uprobe_singlestep_handler()`, which only operates on the
task-local uprobe state with proper checks in place and raises a Thread
Information Flag. The uprobe TIF is processed before returning to
userspace in `do_notify_resume()`, which is already preemptible. The
shared uprobe state contained in `current->active_uprobe` is created by
the preceding BRK and reference counted. It is only cleared when the
uprobe single stepping is processed.
Thus I feel it should be safe to enable preemption for this exception
from EL0 in v2, combined with my other comment moving the BP hardening
to `el0_softstp`. From testing, it seems to fix the reported issue with
PREEMPT_RT [1].
> + debug_exception_exit(regs);
> + local_daif_restore(DAIF_PROCCTX);
> + exit_to_user_mode(regs);
> +}
> +
[1]: https://lore.kernel.org/linux-arm-kernel/Z6YW_Kx4S2tmj2BP@uudg.org/
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2025-05-01 11:52 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-25 15:36 [PATCH 00/11] arm64: debug: remove hook registration, split exception entry Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 01/11] arm64: debug: Clean up single_step_handler logic Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 02/11] arm64: debug: call software break handlers statically Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 03/11] arm64: debug: call step " Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 04/11] arm64: debug: remove break/step handler registration infrastructure Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 05/11] arm64: entry: Add entry and exit functions for debug exceptions Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 06/11] arm64: debug: split hardware breakpoint exeception entry Ada Couprie Diaz
2025-04-25 15:54 ` Ada Couprie Diaz
2025-05-01 11:15 ` Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 07/11] arm64: debug: split single stepping exception entry Ada Couprie Diaz
2025-05-01 11:36 ` Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 08/11] arm64: debug: split hardware watchpoint " Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 09/11] arm64: debug: split brk64 " Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 10/11] arm64: debug: split bkpt32 " Ada Couprie Diaz
2025-04-25 15:36 ` [PATCH 11/11] arm64: debug: remove debug exception registration infrastructure Ada Couprie Diaz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).