[PATCH] x86/debug: Avoid exc_debug_kernel() warning after user data watchpoints

[Zhao Gongyi]

A user-address data watchpoint can fire while the kernel accesses user
memory. If DR6 also reports DR_STEP, hw_breakpoint_handler() consumes
the watchpoint but passes DR_STEP to exc_debug_kernel(). There,
WARN_ON_ONCE(dr6 & DR_STEP) triggers even though the interrupted
frame has TF clear.

On the reproducing QEMU/KVM host, an earlier single-step event left BS
in KVM's saved guest DR6; a later data-watchpoint exception appeared
to the guest as B0|BS. Consume the residual DR_STEP only after the
watchpoint has been handled, no breakpoint bits remain, and TF, ptrace
and KGDB stepping are inactive.

Signed-off-by: Zhao Gongyi <zhaogongyi@xxxxxxxxxxxxx>
---

Testing:
With panic_on_warn=1, the baseline warned in all three QEMU/KVM trials;
the patched kernel showed no target warning in three trials. A focused
KGDB single-step/watchpoint test preserved DR_STEP when TF was set.

 arch/x86/kernel/hw_breakpoint.c | 41 ++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index f846c15f21ca1..0b6131aa05969 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -26,6 +26,7 @@
 #include <linux/kernel.h>
 #include <linux/kvm_types.h>
 #include <linux/export.h>
+#include <linux/kgdb.h>
 #include <linux/sched.h>
 #include <linux/smp.h>
 
@@ -49,6 +50,19 @@ static DEFINE_PER_CPU(unsigned long, cpu_debugreg[HBP_NUM]);
  */
 static DEFINE_PER_CPU(struct perf_event *, bp_per_reg[HBP_NUM]);
 
+static inline bool kgdb_single_step_active(void)
+{
+#ifdef CONFIG_KGDB
+	return atomic_read(&kgdb_cpu_doing_single_step) != -1;
+#else
+	return false;
+#endif
+}
+
+static inline bool hw_breakpoint_is_user_data(struct perf_event *bp)
+{
+	return !arch_check_bp_in_kernelspace(counter_arch_bp(bp));
+}
 
 static inline unsigned long
 __encode_dr7(int drnum, unsigned int len, unsigned int type)
@@ -514,7 +528,7 @@ static int hw_breakpoint_handler(struct die_args *args)
 	struct perf_event *bp;
 	unsigned long *dr6_p;
 	unsigned long dr6;
-	bool bpx;
+	bool bpx, handled_kernel_user_data_bp = false;
 
 	/* The DR6 value is pointed by args->err */
 	dr6_p = (unsigned long *)ERR_PTR(args->err);
@@ -561,6 +575,8 @@ static int hw_breakpoint_handler(struct die_args *args)
 		 */
 		if (bpx)
 			args->regs->flags |= X86_EFLAGS_RF;
+		else if (!user_mode(args->regs) && hw_breakpoint_is_user_data(bp))
+			handled_kernel_user_data_bp = true;
 	}
 
 	/*
@@ -568,6 +584,29 @@ static int hw_breakpoint_handler(struct die_args *args)
 	 * breakpoints (to generate signals) and b) when the system has
 	 * taken exception due to multiple causes
 	 */
+	if ((current->thread.virtual_dr6 & DR_TRAP_BITS) ||
+	    (dr6 & ~DR_TRAP_BITS)) {
+		/*
+		 * User-space data breakpoints are traps, and the CPU can report
+		 * them together with DR_STEP while the kernel is accessing user
+		 * memory. Once the user data watchpoint has been consumed above,
+		 * there is no kernel TF state to report if the interrupted frame
+		 * did not have TF set and neither ptrace nor KGDB requested a
+		 * single-step. Clear that residual DR_STEP before deciding
+		 * whether the generic debug path still needs to see it.
+		 */
+		if (handled_kernel_user_data_bp &&
+		    !(*dr6_p & DR_TRAP_BITS) &&
+		    (*dr6_p & DR_STEP) &&
+		    !(args->regs->flags & X86_EFLAGS_TF) &&
+		    !test_thread_flag(TIF_SINGLESTEP) &&
+		    !test_thread_flag(TIF_BLOCKSTEP) &&
+		    !kgdb_single_step_active())
+			*dr6_p &= ~DR_STEP;
+
+		dr6 = *dr6_p;
+	}
+
 	if ((current->thread.virtual_dr6 & DR_TRAP_BITS) ||
 	    (dr6 & (~DR_TRAP_BITS)))
 		rc = NOTIFY_DONE;

base-commit: 9d66f26fefacebfeed15de31c1123451a17ff119
-- 
2.39.5 (Apple Git-154)