LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andi Kleen <ak@suse.de>
To: Zachary Amsden <zach@vmware.com>,
	patches@x86-64.org, linux-kernel@vger.kernel.org
Subject: [PATCH 2.6.21 review II] [7/10] VMI: Profile pc badness
Date: Sat, 10 Feb 2007 12:52:35 +0100 (CET)	[thread overview]
Message-ID: <20070210115235.6F12313DFE@wotan.suse.de> (raw)
In-Reply-To: <200702101252.033732000@suse.de>


From: Zachary Amsden <zach@vmware.com>
Profile_pc was broken when using paravirtualization because the
assumption the kernel was running at CPL 0 was violated, causing
bad logic to read a random value off the stack.

The only way to be in kernel lock functions is to be in kernel
code, so validate that assumption explicitly by checking the CS
value.  We don't want to be fooled by BIOS / APM segments and
try to read those stacks, so only match KERNEL_CS.

I moved some stuff in segment.h to make it prettier.

Signed-off-by: Zachary Amsden <zach@vmware.com>

diff -r 69d0339b9997 arch/i386/kernel/time.c
--- a/arch/i386/kernel/time.c	Fri Feb 02 15:55:46 2007 -0800
+++ b/arch/i386/kernel/time.c	Fri Feb 02 16:15:45 2007 -0800
@@ -131,15 +131,13 @@ unsigned long profile_pc(struct pt_regs 
 	unsigned long pc = instruction_pointer(regs);
 
 #ifdef CONFIG_SMP
-	if (!user_mode_vm(regs) && in_lock_functions(pc)) {
+	if (!v8086_mode(regs) && SEGMENT_IS_KERNEL_CODE(regs->xcs) &&
+	    in_lock_functions(pc)) {
 #ifdef CONFIG_FRAME_POINTER
 		return *(unsigned long *)(regs->ebp + 4);
 #else
-		unsigned long *sp;
-		if ((regs->xcs & 3) == 0)
-			sp = (unsigned long *)&regs->esp;
-		else
-			sp = (unsigned long *)regs->esp;
+		unsigned long *sp = (unsigned long *)&regs->esp;
+
 		/* Return address is either directly at stack pointer
 		   or above a saved eflags. Eflags has bits 22-31 zero,
 		   kernel addresses don't. */
diff -r 69d0339b9997 include/asm-i386/ptrace.h
--- a/include/asm-i386/ptrace.h	Fri Feb 02 15:55:46 2007 -0800
+++ b/include/asm-i386/ptrace.h	Fri Feb 02 16:12:37 2007 -0800
@@ -49,6 +49,10 @@ static inline int user_mode_vm(struct pt
 {
 	return ((regs->xcs & SEGMENT_RPL_MASK) | (regs->eflags & VM_MASK)) >= USER_RPL;
 }
+static inline int v8086_mode(struct pt_regs *regs)
+{
+	return (regs->eflags & VM_MASK);
+}
 
 #define instruction_pointer(regs) ((regs)->eip)
 #define regs_return_value(regs) ((regs)->eax)
diff -r 69d0339b9997 include/asm-i386/segment.h
--- a/include/asm-i386/segment.h	Fri Feb 02 15:55:46 2007 -0800
+++ b/include/asm-i386/segment.h	Fri Feb 02 16:08:50 2007 -0800
@@ -83,13 +83,7 @@
  * The GDT has 32 entries
  */
 #define GDT_ENTRIES 32
-
 #define GDT_SIZE (GDT_ENTRIES * 8)
-
-/* Matches __KERNEL_CS and __USER_CS (they must be 2 entries apart) */
-#define SEGMENT_IS_FLAT_CODE(x)  (((x) & 0xec) == GDT_ENTRY_KERNEL_CS * 8)
-/* Matches PNP_CS32 and PNP_CS16 (they must be consecutive) */
-#define SEGMENT_IS_PNP_CODE(x)   (((x) & 0xf4) == GDT_ENTRY_PNPBIOS_BASE * 8)
 
 /* Simple and small GDT entries for booting only */
 
@@ -134,4 +128,17 @@
 #ifndef CONFIG_PARAVIRT
 #define get_kernel_rpl()  0
 #endif
+/*
+ * Matching rules for certain types of segments.
+ */
+
+/* Matches only __KERNEL_CS, ignoring PnP / USER / APM segments */
+#define SEGMENT_IS_KERNEL_CODE(x) (((x) & 0xfc) == GDT_ENTRY_KERNEL_CS * 8)
+
+/* Matches __KERNEL_CS and __USER_CS (they must be 2 entries apart) */
+#define SEGMENT_IS_FLAT_CODE(x)  (((x) & 0xec) == GDT_ENTRY_KERNEL_CS * 8)
+
+/* Matches PNP_CS32 and PNP_CS16 (they must be consecutive) */
+#define SEGMENT_IS_PNP_CODE(x)   (((x) & 0xf4) == GDT_ENTRY_PNPBIOS_BASE * 8)
+
 #endif


  parent reply	other threads:[~2007-02-10 11:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-10 11:52 [PATCH 2.6.21 review II] [1/10] i386: page allocation hooks for VMI backend Andi Kleen
2007-02-10 11:52 ` [PATCH 2.6.21 review II] [2/10] i386: paravirt CPU hypercall batching mode Andi Kleen
2007-02-10 11:52 ` [PATCH 2.6.21 review II] [3/10] i386: iOPL handling for paravirt guests Andi Kleen
2007-02-10 11:52 ` [PATCH 2.6.21 review II] [4/10] i386: sMP boot hook for paravirt Andi Kleen
2007-02-10 11:52 ` [PATCH 2.6.21 review II] [5/10] i386: vMI backend for paravirt-ops Andi Kleen
2007-02-10 11:52 ` [PATCH 2.6.21 review II] [6/10] i386: vMI timer patches Andi Kleen
2007-02-10 11:52 ` Andi Kleen [this message]
2007-02-10 11:52 ` [PATCH 2.6.21 review II] [8/10] VMI: Kprobe rpl fix Andi Kleen
2007-02-10 11:52 ` [PATCH 2.6.21 review II] [9/10] VMI: Vmi timer race Andi Kleen
2007-02-10 11:52 ` [PATCH 2.6.21 review II] [10/10] VMI: Paravirt debug defaults off Andi Kleen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070210115235.6F12313DFE@wotan.suse.de \
    --to=ak@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@x86-64.org \
    --cc=zach@vmware.com \
    --subject='Re: [PATCH 2.6.21 review II] [7/10] VMI: Profile pc badness' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).