Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753080AbbKICl5 (ORCPT ); Sun, 8 Nov 2015 21:41:57 -0500 Received: from mail-wm0-f42.google.com ([74.125.82.42]:36255 "EHLO mail-wm0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752769AbbKIClx (ORCPT ); Sun, 8 Nov 2015 21:41:53 -0500 MIME-Version: 1.0 Date: Sun, 8 Nov 2015 19:41:51 -0700 Message-ID: Subject: [PATCH 1/1] fix system hang in v4.3 in hw_breakpoint.c From: Jeffrey Merkey To: linux-kernel Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6544 Lines: 167 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 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 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@gmail.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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/