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