[PATCH 0/2] arm64/debug: clean up some HW BP edge cases

[PATCH 0/2] arm64/debug: clean up some HW BP edge cases

[Ada Couprie Diaz]

Hi,

This is a small series focused on fixing some unsoundess and edge case
related to hardware breakpoints and watchpoints.

It is motivated in part by the recent discussion on the
FEAT_Debugv8p6 series[0][1] regarding unclear guarantees on interruptions.

It doesn't address the issue of instrumentation, as NO_KPROBE is not
sufficient to prevent it (specifically ftrace) and would require a pass
over the whole debug handling code.

The bugs themselves are very much of the self-inflicted
"you should not be doing this" kind and not urgent, but it does solve
some of the questions on the FEAT_Dbugv8p6 series.

Most of the raw additions are comments trying to give more context to
understand `hw_breakpoint_thread_switch()`.

Thanks in advance !
Ada

Based on v7.1-rc5

[0]: https://lore.kernel.org/linux-arm-kernel/20241216040831.2448257-1-anshuman.khandual@arm.com/
[1]: https://lore.kernel.org/linux-arm-kernel/adeE4MD0RgapI8aL@J2N7QTR9R3/

Ada Couprie Diaz (2):
  arm64/debug: update perf slots in sync with BP registers
  arm64/debug: mask exceptions when switching cpu-bound watchpoints

 arch/arm64/kernel/hw_breakpoint.c | 52 +++++++++++++++++++++----------
 1 file changed, 36 insertions(+), 16 deletions(-)


base-commit: e7ae89a0c97ce2b68b0983cd01eda67cf373517d
-- 
2.43.0

[PATCH 1/2] arm64/debug: update perf slots in sync with BP registers

[Ada Couprie Diaz]

The current order of operations in `hw_breakpoint_control()` is unsound
and can lead to the kernel getting stuck on a hardware breakpoint in
some edge cases[0], as it can be called with debug exceptions unmasked.

As we search a relevant perf slot and update depending on the operation
directly in `hw_breakpoint_setup_slot()`, when uninstalling a breakpoint
it can still be triggered while its slot has been cleared, preventing
the debug handlers from handling it properly and getting stuck on it.

Hoist the perf slot updates out of `hw_breakpoint_setup_slot()` into
`hw_breakpoint_control()` : set up the slot before writing to the register
when installing (no change) but clear it after writing to the register
when uninstalling.
Rename `hw_breakpoint_setup_slot()` to `hw_breakpoint_find_slot()`, as
the slot setup is now done directly in `hw_breakpoint_control()`.

[0]: https://lore.kernel.org/linux-arm-kernel/adeE4MD0RgapI8aL@J2N7QTR9R3/

Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
---
 arch/arm64/kernel/hw_breakpoint.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index ab76b36dce82..ce99a00c8596 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c
@@ -174,12 +174,11 @@ static int is_compat_bp(struct perf_event *bp)
 }
 
 /**
- * hw_breakpoint_slot_setup - Find and setup a perf slot according to
- *			      operations
+ * hw_breakpoint_find_slot - Find a perf slot according to operation
  *
  * @slots: pointer to array of slots
  * @max_slots: max number of slots
- * @bp: perf_event to setup
+ * @bp: perf_event to find, for uninstall and restore operations
  * @ops: operation to be carried out on the slot
  *
  * Return:
@@ -187,30 +186,26 @@ static int is_compat_bp(struct perf_event *bp)
  *	-ENOSPC if no slot is available/matches
  *	-EINVAL on wrong operations parameter
  */
-static int hw_breakpoint_slot_setup(struct perf_event **slots, int max_slots,
+static int hw_breakpoint_find_slot(struct perf_event **slots, int max_slots,
 				    struct perf_event *bp,
 				    enum hw_breakpoint_ops ops)
 {
 	int i;
-	struct perf_event **slot;
+	struct perf_event *slot;
 
 	for (i = 0; i < max_slots; ++i) {
-		slot = &slots[i];
+		slot = slots[i];
 		switch (ops) {
 		case HW_BREAKPOINT_INSTALL:
-			if (!*slot) {
-				*slot = bp;
+			if (!slot)
 				return i;
-			}
 			break;
 		case HW_BREAKPOINT_UNINSTALL:
-			if (*slot == bp) {
-				*slot = NULL;
+			if (slot == bp)
 				return i;
-			}
 			break;
 		case HW_BREAKPOINT_RESTORE:
-			if (*slot == bp)
+			if (slot == bp)
 				return i;
 			break;
 		default:
@@ -247,13 +242,15 @@ static int hw_breakpoint_control(struct perf_event *bp,
 		reg_enable = !debug_info->wps_disabled;
 	}
 
-	i = hw_breakpoint_slot_setup(slots, max_slots, bp, ops);
+	i = hw_breakpoint_find_slot(slots, max_slots, bp, ops);
 
 	if (WARN_ONCE(i < 0, "Can't find any breakpoint slot"))
 		return i;
 
 	switch (ops) {
 	case HW_BREAKPOINT_INSTALL:
+		slots[i] = bp;
+		barrier();
 		/*
 		 * Ensure debug monitors are enabled at the correct exception
 		 * level.
@@ -278,6 +275,9 @@ static int hw_breakpoint_control(struct perf_event *bp,
 		 * level.
 		 */
 		disable_debug_monitors(dbg_el);
+
+		barrier();
+		slots[i] = NULL;
 		break;
 	}
 
-- 
2.43.0

[PATCH 2/2] arm64/debug: mask exceptions when switching cpu-bound watchpoints

[Ada Couprie Diaz]

When hit, EL1 watchpoints always disable EL0 watchpoints to avoid
the kernel triggering them during uaccess, and re-enable them depending
on the current task's `wps_disabled`.

This can lead to an inconsistent state when task switching between two
tasks with different `wps_disabled` and CPU-bound watchpoints :
`hw_breakpoint_thread_switch()` is called without debug exceptions masked
and could raise a watchpoint exception at EL1 while toggling the CPU-bound
watchpoints.
As the watchpoint and single step handlers are not aware of the switch
nor of any "next task", this can lead to an inconsistent enabled/disabled
state for CPU-bound hardware watchpoints.

In the worst case, some CPU-bound watchpoint will not trigger
when expected until the next time they are enabled (similar task switch,
single step after another watchpoint).
It can also lead to a watchpoint triggering while we were stepping another
watchpoint, but as far as I can tell this is covered by the existing
behaviour of the watchpoint and single step handlers.

Mask all DAIF exceptions if we are toggling CPU-bound watchpoint
during thread-switch to avoid this edge case.

Add some context on what breakpoints are switched in this function
and some reasoning as to why we are masking here.

Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
---
 arch/arm64/kernel/hw_breakpoint.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index ce99a00c8596..ce4f58f19bf4 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c
@@ -20,6 +20,7 @@
 #include <linux/uaccess.h>
 
 #include <asm/current.h>
+#include <asm/daifflags.h>
 #include <asm/debug-monitors.h>
 #include <asm/esr.h>
 #include <asm/exception.h>
@@ -899,7 +900,12 @@ bool try_step_suspended_breakpoints(struct pt_regs *regs)
 NOKPROBE_SYMBOL(try_step_suspended_breakpoints);
 
 /*
- * Context-switcher for restoring suspended breakpoints.
+ * Context-switcher for restoring suspended breakpoints
+ * which are not task-bound.
+ *
+ * Breakpoints of the previous task are uninstalled before
+ * this function is called, in perf_event_task_sched_out(), and those
+ * of the next task are installed after, in perf_event_task_sched_in().
  */
 void hw_breakpoint_thread_switch(struct task_struct *next)
 {
@@ -924,10 +930,24 @@ void hw_breakpoint_thread_switch(struct task_struct *next)
 				    !next_debug_info->bps_disabled);
 
 	/* Update watchpoints. */
-	if (current_debug_info->wps_disabled != next_debug_info->wps_disabled)
+	if (current_debug_info->wps_disabled != next_debug_info->wps_disabled) {
+		unsigned long mask;
+		/*
+		 * This is called with local_irqs masked, but not DAIF.D.
+		 * EL1 watchpoints always toggle EL0 watchpoints when hit,
+		 * re-enabling them depending on the _current_ task.
+		 *
+		 * Mask all exceptions to avoid creating an inconsistent state
+		 * by triggering a watchpoint mid-way through the switch.
+		 */
+		mask = local_daif_save();
+
 		toggle_bp_registers(AARCH64_DBG_REG_WCR,
 				    DBG_ACTIVE_EL0,
 				    !next_debug_info->wps_disabled);
+
+		local_daif_restore(mask);
+	}
 }
 
 /*
-- 
2.43.0

Re: [PATCH 0/2] arm64/debug: clean up some HW BP edge cases

[Will Deacon]

On Wed, May 27, 2026 at 05:15:51PM +0100, Ada Couprie Diaz wrote:
> This is a small series focused on fixing some unsoundess and edge case
> related to hardware breakpoints and watchpoints.
> 
> It is motivated in part by the recent discussion on the
> FEAT_Debugv8p6 series[0][1] regarding unclear guarantees on interruptions.
> 
> It doesn't address the issue of instrumentation, as NO_KPROBE is not
> sufficient to prevent it (specifically ftrace) and would require a pass
> over the whole debug handling code.
> 
> The bugs themselves are very much of the self-inflicted
> "you should not be doing this" kind and not urgent, but it does solve
> some of the questions on the FEAT_Dbugv8p6 series.
> 
> Most of the raw additions are comments trying to give more context to
> understand `hw_breakpoint_thread_switch()`.

Hmm. From what I can tell, these patches are trying to handle various
cases where we take a debug exception while executing the hw_breakpoint
code itself. Even with these two fixes, is perf expecting to handle that
sort of thing? I think it would be a lot more robust if we just prevented
this from happening in the first place, similarly to the discussion from
the other day around kprobes [2].

Will

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

Re: [PATCH 0/2] arm64/debug: clean up some HW BP edge cases

[Mark Rutland]

On Wed, Jul 22, 2026 at 12:30:34PM +0100, Will Deacon wrote:
> On Wed, May 27, 2026 at 05:15:51PM +0100, Ada Couprie Diaz wrote:
> > This is a small series focused on fixing some unsoundess and edge case
> > related to hardware breakpoints and watchpoints.
> > 
> > It is motivated in part by the recent discussion on the
> > FEAT_Debugv8p6 series[0][1] regarding unclear guarantees on interruptions.
> > 
> > It doesn't address the issue of instrumentation, as NO_KPROBE is not
> > sufficient to prevent it (specifically ftrace) and would require a pass
> > over the whole debug handling code.
> > 
> > The bugs themselves are very much of the self-inflicted
> > "you should not be doing this" kind and not urgent, but it does solve
> > some of the questions on the FEAT_Dbugv8p6 series.
> > 
> > Most of the raw additions are comments trying to give more context to
> > understand `hw_breakpoint_thread_switch()`.
> 
> Hmm. From what I can tell, these patches are trying to handle various
> cases where we take a debug exception while executing the hw_breakpoint
> code itself. Even with these two fixes, is perf expecting to handle that
> sort of thing? I think it would be a lot more robust if we just prevented
> this from happening in the first place, similarly to the discussion from
> the other day around kprobes [2].

Since [2] mentions noinstr, it's worth noting that noinstr alone isn't
sufficient. Watchpoints also use this path, and we can't practically
filter watchpoint addresses to avoid hitting a watchpoint here.

I assume you'd be happy with masking debug exceptions entirely?

Mark.

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