2006-08-10 19:38:09

by Andi Kleen

[permalink] [raw]
Subject: [PATCH for review] [145/145] i386: Disallow kprobes on NMI handlers

r

From: Fernando Luis =?ISO-8859-1?Q?V=E1zquez?= Cao <[email protected]>
A kprobe executes IRET early and that could cause NMI recursion and stack
corruption.

Note: This problem was originally spotted and solved by Andi Kleen in the
x86_64 architecture. This patch is an adaption of his patch for i386.

AK: Merged with current code which was a bit different.
AK: Removed printk in nmi handler that shouldn't be there in the first time
AK: Added missing include.

Signed-off-by: Fernando Vazquez <[email protected]>
Signed-off-by: Andi Kleen <[email protected]>

---

---
arch/i386/kernel/entry.S | 2 +-
arch/i386/kernel/nmi.c | 6 +++---
arch/i386/kernel/traps.c | 15 +++++++++------
3 files changed, 13 insertions(+), 10 deletions(-)

Index: linux/arch/i386/kernel/entry.S
===================================================================
--- linux.orig/arch/i386/kernel/entry.S
+++ linux/arch/i386/kernel/entry.S
@@ -725,7 +725,7 @@ debug_stack_correct:
* check whether we got an NMI on the debug path where the debug
* fault happened on the sysenter path.
*/
-ENTRY(nmi)
+KPROBE_ENTRY(nmi)
RING0_INT_FRAME
pushl %eax
CFI_ADJUST_CFA_OFFSET 4
Index: linux/arch/i386/kernel/nmi.c
===================================================================
--- linux.orig/arch/i386/kernel/nmi.c
+++ linux/arch/i386/kernel/nmi.c
@@ -22,6 +22,7 @@
#include <linux/sysctl.h>
#include <linux/percpu.h>
#include <linux/dmi.h>
+#include <linux/kprobes.h>

#include <asm/smp.h>
#include <asm/nmi.h>
@@ -882,7 +883,7 @@ EXPORT_SYMBOL(touch_nmi_watchdog);

extern void die_nmi(struct pt_regs *, const char *msg);

-int nmi_watchdog_tick (struct pt_regs * regs, unsigned reason)
+__kprobes int nmi_watchdog_tick(struct pt_regs * regs, unsigned reason)
{

/*
@@ -962,8 +963,7 @@ int nmi_watchdog_tick (struct pt_regs *
* This matches the old behaviour.
*/
rc = 1;
- } else
- printk(KERN_WARNING "Unknown enabled NMI hardware?!\n");
+ }
}
done:
return rc;
Index: linux/arch/i386/kernel/traps.c
===================================================================
--- linux.orig/arch/i386/kernel/traps.c
+++ linux/arch/i386/kernel/traps.c
@@ -680,7 +680,8 @@ gp_in_kernel:
}
}

-static void mem_parity_error(unsigned char reason, struct pt_regs * regs)
+static __kprobes void
+mem_parity_error(unsigned char reason, struct pt_regs * regs)
{
printk(KERN_EMERG "Uhhuh. NMI received for unknown reason %02x on "
"CPU %d.\n", reason, smp_processor_id());
@@ -695,7 +696,8 @@ static void mem_parity_error(unsigned ch
clear_mem_error(reason);
}

-static void io_check_error(unsigned char reason, struct pt_regs * regs)
+static __kprobes void
+io_check_error(unsigned char reason, struct pt_regs * regs)
{
unsigned long i;

@@ -711,7 +713,8 @@ static void io_check_error(unsigned char
outb(reason, 0x61);
}

-static void unknown_nmi_error(unsigned char reason, struct pt_regs * regs)
+static __kprobes void
+unknown_nmi_error(unsigned char reason, struct pt_regs * regs)
{
#ifdef CONFIG_MCA
/* Might actually be able to figure out what the guilty party
@@ -732,7 +735,7 @@ static void unknown_nmi_error(unsigned c

static DEFINE_SPINLOCK(nmi_print_lock);

-void die_nmi (struct pt_regs *regs, const char *msg)
+void __kprobes die_nmi(struct pt_regs *regs, const char *msg)
{
if (notify_die(DIE_NMIWATCHDOG, msg, regs, 0, 2, SIGINT) ==
NOTIFY_STOP)
@@ -764,7 +767,7 @@ void die_nmi (struct pt_regs *regs, cons
do_exit(SIGSEGV);
}

-static void default_do_nmi(struct pt_regs * regs)
+static __kprobes void default_do_nmi(struct pt_regs * regs)
{
unsigned char reason = 0;

@@ -802,7 +805,7 @@ static void default_do_nmi(struct pt_reg
reassert_nmi();
}

-fastcall void do_nmi(struct pt_regs * regs, long error_code)
+fastcall __kprobes void do_nmi(struct pt_regs * regs, long error_code)
{
int cpu;


2006-08-11 06:42:14

by Chuck Ebbert

[permalink] [raw]
Subject: Re: [PATCH for review] [145/145] i386: Disallow kprobes on NMI handlers

In-Reply-To: <[email protected]>

On Thu, 10 Aug 2006 21:37:45 +0200, Andi Kleen wrote:

> --- linux.orig/arch/i386/kernel/entry.S
> +++ linux/arch/i386/kernel/entry.S
> @@ -725,7 +725,7 @@ debug_stack_correct:
> * check whether we got an NMI on the debug path where the debug
> * fault happened on the sysenter path.
> */
> -ENTRY(nmi)
> +KPROBE_ENTRY(nmi)
> RING0_INT_FRAME
> pushl %eax
> CFI_ADJUST_CFA_OFFSET 4

Needs .popsection at the end of the NMI code.

--
Chuck

2006-08-11 06:54:39

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH for review] [145/145] i386: Disallow kprobes on NMI handlers

On Friday 11 August 2006 08:37, Chuck Ebbert wrote:
> In-Reply-To: <[email protected]>
>
> On Thu, 10 Aug 2006 21:37:45 +0200, Andi Kleen wrote:
>
> > --- linux.orig/arch/i386/kernel/entry.S
> > +++ linux/arch/i386/kernel/entry.S
> > @@ -725,7 +725,7 @@ debug_stack_correct:
> > * check whether we got an NMI on the debug path where the debug
> > * fault happened on the sysenter path.
> > */
> > -ENTRY(nmi)
> > +KPROBE_ENTRY(nmi)
> > RING0_INT_FRAME
> > pushl %eax
> > CFI_ADJUST_CFA_OFFSET 4
>
> Needs .popsection at the end of the NMI code.

This is fixed up in a later patch I think.
i386: KPROBE_ENTRY ends up putting code into .fixup


-andi

2006-08-11 23:18:13

by Chuck Ebbert

[permalink] [raw]
Subject: Re: [PATCH for review] [145/145] i386: Disallow kprobes on NMI handlers

In-Reply-To: <[email protected]>

On Fri, 11 Aug 2006 08:53:19 +0200, Andi Kleen wrote:

> > > --- linux.orig/arch/i386/kernel/entry.S
> > > +++ linux/arch/i386/kernel/entry.S
> > > @@ -725,7 +725,7 @@ debug_stack_correct:
> > > * check whether we got an NMI on the debug path where the debug
> > > * fault happened on the sysenter path.
> > > */
> > > -ENTRY(nmi)
> > > +KPROBE_ENTRY(nmi)
> > > RING0_INT_FRAME
> > > pushl %eax
> > > CFI_ADJUST_CFA_OFFSET 4
> >
> > Needs .popsection at the end of the NMI code.
>
> This is fixed up in a later patch I think.
> i386: KPROBE_ENTRY ends up putting code into .fixup

That patch was earlier, not later. I applied the 060811 patchset from
firstfloor and you still need this:

--- 2.6.18-rc4-ff.orig/arch/i386/kernel/entry.S
+++ 2.6.18-rc4-ff/arch/i386/kernel/entry.S
@@ -801,6 +801,7 @@ nmi_16bit_stack:
.align 4
.long 1b,iret_exc
.previous
+.popsection

KPROBE_ENTRY(int3)
RING0_INT_FRAME
--
Chuck