On 29/02/20(Sat) 19:55, Martin Pieuchot wrote:
> 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
Add a KASSERT() and respect the style of the file, prodded by visa@.
ok?
Index: kern/kern_exit.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_exit.c,v
retrieving revision 1.185
diff -u -p -r1.185 kern_exit.c
--- kern/kern_exit.c 1 Mar 2020 18:50:52 -0000 1.185
+++ kern/kern_exit.c 6 Mar 2020 10:57:03 -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) {
@@ -586,9 +622,9 @@ proc_finish_wait(struct proc *waiter, st
pr = p->p_p;
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);
+ atomic_clearbits_int(&pr->ps_flags, PS_TRACED);
+ process_reparent(pr, tr);
prsignal(tr, SIGCHLD);
wakeup(tr);
} else {
@@ -602,17 +638,56 @@ 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 (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)
return;
+ KASSERT(child->ps_oppid == 0 ||
+ child->ps_oppid == child->ps_pptr->ps_pid);
+
LIST_REMOVE(child, ps_sibling);
LIST_INSERT_HEAD(&parent->ps_children, child, ps_sibling);
+
+ process_clear_orphan(child);
+ if (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;
}
@@ -628,6 +703,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 6 Mar 2020 09:18:53 -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 6 Mar 2020 09:18:53 -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 6 Mar 2020 09:25:23 -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,13 +312,15 @@ 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" \
"\06SUGIDEXEC" "\07PPWAIT" "\010ISPWAIT" "\011PROFIL" "\012TRACED" \
"\013WAITED" "\014COREDUMP" "\015SINGLEEXIT" "\016SINGLEUNWIND" \
"\017NOZOMBIE" "\020STOPPED" "\021SYSTEM" "\022EMBRYO" "\023ZOMBIE" \
- "\024NOBROADCASTKILL" "\025PLEDGE" "\026WXNEEDED" "\027EXECPLEDGE" )
+ "\024NOBROADCASTKILL" "\025PLEDGE" "\026WXNEEDED" "\027EXECPLEDGE" \
+ "\028ORPHAN")
struct kcov_dev;
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 6 Mar 2020 09:18:53 -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
