[PATCH] kmemcheck: move hook before preempt_conditional_sti()

* [PATCH] kmemcheck: move hook before preempt_conditional_sti()
@ 2009-06-07 16:26 Vegard Nossum
  2009-06-07 16:45 ` Ingo Molnar
  2009-06-08  6:41 ` Ananth N Mavinakayanahalli
  0 siblings, 2 replies; 6+ messages in thread
From: Vegard Nossum @ 2009-06-07 16:26 UTC (permalink / raw)
  Cc: linux-kernel, Vegard Nossum, Alexander van Heukelum, K.Prasad,
	Alan Stern, Frederic Weisbecker, Pekka Enberg

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 <heukelum@fastmail.fm>
Date:   Tue Sep 30 18:41:37 2008 +0200

    x86, traps: converge do_debug handlers

commit 08d68323d1f0c34452e614263b212ca556dae47f
Author: K.Prasad <prasad@linux.vnet.ibm.com>
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 <vegard.nossum@gmail.com>
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 <mingo@elte.hu>
Cc: Alexander van Heukelum <heukelum@fastmail.fm>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>
Signed-off-by: Vegard Nossum <vegard.nossum@gmail.com>
---
 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.
-- 
1.6.0.6


^ permalink raw reply related	[flat|nested] 6+ messages in thread