On 28/02/20(Fri) 18:24, Martin Pieuchot wrote:
> Diff below fixes multiple issues with traced process, exposed by the
> regression test attached:
> 
> - When a debugging process exit, give back the traced process to its
>   original parent, if it exists, instead of init(8).
> 
> - When a traced process exit, make sure the original parent receives
>   the exit status only after the debugger has seen it.  This is done
>   by keeping a list of 'orphaned' children in the original parent and
>   looking in it in dowait4().
> 
> The logic and most of the code comes from FreeBSD as pointed out by
> guenther@.

Improved versions that:

 - Introduce process_clear_orphan() to remove a process from an orphan
   list.

 - Introduce process_untrace() to give back a process to its original
   parent or to init(8), used in both PT_DETACH & exit1(). 

 - Rename proc_reparent() into process_reparent() for coherency

Ok?

Index: kern/kern_exit.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_exit.c,v
retrieving revision 1.184
diff -u -p -r1.184 kern_exit.c
--- kern/kern_exit.c    28 Feb 2020 17:03:05 -0000      1.184
+++ kern/kern_exit.c    29 Feb 2020 18:50:09 -0000
@@ -76,6 +76,7 @@
 #endif
 
 void   proc_finish_wait(struct proc *, struct proc *);
+void   process_clear_orphan(struct process *);
 void   process_zap(struct process *);
 void   proc_free(struct proc *);
 void   unveil_destroy(struct process *ps);
@@ -253,21 +254,22 @@ exit1(struct proc *p, int xexit, int xsi
                }
 
                /*
-                * Give orphaned children to init(8).
+                * Reparent children to their original parent, in case
+                * they were being traced, or to init(8).
                 */
                qr = LIST_FIRST(&pr->ps_children);
                if (qr)         /* only need this if any child is S_ZOMB */
                        wakeup(initprocess);
                for (; qr != 0; qr = nqr) {
                        nqr = LIST_NEXT(qr, ps_sibling);
-                       proc_reparent(qr, initprocess);
                        /*
                         * Traced processes are killed since their
                         * existence means someone is screwing up.
                         */
                        if (qr->ps_flags & PS_TRACED &&
                            !(qr->ps_flags & PS_EXITING)) {
-                               atomic_clearbits_int(&qr->ps_flags, PS_TRACED);
+                               process_untrace(qr);
+
                                /*
                                 * If single threading is active,
                                 * direct the signal to the active
@@ -278,8 +280,19 @@ exit1(struct proc *p, int xexit, int xsi
                                            STHREAD);
                                else
                                        prsignal(qr, SIGKILL);
+                       } else {
+                               process_reparent(qr, initprocess);
                        }
                }
+
+               /*
+                * Make sure orphans won't remember the exiting process.
+                */
+               while ((qr = LIST_FIRST(&pr->ps_orphans)) != NULL) {
+                       KASSERT(qr->ps_oppid == pr->ps_pid);
+                       qr->ps_oppid = 0;
+                       process_clear_orphan(qr);
+               }
        }
 
        /* add thread's accumulated rusage into the process's total */
@@ -310,7 +323,7 @@ exit1(struct proc *p, int xexit, int xsi
                 */
                if (pr->ps_flags & PS_NOZOMBIE) {
                        struct process *ppr = pr->ps_pptr;
-                       proc_reparent(pr, initprocess);
+                       process_reparent(pr, initprocess);
                        wakeup(ppr);
                }
 
@@ -562,6 +575,29 @@ loop:
                        return (0);
                }
        }
+       /*
+        * Look in the orphans list too, to allow the parent to
+        * collect it's child exit status even if child is being
+        * debugged.
+        *
+        * Debugger detaches from the parent upon successful
+        * switch-over from parent to child.  At this point due to
+        * re-parenting the parent loses the child to debugger and a
+        * wait4(2) call would report that it has no children to wait
+        * for.  By maintaining a list of orphans we allow the parent
+        * to successfully wait until the child becomes a zombie.
+        */
+       if (nfound == 0) {
+               LIST_FOREACH(pr, &q->p_p->ps_orphans, ps_orphan) {
+                       if ((pr->ps_flags & PS_NOZOMBIE) ||
+                           (pid != WAIT_ANY &&
+                           pr->ps_pid != pid &&
+                           pr->ps_pgid != -pid))
+                               continue;
+                       nfound++;
+                       break;
+               }
+       }
        if (nfound == 0)
                return (ECHILD);
        if (options & WNOHANG) {
@@ -584,10 +620,11 @@ proc_finish_wait(struct proc *waiter, st
         * we need to give it back to the old parent.
         */
        pr = p->p_p;
-       if (pr->ps_oppid && (tr = prfind(pr->ps_oppid))) {
+       if (pr->ps_oppid != 0 && (pr->ps_oppid != pr->ps_pptr->ps_pid) &&
+          (tr = prfind(pr->ps_oppid))) {
                atomic_clearbits_int(&pr->ps_flags, PS_TRACED);
                pr->ps_oppid = 0;
-               proc_reparent(pr, tr);
+               process_reparent(pr, tr);
                prsignal(tr, SIGCHLD);
                wakeup(tr);
        } else {
@@ -601,10 +638,39 @@ proc_finish_wait(struct proc *waiter, st
 }
 
 /*
+ * give process back to original parent or init(8)
+ */
+void
+process_untrace(struct process *pr)
+{
+       struct process *ppr = NULL;
+
+       KASSERT(pr->ps_flags & PS_TRACED);
+
+       if (pr->ps_oppid != 0 &&
+           (pr->ps_oppid != pr->ps_pptr->ps_pid))
+               ppr = prfind(pr->ps_oppid);
+
+       /* not being traced any more */
+       pr->ps_oppid = 0;
+       atomic_clearbits_int(&pr->ps_flags, PS_TRACED);
+       process_reparent(pr, ppr ? ppr : initprocess);
+}
+
+void
+process_clear_orphan(struct process *pr)
+{
+       if (ISSET(pr->ps_flags, PS_ORPHAN)) {
+               LIST_REMOVE(pr, ps_orphan);
+               atomic_clearbits_int(&pr->ps_flags, PS_ORPHAN);
+       }
+}
+
+/*
  * make process 'parent' the new parent of process 'child'.
  */
 void
-proc_reparent(struct process *child, struct process *parent)
+process_reparent(struct process *child, struct process *parent)
 {
 
        if (child->ps_pptr == parent)
@@ -612,6 +678,13 @@ proc_reparent(struct process *child, str
 
        LIST_REMOVE(child, ps_sibling);
        LIST_INSERT_HEAD(&parent->ps_children, child, ps_sibling);
+
+       process_clear_orphan(child);
+       if (ISSET(child->ps_flags, PS_TRACED)) {
+               atomic_setbits_int(&child->ps_flags, PS_ORPHAN);
+               LIST_INSERT_HEAD(&child->ps_pptr->ps_orphans, child, ps_orphan);
+       }
+
        child->ps_pptr = parent;
 }
 
@@ -627,6 +700,7 @@ process_zap(struct process *pr)
         */
        leavepgrp(pr);
        LIST_REMOVE(pr, ps_sibling);
+       process_clear_orphan(pr);
 
        /*
         * Decrement the count of procs running with this uid.
Index: kern/kern_fork.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_fork.c,v
retrieving revision 1.223
diff -u -p -r1.223 kern_fork.c
--- kern/kern_fork.c    21 Feb 2020 11:10:23 -0000      1.223
+++ kern/kern_fork.c    29 Feb 2020 18:31:48 -0000
@@ -190,6 +190,7 @@ process_initialize(struct process *pr, s
        KASSERT(p->p_ucred->cr_ref >= 2);       /* new thread and new process */
 
        LIST_INIT(&pr->ps_children);
+       LIST_INIT(&pr->ps_orphans);
        LIST_INIT(&pr->ps_ftlist);
        LIST_INIT(&pr->ps_sigiolst);
        TAILQ_INIT(&pr->ps_tslpqueue);
@@ -430,8 +431,7 @@ fork1(struct proc *curp, int flags, void
 
        if (pr->ps_flags & PS_TRACED) {
                pr->ps_oppid = curpr->ps_pid;
-               if (pr->ps_pptr != curpr->ps_pptr)
-                       proc_reparent(pr, curpr->ps_pptr);
+               process_reparent(pr, curpr->ps_pptr);
 
                /*
                 * Set ptrace status.
Index: kern/sys_process.c
===================================================================
RCS file: /cvs/src/sys/kern/sys_process.c,v
retrieving revision 1.82
diff -u -p -r1.82 sys_process.c
--- kern/sys_process.c  11 Dec 2019 07:30:09 -0000      1.82
+++ kern/sys_process.c  29 Feb 2020 18:43:07 -0000
@@ -476,17 +476,8 @@ ptrace_ctrl(struct proc *p, int req, pid
                        goto fail;
 #endif
 
-               /* give process back to original parent or init */
-               if (tr->ps_oppid != tr->ps_pptr->ps_pid) {
-                       struct process *ppr;
-
-                       ppr = prfind(tr->ps_oppid);
-                       proc_reparent(tr, ppr ? ppr : initprocess);
-               }
-
-               /* not being traced any more */
-               tr->ps_oppid = 0;
-               atomic_clearbits_int(&tr->ps_flags, PS_TRACED|PS_WAITED);
+               process_untrace(tr);
+               atomic_clearbits_int(&tr->ps_flags, PS_WAITED);
 
        sendsig:
                memset(tr->ps_ptstat, 0, sizeof(*tr->ps_ptstat));
@@ -523,8 +514,7 @@ ptrace_ctrl(struct proc *p, int req, pid
                 */
                atomic_setbits_int(&tr->ps_flags, PS_TRACED);
                tr->ps_oppid = tr->ps_pptr->ps_pid;
-               if (tr->ps_pptr != p->p_p)
-                       proc_reparent(tr, p->p_p);
+               process_reparent(tr, p->p_p);
                if (tr->ps_ptstat == NULL)
                        tr->ps_ptstat = malloc(sizeof(*tr->ps_ptstat),
                            M_SUBPROC, M_WAITOK);
Index: sys/proc.h
===================================================================
RCS file: /cvs/src/sys/sys/proc.h,v
retrieving revision 1.289
diff -u -p -r1.289 proc.h
--- sys/proc.h  21 Feb 2020 11:10:23 -0000      1.289
+++ sys/proc.h  28 Feb 2020 17:04:46 -0000
@@ -182,6 +182,15 @@ struct process {
        LIST_HEAD(, process) ps_children;/* Pointer to list of children. */
        LIST_ENTRY(process) ps_hash;    /* Hash chain. */
 
+       /*
+        * An orphan is the child that has been re-parented to the
+        * debugger as a result of attaching to it.  Need to keep
+        * track of them for parent to be able to collect the exit
+        * status of what used to be children.
+        */
+       LIST_ENTRY(process) ps_orphan;  /* List of orphan processes. */
+       LIST_HEAD(, process) ps_orphans;/* Pointer to list of orphans. */
+
        struct  sigiolst ps_sigiolst;   /* List of sigio structures. */
        struct  sigacts *ps_sigacts;    /* Signal actions, state */
        struct  vnode *ps_textvp;       /* Vnode of executable. */
@@ -303,6 +312,7 @@ struct process {
 #define        PS_PLEDGE       0x00100000      /* Has called pledge(2) */
 #define        PS_WXNEEDED     0x00200000      /* Process may violate W^X */
 #define        PS_EXECPLEDGE   0x00400000      /* Has exec pledges */
+#define        PS_ORPHAN       0x00800000      /* Process is on an orphan list 
*/
 
 #define        PS_BITS \
     ("\20" "\01CONTROLT" "\02EXEC" "\03INEXEC" "\04EXITING" "\05SUGID" \
Index: sys/ptrace.h
===================================================================
RCS file: /cvs/src/sys/sys/ptrace.h,v
retrieving revision 1.15
diff -u -p -r1.15 ptrace.h
--- sys/ptrace.h        19 Oct 2016 08:31:33 -0000      1.15
+++ sys/ptrace.h        29 Feb 2020 18:45:24 -0000
@@ -104,7 +104,8 @@ struct reg;
 struct fpreg;
 #endif
 
-void   proc_reparent(struct process *_child, struct process *_newparent);
+void   process_reparent(struct process *_child, struct process *_newparent);
+void   process_untrace(struct process *_tr);
 #ifdef PT_GETFPREGS
 int    process_read_fpregs(struct proc *_t, struct fpreg *);
 #endif

Reply via email to