Message-ID: <CA+ekxPUv_cDSDCp4z1L7Hm2bm4h+GNh-kXj7b_LUX7Y0SGcTQw@mail.gmail.com>
Date:	Fri, 13 Nov 2015 00:05:25 -0700
From:	Jeffrey Merkey <jeffmerkey@...il.com>
To:	linux-kernel <linux-kernel@...r.kernel.org>
Subject: [PATCH 1/1] fix system hang in v4.3 in hw_breakpoint.c RESEND

Disregard the previous patch.  This version has been tested and is correct.

Previous fix was:

If another debugger or int1 handler is registered, and triggers an int
1 exception, the current code fails to properly detect that an execute
breakpoint has occurred, and does not set the resume flag, causing the
system to hang with a recursive int exception for any breakpoint not
defined by an end user in the arch specific hardware breakpoint
setting code.  This is not robust as the system should query the dr7
register for information about whether or not the breakpoint was an
execution breakpoint triggered by the processor.  The current code
does not handle the error correctly and will crash the system.   The
system should not depend on user supplied input as to what type of
breakpoint has been triggered, particularly not when the information
exists in the dr7 register.

Signed-off-by:  Jeff Merkey (jeffmerkey@...il.com)
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 50a3fad..079b513 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -438,12 +438,14 @@ EXPORT_SYMBOL_GPL(hw_breakpoint_restore);
  * as BD, BS or BT) indicating that more than one debug condition is
  * met and requires some more action in do_debug().
  *
- * NOTIFY_STOP returned for all other cases
+	* NOTIFY_DONE returned for all other cases in the event another
+	* do_debug int1 handler is active.
  *
  */
+
 static int hw_breakpoint_handler(struct die_args *args)
 {
-	int i, cpu, rc = NOTIFY_STOP;
+	int i, cpu, rc = NOTIFY_DONE;
 	struct perf_event *bp;
 	unsigned long dr7, dr6;
 	unsigned long *dr6_p;
@@ -463,6 +465,7 @@ static int hw_breakpoint_handler(struct die_args *args)
 	get_debugreg(dr7, 7);
 	/* Disable breakpoints during exception handling */
 	set_debugreg(0UL, 7);
+
 	/*
 	 * Assert that local interrupts are disabled
 	 * Reset the DRn bits in the virtualized register value.
@@ -475,45 +478,56 @@ static int hw_breakpoint_handler(struct die_args *args)
 	for (i = 0; i < HBP_NUM; ++i) {
 		if (likely(!(dr6 & (DR_TRAP0 << i))))
 			continue;
+		/*
+		 * Set up resume flag to avoid breakpoint recursion when
+		 * returning back to origin.   check dr7 and do not depend
+		 * on user input as to what type of breakpoint we are
+		 * dealing with.  if a breakpoint register is set or
+		 * triggered by another do_debug int1 handler, and we
+		 * depend on user input, the system will hang in a
+		 * recursive int1 exception state if the resume flag
+		 * is not set.  if execute breakpoint, set the resume
+		 * flag.
+		 */
+		if (!(dr7 & (3 << ((i * 4) + 16))))
+			args->regs->flags |= X86_EFLAGS_RF;

 		/*
-		 * The counter may be concurrently released but that can only
-		 * occur from a call_rcu() path. We can then safely fetch
-		 * the breakpoint, use its callback, touch its counter
-		 * while we are in an rcu_read_lock() path.
+		 * The counter may be concurrently released but that can
+		 * only occur from a call_rcu() path. We can then safely
+		 * fetch the breakpoint, use its callback, touch its
+		 * counter while we are in an rcu_read_lock() path.
 		 */
 		rcu_read_lock();

 		bp = per_cpu(bp_per_reg[i], cpu);
-		/*
-		 * Reset the 'i'th TRAP bit in dr6 to denote completion of
-		 * exception handling
-		 */
-		(*dr6_p) &= ~(DR_TRAP0 << i);
+
 		/*
 		 * bp can be NULL due to lazy debug register switching
 		 * or due to concurrent perf counter removing.
 		 */
 		if (!bp) {
 			rcu_read_unlock();
-			break;
+			continue;
 		}

-		perf_bp_event(bp, args->regs);
-
 		/*
-		 * Set up resume flag to avoid breakpoint recursion when
-		 * returning back to origin.
+		 * Reset the 'i'th TRAP bit in dr6 to denote completion of
+		 * exception handling.  don't clear dr6 status if
+		 * another do_debug int1 handler is active unless its
+		 * really one of our breakpoints.
 		 */
-		if (bp->hw.info.type == X86_BREAKPOINT_EXECUTE)
-			args->regs->flags |= X86_EFLAGS_RF;
+		(*dr6_p) &= ~(DR_TRAP0 << i);
+
+		perf_bp_event(bp, args->regs);

 		rcu_read_unlock();
 	}
 	/*
 	 * Further processing in do_debug() is needed for a) user-space
 	 * breakpoints (to generate signals) and b) when the system has
-	 * taken exception due to multiple causes
+	 * taken exception due to multiple causes and c) other do_debug
+	 * int handlers.
 	 */
 	if ((current->thread.debugreg6 & DR_TRAP_BITS) ||
 	    (dr6 & (~DR_TRAP_BITS)))
@@ -523,6 +537,7 @@ static int hw_breakpoint_handler(struct die_args *args)
 	put_cpu();

 	return rc;
+
 }

 /*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists