[PATCH] fix crash in entry.S restore_all

Fix the access-above-bottom-of-stack crash.

1. Allows to preserve the valueable optimization

2. Works for NMIs

3.  Doesn't care whether or not there are more of the like instances
   where the stack is left empty.

4. Seems to work for me without the crashes:)

(akpm: this is still under discussion, although I _think_ it's OK.  You might
want to hold off)

Signed-off-by: Stas Sergeev <stsp@aknet.ru>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/arch/i386/kernel/entry.S b/arch/i386/kernel/entry.S
index 1e45ff292bc99df5630b375f5518b86bbfbc3319..3c73dc865ead3ad2323098a3b78b0fc8042e7489 100644 (file)
--- a/arch/i386/kernel/entry.S
+++ b/arch/i386/kernel/entry.S
@@ -245,6 +245,9 @@ syscall_exit:
 
 restore_all:
        movl EFLAGS(%esp), %eax         # mix EFLAGS, SS and CS
+       # Warning: OLDSS(%esp) contains the wrong/random values if we
+       # are returning to the kernel.
+       # See comments in process.c:copy_thread() for details.
        movb OLDSS(%esp), %ah
        movb CS(%esp), %al
        andl $(VM_MASK | (4 << 8) | 3), %eax

diff --git a/arch/i386/kernel/process.c b/arch/i386/kernel/process.c
index c36fedf40e958093edcbe2f8fe82b64fe3ad0aeb..36145efc61b5e7aeb6da9360942152b08f449c0d 100644 (file)
--- a/arch/i386/kernel/process.c
+++ b/arch/i386/kernel/process.c
@@ -405,7 +405,17 @@ int copy_thread(int nr, unsigned long clone_flags, unsigned long esp,
        childregs->esp = esp;
 
        p->thread.esp = (unsigned long) childregs;
-       p->thread.esp0 = (unsigned long) (childregs+1);
+       /*
+        * The below -8 is to reserve 8 bytes on top of the ring0 stack.
+        * This is necessary to guarantee that the entire "struct pt_regs"
+        * is accessable even if the CPU haven't stored the SS/ESP registers
+        * on the stack (interrupt gate does not save these registers
+        * when switching to the same priv ring).
+        * Therefore beware: accessing the xss/esp fields of the
+        * "struct pt_regs" is possible, but they may contain the
+        * completely wrong values.
+        */
+       p->thread.esp0 = (unsigned long) (childregs+1) - 8;
 
        p->thread.eip = (unsigned long) ret_from_fork;