Author: kib
Date: Thu Aug  7 05:47:53 2014
New Revision: 269656
URL: http://svnweb.freebsd.org/changeset/base/269656

Log:
  Correct the problems with the ptrace(2) making the debuggee an orphan.
  One problem is inferior(9) looping due to the process tree becoming a
  graph instead of tree if the parent is traced by child. Another issue
  is due to the use of p_oppid to restore the original parent/child
  relationship, because real parent could already exited and its pid
  reused (noted by mjg).
  
  Add the function proc_realparent(9), which calculates the parent for
  given process. It uses the flag P_TREE_FIRST_ORPHAN to detect the head
  element of the p_orphan list and than stepping back to its container
  to find the parent process. If the parent has already exited, the
  init(8) is returned.
  
  Move the P_ORPHAN and the new helper flag from the p_flag* to new
  p_treeflag field of struct proc, which is protected by proctree lock
  instead of proc lock, since the orphans relationship is managed under
  the proctree_lock already.
  
  The remaining uses of p_oppid in ptrace(PT_DETACH) and process
  reapping are replaced by proc_realparent(9).
  
  Phabric:      D417
  Reviewed by:  jhb
  Tested by:    pho
  Sponsored by: The FreeBSD Foundation
  MFC after:    2 weeks

Modified:
  head/bin/ps/ps.1
  head/sys/kern/kern_exit.c
  head/sys/kern/kern_proc.c
  head/sys/kern/sys_process.c
  head/sys/sys/proc.h

Modified: head/bin/ps/ps.1
==============================================================================
--- head/bin/ps/ps.1    Thu Aug  7 03:50:30 2014        (r269655)
+++ head/bin/ps/ps.1    Thu Aug  7 05:47:53 2014        (r269656)
@@ -29,7 +29,7 @@
 .\"     @(#)ps.1       8.3 (Berkeley) 4/18/94
 .\" $FreeBSD$
 .\"
-.Dd June 6, 2014
+.Dd August 7, 2014
 .Dt PS 1
 .Os
 .Sh NAME
@@ -332,7 +332,6 @@ the include file
 .It Dv "P_SINGLE_BOUNDARY" Ta No "0x400000" Ta "Threads should suspend at user 
boundary"
 .It Dv "P_HWPMC" Ta No "0x800000" Ta "Process is using HWPMCs"
 .It Dv "P_JAILED" Ta No "0x1000000" Ta "Process is in jail"
-.It Dv "P_ORPHAN" Ta No "0x2000000" Ta "Orphaned by original parent, 
reparented to debugger"
 .It Dv "P_INEXEC" Ta No "0x4000000" Ta "Process is in execve()"
 .It Dv "P_STATCHILD" Ta No "0x8000000" Ta "Child process stopped or exited"
 .It Dv "P_INMEM" Ta No "0x10000000" Ta "Loaded into memory"

Modified: head/sys/kern/kern_exit.c
==============================================================================
--- head/sys/kern/kern_exit.c   Thu Aug  7 03:50:30 2014        (r269655)
+++ head/sys/kern/kern_exit.c   Thu Aug  7 05:47:53 2014        (r269656)
@@ -97,16 +97,44 @@ SDT_PROBE_DEFINE1(proc, kernel, , exit, 
 /* Hook for NFS teardown procedure. */
 void (*nlminfo_release_p)(struct proc *p);
 
+struct proc *
+proc_realparent(struct proc *child)
+{
+       struct proc *p, *parent;
+
+       sx_assert(&proctree_lock, SX_LOCKED);
+       if ((child->p_treeflag & P_TREE_ORPHANED) == 0) {
+               return (child->p_pptr->p_pid == child->p_oppid ?
+                   child->p_pptr : initproc);
+       }
+       for (p = child; (p->p_treeflag & P_TREE_FIRST_ORPHAN) == 0;) {
+               /* Cannot use LIST_PREV(), since the list head is not known. */
+               p = __containerof(p->p_orphan.le_prev, struct proc,
+                   p_orphan.le_next);
+               KASSERT((p->p_treeflag & P_TREE_ORPHANED) != 0,
+                   ("missing P_ORPHAN %p", p));
+       }
+       parent = __containerof(p->p_orphan.le_prev, struct proc,
+           p_orphans.lh_first);
+       return (parent);
+}
+
 static void
 clear_orphan(struct proc *p)
 {
+       struct proc *p1;
 
-       PROC_LOCK_ASSERT(p, MA_OWNED);
-
-       if (p->p_flag & P_ORPHAN) {
-               LIST_REMOVE(p, p_orphan);
-               p->p_flag &= ~P_ORPHAN;
+       sx_assert(&proctree_lock, SA_XLOCKED);
+       if ((p->p_treeflag & P_TREE_ORPHANED) == 0)
+               return;
+       if ((p->p_treeflag & P_TREE_FIRST_ORPHAN) != 0) {
+               p1 = LIST_NEXT(p, p_orphan);
+               if (p1 != NULL)
+                       p1->p_treeflag |= P_TREE_FIRST_ORPHAN;
+               p->p_treeflag &= ~P_TREE_FIRST_ORPHAN;
        }
+       LIST_REMOVE(p, p_orphan);
+       p->p_treeflag &= ~P_TREE_ORPHANED;
 }
 
 /*
@@ -772,7 +800,9 @@ proc_reap(struct thread *td, struct proc
         * If we got the child via a ptrace 'attach', we need to give it back
         * to the old parent.
         */
-       if (p->p_oppid && (t = pfind(p->p_oppid)) != NULL) {
+       if (p->p_oppid != 0) {
+               t = proc_realparent(p);
+               PROC_LOCK(t);
                PROC_LOCK(p);
                proc_reparent(p, t);
                p->p_oppid = 0;
@@ -1243,8 +1273,15 @@ proc_reparent(struct proc *child, struct
 
        clear_orphan(child);
        if (child->p_flag & P_TRACED) {
-               LIST_INSERT_HEAD(&child->p_pptr->p_orphans, child, p_orphan);
-               child->p_flag |= P_ORPHAN;
+               if (LIST_EMPTY(&child->p_pptr->p_orphans)) {
+                       child->p_treeflag |= P_TREE_FIRST_ORPHAN;
+                       LIST_INSERT_HEAD(&child->p_pptr->p_orphans, child,
+                           p_orphan);
+               } else {
+                       LIST_INSERT_AFTER(child,
+                           LIST_FIRST(&child->p_pptr->p_orphans), p_orphan);
+               }
+               child->p_treeflag |= P_TREE_ORPHANED;
        }
 
        child->p_pptr = parent;

Modified: head/sys/kern/kern_proc.c
==============================================================================
--- head/sys/kern/kern_proc.c   Thu Aug  7 03:50:30 2014        (r269655)
+++ head/sys/kern/kern_proc.c   Thu Aug  7 05:47:53 2014        (r269656)
@@ -263,14 +263,15 @@ proc_fini(void *mem, int size)
  * Is p an inferior of the current process?
  */
 int
-inferior(p)
-       register struct proc *p;
+inferior(struct proc *p)
 {
 
        sx_assert(&proctree_lock, SX_LOCKED);
-       for (; p != curproc; p = p->p_pptr)
+       PROC_LOCK_ASSERT(p, MA_OWNED);
+       for (; p != curproc; p = proc_realparent(p)) {
                if (p->p_pid == 0)
                        return (0);
+       }
        return (1);
 }
 

Modified: head/sys/kern/sys_process.c
==============================================================================
--- head/sys/kern/sys_process.c Thu Aug  7 03:50:30 2014        (r269655)
+++ head/sys/kern/sys_process.c Thu Aug  7 05:47:53 2014        (r269656)
@@ -917,19 +917,11 @@ kern_ptrace(struct thread *td, int req, 
                case PT_DETACH:
                        /* reset process parent */
                        if (p->p_oppid != p->p_pptr->p_pid) {
-                               struct proc *pp;
-
                                PROC_LOCK(p->p_pptr);
                                sigqueue_take(p->p_ksi);
                                PROC_UNLOCK(p->p_pptr);
 
-                               PROC_UNLOCK(p);
-                               pp = pfind(p->p_oppid);
-                               if (pp == NULL)
-                                       pp = initproc;
-                               else
-                                       PROC_UNLOCK(pp);
-                               PROC_LOCK(p);
+                               pp = proc_realparent(p);
                                proc_reparent(p, pp);
                                if (pp == initproc)
                                        p->p_sigparent = SIGCHLD;

Modified: head/sys/sys/proc.h
==============================================================================
--- head/sys/sys/proc.h Thu Aug  7 03:50:30 2014        (r269655)
+++ head/sys/sys/proc.h Thu Aug  7 05:47:53 2014        (r269656)
@@ -549,6 +549,7 @@ struct proc {
        int             p_pendingcnt;   /* how many signals are pending */
        struct itimers  *p_itimers;     /* (c) POSIX interval timers. */
        struct procdesc *p_procdesc;    /* (e) Process descriptor, if any. */
+       u_int           p_treeflag;     /* (e) P_TREE flags */
 /* End area that is zeroed on creation. */
 #define        p_endzero       p_magic
 
@@ -632,7 +633,7 @@ struct proc {
 #define        P_SINGLE_BOUNDARY 0x400000 /* Threads should suspend at user 
boundary. */
 #define        P_HWPMC         0x800000 /* Process is using HWPMCs */
 #define        P_JAILED        0x1000000 /* Process is in jail. */
-#define        P_ORPHAN        0x2000000 /* Orphaned. */
+#define        P_UNUSED1       0x2000000
 #define        P_INEXEC        0x4000000 /* Process is in execve(). */
 #define        P_STATCHILD     0x8000000 /* Child process stopped or exited. */
 #define        P_INMEM         0x10000000 /* Loaded into memory. */
@@ -647,6 +648,11 @@ struct proc {
 /* These flags are kept in p_flag2. */
 #define        P2_INHERIT_PROTECTED 0x00000001 /* New children get 
P_PROTECTED. */
 
+/* Flags protected by proctree_lock, kept in p_treeflags. */
+#define        P_TREE_ORPHANED         0x00000001      /* Reparented, on 
orphan list */
+#define        P_TREE_FIRST_ORPHAN     0x00000002      /* First element of 
orphan
+                                                  list */
+
 /*
  * These were process status values (p_stat), now they are only used in
  * legacy conversion code.
@@ -887,6 +893,7 @@ int proc_getenvv(struct thread *td, stru
 void   procinit(void);
 void   proc_linkup0(struct proc *p, struct thread *td);
 void   proc_linkup(struct proc *p, struct thread *td);
+struct proc *proc_realparent(struct proc *child);
 void   proc_reap(struct thread *td, struct proc *p, int *status, int options);
 void   proc_reparent(struct proc *child, struct proc *newparent);
 struct pstats *pstats_alloc(void);
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to