xtensa: copy_thread with CLONE_VM must not copy live parent AR windows
authorMarc Gauthier <marc@tensilica.com>
Sun, 14 Oct 2012 23:55:35 +0000 (03:55 +0400)
committerChris Zankel <chris@zankel.net>
Tue, 16 Oct 2012 04:42:27 +0000 (21:42 -0700)
When doing a fork (new VM), the new task has a mirror image of the
parent's stack, so keeps the same live register windows etc.
However when doing a clone with CLONE_VM, keeping the same VM
(eg. when creating a new thread), the child starts afresh on a new
stack -- it cannot share any part of the parent stack.  It
especially cannot have the same live AR windows as the parent,
otherwise it will overwrite the parent stack on overflow, likely
causing corruption.  (and so it did...)

Effectively, the register windows need to be spilled.
Turns out it's much easier to simply not copy parent register
windows when CLONE_VM is set.

Signed-off-by: Marc Gauthier <marc@tensilica.com>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Chris Zankel <chris@zankel.net>
arch/xtensa/kernel/process.c

index bc020825cce55197e6aaf7dfc127203a143e3553..7901ee76b9be8f50fc344eb9d4cbea9247a82785 100644 (file)
@@ -173,6 +173,16 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
  *
  * Note: This is a pristine frame, so we don't need any spill region on top of
  *       childregs.
+ *
+ * The fun part:  if we're keeping the same VM (i.e. cloning a thread,
+ * not an entire process), we're normally given a new usp, and we CANNOT share
+ * any live address register windows.  If we just copy those live frames over,
+ * the two threads (parent and child) will overflow the same frames onto the
+ * parent stack at different times, likely corrupting the parent stack (esp.
+ * if the parent returns from functions that called clone() and calls new
+ * ones, before the child overflows its now old copies of its parent windows).
+ * One solution is to spill windows to the parent stack, but that's fairly
+ * involved.  Much simpler to just not copy those live frames across.
  */
 
 int copy_thread(unsigned long clone_flags, unsigned long usp,
@@ -191,13 +201,14 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
        else
                childregs = (struct pt_regs*)tos - 1;
 
+       /* This does not copy all the regs.  In a bout of brilliance or madness,
+          ARs beyond a0-a15 exist past the end of the struct. */
        *childregs = *regs;
 
        /* Create a call4 dummy-frame: a0 = 0, a1 = childregs. */
        *((int*)childregs - 3) = (unsigned long)childregs;
        *((int*)childregs - 4) = 0;
 
-       childregs->areg[1] = tos;
        childregs->areg[2] = 0;
        p->set_child_tid = p->clear_child_tid = NULL;
        p->thread.ra = MAKE_RA_FOR_CALL((unsigned long)ret_from_fork, 0x1);
@@ -205,10 +216,14 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
 
        if (user_mode(regs)) {
 
-               int len = childregs->wmask & ~0xf;
                childregs->areg[1] = usp;
-               memcpy(&childregs->areg[XCHAL_NUM_AREGS - len/4],
-                      &regs->areg[XCHAL_NUM_AREGS - len/4], len);
+               if (clone_flags & CLONE_VM) {
+                       childregs->wmask = 1;   /* can't share live windows */
+               } else {
+                       int len = childregs->wmask & ~0xf;
+                       memcpy(&childregs->areg[XCHAL_NUM_AREGS - len/4],
+                              &regs->areg[XCHAL_NUM_AREGS - len/4], len);
+               }
 // FIXME: we need to set THREADPTR in thread_info...
                if (clone_flags & CLONE_SETTLS)
                        childregs->areg[2] = childregs->areg[6];
@@ -216,6 +231,7 @@ int copy_thread(unsigned long clone_flags, unsigned long usp,
        } else {
                /* In kernel space, we start a new thread with a new stack. */
                childregs->wmask = 1;
+               childregs->areg[1] = tos;
        }
 
 #if (XTENSA_HAVE_COPROCESSORS || XTENSA_HAVE_IO_PORTS)