kernel - Fix proc_reparent() race/assertion panic

* Fix proc_reparent() race/assertion panic.  p_pptr changes can race,
  and the procedure had an assertion for the case.  Recode the procedure
  to retry on a mismatch instead of assert.

* Also move the old-parent-wakeup code into the procedure so it is
  properly executed in all cases.

Reported-by: Peter Avalos

diff --git a/sys/emulation/linux/i386/linux_machdep.c b/sys/emulation/linux/i386/linux_machdep.c
index c0b0f2e..b32e0c0 100644 (file)
--- a/sys/emulation/linux/i386/linux_machdep.c
+++ b/sys/emulation/linux/i386/linux_machdep.c
@@ -513,8 +513,12 @@ sys_linux_clone(struct linux_clone_args *args)
        args->sysmsg_fds[0] = p2 ? p2->p_pid : 0;
        args->sysmsg_fds[1] = 0;
        
-       if (args->flags & (LINUX_CLONE_PARENT | LINUX_CLONE_THREAD))
-               proc_reparent(p2, curproc->p_pptr /* XXX */);
+       if (args->flags & (LINUX_CLONE_PARENT | LINUX_CLONE_THREAD)) {
+               lwkt_gettoken(&curproc->p_token);
+               while (p2->p_pptr != curproc->p_pptr)
+                       proc_reparent(p2, curproc->p_pptr);
+               lwkt_reltoken(&curproc->p_token);
+       }
 
        emuldata_init(curproc, p2, args->flags);
        linux_proc_fork(p2, curproc, args->child_tidptr);

diff --git a/sys/emulation/linux/linux_emuldata.c b/sys/emulation/linux/linux_emuldata.c
index 7daf8f5..79cb180 100644 (file)
--- a/sys/emulation/linux/linux_emuldata.c
+++ b/sys/emulation/linux/linux_emuldata.c
@@ -204,8 +204,8 @@ emuldata_exit(void *unused, struct proc *p)
            (em->clone_flags & LINUX_CLONE_THREAD)) {
                p->p_sigparent = SIGCHLD;
 
-               wakeup((caddr_t) initproc); /* kern_exit seems to do this */
-               proc_reparent(p, initproc); /* XXX: might be dangerous */
+               proc_reparent(p, initproc);
+               wakeup((caddr_t)initproc); /* kern_exit seems to do this */
        }
 
        if ((em->s->group_pid == p->p_pid) &&

diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c
index dc5da33..a7c8aed 100644 (file)
--- a/sys/kern/kern_exit.c
+++ b/sys/kern/kern_exit.c
@@ -543,19 +543,7 @@ exit1(int rv)
         * instead (and hope it will handle this situation).
         */
        if (p->p_pptr->p_sigacts->ps_flag & (PS_NOCLDWAIT | PS_CLDSIGIGN)) {
-               struct proc *pp = p->p_pptr;
-
-               PHOLD(pp);
                proc_reparent(p, initproc);
-
-               /*
-                * If this was the last child of our parent, notify
-                * parent, so in case he was wait(2)ing, he will
-                * continue.  This function interlocks with pptr->p_token.
-                */
-               if (LIST_EMPTY(&pp->p_children))
-                       wakeup((caddr_t)pp);
-               PRELE(pp);
        }
 
        /* lwkt_gettoken(&proc_token); */
@@ -1122,33 +1110,49 @@ done:
 }
 
 /*
- * Make process 'parent' the new parent of process 'child'.
+ * Change child's parent process to parent.
  *
  * p_children/p_sibling requires the parent's token, and
  * changing pptr requires the child's token, so we have to
- * get three tokens to do this operation.
+ * get three tokens to do this operation.  We also need to
+ * hold pointers that might get ripped out from under us to
+ * preserve structural integrity.
+ *
+ * It is possible to race another reparent or disconnect or other
+ * similar operation.  We must retry when this situation occurs.
+ * Once we successfully reparent the process we no longer care
+ * about any races.
  */
 void
 proc_reparent(struct proc *child, struct proc *parent)
 {
-       struct proc *opp = child->p_pptr;
+       struct proc *opp;
 
-       if (opp == parent)
-               return;
-       PHOLD(opp);
        PHOLD(parent);
-       lwkt_gettoken(&opp->p_token);
-       lwkt_gettoken(&child->p_token);
-       lwkt_gettoken(&parent->p_token);
-       KKASSERT(child->p_pptr == opp);
-       LIST_REMOVE(child, p_sibling);
-       LIST_INSERT_HEAD(&parent->p_children, child, p_sibling);
-       child->p_pptr = parent;
-       lwkt_reltoken(&parent->p_token);
-       lwkt_reltoken(&child->p_token);
-       lwkt_reltoken(&opp->p_token);
+       while ((opp = child->p_pptr) != parent) {
+               PHOLD(opp);
+               lwkt_gettoken(&opp->p_token);
+               lwkt_gettoken(&child->p_token);
+               lwkt_gettoken(&parent->p_token);
+               if (child->p_pptr != opp) {
+                       lwkt_reltoken(&parent->p_token);
+                       lwkt_reltoken(&child->p_token);
+                       lwkt_reltoken(&opp->p_token);
+                       PRELE(opp);
+                       continue;
+               }
+               LIST_REMOVE(child, p_sibling);
+               LIST_INSERT_HEAD(&parent->p_children, child, p_sibling);
+               child->p_pptr = parent;
+               lwkt_reltoken(&parent->p_token);
+               lwkt_reltoken(&child->p_token);
+               lwkt_reltoken(&opp->p_token);
+               if (LIST_EMPTY(&opp->p_children))
+                       wakeup(opp);
+               PRELE(opp);
+               break;
+       }
        PRELE(parent);
-       PRELE(opp);
 }
 
 /*

diff --git a/sys/kern/sys_process.c b/sys/kern/sys_process.c
index 2b78add..4765c0c 100644 (file)
--- a/sys/kern/sys_process.c
+++ b/sys/kern/sys_process.c
@@ -458,8 +458,7 @@ kern_ptrace(struct proc *curp, int req, pid_t pid, void *addr,
                /* security check done above */
                p->p_flags |= P_TRACED;
                p->p_oppid = p->p_pptr->p_pid;
-               if (p->p_pptr != curp)
-                       proc_reparent(p, curp);
+               proc_reparent(p, curp);
                data = SIGSTOP;
                goto sendsig;   /* in PT_CONTINUE below */
 
@@ -504,8 +503,8 @@ kern_ptrace(struct proc *curp, int req, pid_t pid, void *addr,
                                struct proc *pp;
 
                                pp = pfind(p->p_oppid);
-                               proc_reparent(p, pp ? pp : initproc);
-                               if (pp != NULL)
+                               proc_reparent(p, pp);
+                               if (pp)
                                        PRELE(pp);
                        }