Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755936AbZFGRw3 (ORCPT ); Sun, 7 Jun 2009 13:52:29 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753138AbZFGRwV (ORCPT ); Sun, 7 Jun 2009 13:52:21 -0400 Received: from mx3.mail.elte.hu ([157.181.1.138]:44077 "EHLO mx3.mail.elte.hu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752695AbZFGRwU (ORCPT ); Sun, 7 Jun 2009 13:52:20 -0400 Date: Sun, 7 Jun 2009 19:52:11 +0200 From: Ingo Molnar To: Vegard Nossum Cc: linux-kernel@vger.kernel.org, Alexander van Heukelum , "K.Prasad" , Alan Stern , Frederic Weisbecker , Pekka Enberg Subject: Re: [PATCH] kmemcheck: move hook before preempt_conditional_sti() Message-ID: <20090607175211.GB9997@elte.hu> References: <1244392012-638-1-git-send-email-vegard.nossum@gmail.com> <20090607164520.GA20672@elte.hu> <19f34abd0906071025g46529b30v41c70b0afc0be030@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <19f34abd0906071025g46529b30v41c70b0afc0be030@mail.gmail.com> User-Agent: Mutt/1.5.18 (2008-05-17) X-ELTE-SpamScore: -1.5 X-ELTE-SpamLevel: X-ELTE-SpamCheck: no X-ELTE-SpamVersion: ELTE 2.0 X-ELTE-SpamCheck-Details: score=-1.5 required=5.9 tests=BAYES_00 autolearn=no SpamAssassin version=3.2.5 -1.5 BAYES_00 BODY: Bayesian spam probability is 0 to 1% [score: 0.0000] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3798 Lines: 106 * Vegard Nossum wrote: > 2009/6/7 Ingo Molnar : > > > > * Vegard Nossum wrote: > > > >> There are actually two problems here: > >> > >> 1. We absolutely cannot enable IRQs in case the fault was caused by > >> ? ?kmemcheck. > >> > >> 2. We cannot enable preemption and then return from the debug handler > >> ? ?without disabling preemption afterwards. > >> > >> The problem seems to be a merge fallout between three commits: > >> > >> commit 3d2a71a596bd9c761c8487a2178e95f8a61da083 > >> Author: Alexander van Heukelum > >> Date: ? Tue Sep 30 18:41:37 2008 +0200 > >> > >> ? ? x86, traps: converge do_debug handlers > >> > >> commit 08d68323d1f0c34452e614263b212ca556dae47f > >> Author: K.Prasad > >> Date: ? Mon Jun 1 23:44:08 2009 +0530 > >> > >> ? ? hw-breakpoints: modifying generic debug exception to use thread-specific deb > >> > >> commit 787ecfaa503dc63ff1831ddc74b15dad49bace1d > >> Author: Vegard Nossum > >> Date: ? Fri Apr 4 00:53:23 2008 +0200 > >> > >> ? ? x86: add hooks for kmemcheck > >> > >> I encourage the kprobe developers to check whether their code is correct > >> as it stands in current tip/master. Also, comments on this particular > >> change is welcome. > >> > >> Reported-by: Ingo Molnar > >> Cc: Alexander van Heukelum > >> Cc: K.Prasad > >> Cc: Alan Stern > >> Cc: Frederic Weisbecker > >> Cc: Pekka Enberg > >> Signed-off-by: Vegard Nossum > >> --- > >> ?arch/x86/kernel/traps.c | ? ?8 ++++---- > >> ?1 files changed, 4 insertions(+), 4 deletions(-) > > > >> > >> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c > >> index c8a7f87..a898c6b 100644 > >> --- a/arch/x86/kernel/traps.c > >> +++ b/arch/x86/kernel/traps.c > >> @@ -550,6 +550,10 @@ dotraplinkage void __kprobes do_debug(struct pt_regs *regs, long error_code) > >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? SIGTRAP) == NOTIFY_STOP) > >> ? ? ? ? ? ? ? return; > >> > >> + ? ? /* Catch kmemcheck conditions first of all! */ > >> + ? ? if ((dr6 & DR_STEP) && kmemcheck_trap(regs)) > >> + ? ? ? ? ? ? return; > >> + > >> ? ? ? /* It's safe to allow irq's after DR6 has been saved */ > >> ? ? ? preempt_conditional_sti(regs); > >> > >> @@ -559,10 +563,6 @@ dotraplinkage void __kprobes do_debug(struct pt_regs *regs, long error_code) > >> ? ? ? ? ? ? ? return; > >> ? ? ? } > >> > >> - ? ? /* Catch kmemcheck conditions first of all! */ > >> - ? ? if ((dr6 & DR_STEP) && kmemcheck_trap(regs)) > >> - ? ? ? ? ? ? return; > >> - > >> ? ? ? /* > >> ? ? ? ?* Single-stepping through system calls: ignore any exceptions in > >> ? ? ? ?* kernel space, but re-enable TF when returning to user mode. > > > > Yeah - this could solve the crash i saw. Mind sending a pull request > > too? > > It did solve it, I tested it :-D > > This patch was against tip/master; tip/kmemcheck does not have the > problem. I think it might have been introduced in this merge: > > commit 85b9b2801e46a147330b8a0f321bc40342ff5b4c > Merge: bf8d9b3... 7387400... > Author: Ingo Molnar > Date: Thu Jun 4 13:56:43 2009 +0200 > > Merge branch 'tracing/hw-breakpoints' > > Conflicts: > arch/x86/Kconfig > arch/x86/kernel/traps.c > kernel/Makefile Ah. kmemcheck + hw-breakpoints interaction. Fun. Ingo -- 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/