From mboxrd@z Thu Jan 1 00:00:00 1970 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 * 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