[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 at 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