linux-kernel - [PATCH] kmemcheck: move hook before preempt_conditional

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1244392012-638-1-git-send-email-vegard.nossum@gmail.com>
Date:	Sun,  7 Jun 2009 18:26:52 +0200
From:	Vegard Nossum <vegard.nossum@...il.com>
To:	unlisted-recipients:; (no To-header on input)
Cc:	linux-kernel@...r.kernel.org,
	Vegard Nossum <vegard.nossum@...il.com>,
	Alexander van Heukelum <heukelum@...tmail.fm>,
	"K.Prasad" <prasad@...ux.vnet.ibm.com>,
	Alan Stern <stern@...land.harvard.edu>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Pekka Enberg <penberg@...helsinki.fi>
Subject: [PATCH] kmemcheck: move hook before preempt_conditional_sti()

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

    x86, traps: converge do_debug handlers

commit 08d68323d1f0c34452e614263b212ca556dae47f
Author: K.Prasad <prasad@...ux.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@...il.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@...e.hu>
Cc: Alexander van Heukelum <heukelum@...tmail.fm>
Cc: K.Prasad <prasad@...ux.vnet.ibm.com>
Cc: Alan Stern <stern@...land.harvard.edu>
Cc: Frederic Weisbecker <fweisbec@...il.com>
Cc: Pekka Enberg <penberg@...helsinki.fi>
Signed-off-by: Vegard Nossum <vegard.nossum@...il.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

--
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/