Message-ID: <CA+ekxPVRrzwPAEeM0zfrsNWe+oC4_hX1i9Jy0sVr67TjGz6h7w@mail.gmail.com>
Date:	Sun, 8 Nov 2015 19:41:51 -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

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.

diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 50a3fad..f3ccb38 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -428,22 +428,26 @@ EXPORT_SYMBOL_GPL(hw_breakpoint_restore);
 /*
  * Handle debug exception notifications.
  *
- * Return value is either NOTIFY_STOP or NOTIFY_DONE as explained below.
+ * Return value is either NOTIFY_STOP or NOTIFY_DONE as
+ * explained below.
  *
- * NOTIFY_DONE returned if one of the following conditions is true.
- * i) When the causative address is from user-space and the exception
- * is a valid one, i.e. not triggered as a result of lazy debug register
- * switching
- * ii) When there are more bits than trap<n> set in DR6 register (such
- * as BD, BS or BT) indicating that more than one debug condition is
- * met and requires some more action in do_debug().
+ * NOTIFY_DONE returned if one of the following conditions
+ * is true.
+ * i) When the causative address is from user-space and the
+ * exception is a valid one, i.e. not triggered as a result of
+ * lazy debug register switching
+ * ii) When there are more bits than trap<n> set in DR6 register
+ * (such 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 +467,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,6 +480,19 @@ 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-1) * 4) + 16))))
+			args->regs->flags |= X86_EFLAGS_RF;

 		/*
 		 * The counter may be concurrently released but that can only
@@ -485,35 +503,33 @@ static int hw_breakpoint_handler(struct die_args *args)
 		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 +539,7 @@ static int hw_breakpoint_handler(struct die_args *args)
 	put_cpu();

 	return rc;
+
 }

 /*
Signed-off-by: Jeff Merkey (jeffmerkey@...il.com)

By making a contribution to this project, I certify that:

        (a) The contribution was created in whole or in part by me and I
            have the right to submit it under the open source license
            indicated in the file; or

        (b) The contribution is based upon previous work that, to the best
            of my knowledge, is covered under an appropriate open source
            license and I have the right under that license to submit that
            work with modifications, whether created in whole or in part
            by me, under the same open source license (unless I am
            permitted to submit under a different license), as indicated
            in the file; or

        (c) The contribution was provided directly to me by some other
            person who certified (a), (b) or (c) and I have not modified
            it.

        (d) I understand and agree that this project and the contribution
            are public and that a record of the contribution (including all
            personal information I submit with it, including my sign-off) is
            maintained indefinitely and may be redistributed consistent with
            this project or the open source license(s) involved.
--
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