Date: Wed, 9 Sep 2015 23:05:53 +0000 (UTC) From: John Baldwin <jhb@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r287603 - in stable/9: bin/ps sys/kern sys/sys Message-ID: <201509092305.t89N5rbS045907@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jhb Date: Wed Sep 9 23:05:52 2015 New Revision: 287603 URL: https://svnweb.freebsd.org/changeset/base/287603 Log: MFC 269656,270024,270443,270993: Fix problems with orphan handling. Note that to preserve KBI, p_treeflag has been added to the end of struct proc and is explicitly zero'd during fork. 269656: 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). 270024: Correct the order of arguments passed to LIST_INSERT_AFTER(). 270443: Properly reparent traced processes when the tracer dies. Previously they were uncoditionally reparented to init. In effect it was possible that tracee was never returned to original parent. 270993: Fix up proc_realparent to always return correct process. Prior to the change it would always return initproc for non-traced processes. Modified: stable/9/bin/ps/ps.1 stable/9/sys/kern/kern_exit.c stable/9/sys/kern/kern_fork.c stable/9/sys/kern/kern_proc.c stable/9/sys/kern/sys_process.c stable/9/sys/sys/proc.h Directory Properties: stable/9/bin/ps/ (props changed) stable/9/sys/ (props changed) stable/9/sys/sys/ (props changed) Modified: stable/9/bin/ps/ps.1 ============================================================================== --- stable/9/bin/ps/ps.1 Wed Sep 9 22:54:07 2015 (r287602) +++ stable/9/bin/ps/ps.1 Wed Sep 9 23:05:52 2015 (r287603) @@ -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: stable/9/sys/kern/kern_exit.c ============================================================================== --- stable/9/sys/kern/kern_exit.c Wed Sep 9 22:54:07 2015 (r287602) +++ stable/9/sys/kern/kern_exit.c Wed Sep 9 23:05:52 2015 (r287603) @@ -99,16 +99,48 @@ 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) { + if (child->p_oppid == 0 || + child->p_pptr->p_pid == child->p_oppid) + parent = child->p_pptr; + else + parent = initproc; + return (parent); + } + 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; } /* @@ -130,7 +162,8 @@ sys_sys_exit(struct thread *td, struct s void exit1(struct thread *td, int rv) { - struct proc *p, *nq, *q; + struct proc *p, *nq, *q, *t; + struct thread *tdt; struct vnode *vtmp; struct vnode *ttyvp = NULL; struct plimit *plim; @@ -419,7 +452,9 @@ exit1(struct thread *td, int rv) WITNESS_WARN(WARN_PANIC, NULL, "process (pid %d) exiting", p->p_pid); /* - * Reparent all of our children to init. + * Reparent all children processes: + * - traced ones to the original parent (or init if we are that parent) + * - the rest to init */ sx_xlock(&proctree_lock); q = LIST_FIRST(&p->p_children); @@ -428,15 +463,23 @@ exit1(struct thread *td, int rv) for (; q != NULL; q = nq) { nq = LIST_NEXT(q, p_sibling); PROC_LOCK(q); - proc_reparent(q, initproc); q->p_sigparent = SIGCHLD; - /* - * Traced processes are killed - * since their existence means someone is screwing up. - */ - if (q->p_flag & P_TRACED) { - struct thread *temp; + if (!(q->p_flag & P_TRACED)) { + proc_reparent(q, initproc); + } else { + /* + * Traced processes are killed since their existence + * means someone is screwing up. + */ + t = proc_realparent(q); + if (t == p) { + proc_reparent(q, initproc); + } else { + PROC_LOCK(t); + proc_reparent(q, t); + PROC_UNLOCK(t); + } /* * Since q was found on our children list, the * proc_reparent() call moved q to the orphan @@ -445,8 +488,8 @@ exit1(struct thread *td, int rv) */ clear_orphan(q); q->p_flag &= ~(P_TRACED | P_STOPPED_TRACE); - FOREACH_THREAD_IN_PROC(q, temp) - temp->td_dbgflags &= ~TDB_SUSPEND; + FOREACH_THREAD_IN_PROC(q, tdt) + tdt->td_dbgflags &= ~TDB_SUSPEND; kern_psignal(q, SIGKILL); } PROC_UNLOCK(q); @@ -788,7 +831,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); CTR2(KTR_PTRACE, "wait: traced child %d moved back to parent %d", p->p_pid, @@ -1269,8 +1314,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(LIST_FIRST(&child->p_pptr->p_orphans), + child, p_orphan); + } + child->p_treeflag |= P_TREE_ORPHANED; } child->p_pptr = parent; Modified: stable/9/sys/kern/kern_fork.c ============================================================================== --- stable/9/sys/kern/kern_fork.c Wed Sep 9 22:54:07 2015 (r287602) +++ stable/9/sys/kern/kern_fork.c Wed Sep 9 23:05:52 2015 (r287603) @@ -408,6 +408,7 @@ do_fork(struct thread *td, int flags, st bzero(&p2->p_startzero, __rangeof(struct proc, p_startzero, p_endzero)); + p2->p_treeflag = 0; p2->p_ucred = crhold(td->td_ucred); Modified: stable/9/sys/kern/kern_proc.c ============================================================================== --- stable/9/sys/kern/kern_proc.c Wed Sep 9 22:54:07 2015 (r287602) +++ stable/9/sys/kern/kern_proc.c Wed Sep 9 23:05:52 2015 (r287603) @@ -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: stable/9/sys/kern/sys_process.c ============================================================================== --- stable/9/sys/kern/sys_process.c Wed Sep 9 22:54:07 2015 (r287602) +++ stable/9/sys/kern/sys_process.c Wed Sep 9 23:05:52 2015 (r287603) @@ -951,19 +951,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: stable/9/sys/sys/proc.h ============================================================================== --- stable/9/sys/sys/proc.h Wed Sep 9 22:54:07 2015 (r287602) +++ stable/9/sys/sys/proc.h Wed Sep 9 23:05:52 2015 (r287603) @@ -593,6 +593,7 @@ struct proc { LIST_ENTRY(proc) p_orphan; /* (e) List of orphan processes. */ LIST_HEAD(, proc) p_orphans; /* (e) Pointer to list of orphans. */ u_char p_throttled; /* (c) Flag for racct pcpu throttling */ + u_int p_treeflag; /* (e) P_TREE flags */ }; #define p_session p_pgrp->pg_session @@ -630,7 +631,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. */ @@ -645,6 +646,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. @@ -891,6 +897,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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201509092305.t89N5rbS045907>