svn commit: r339390 - in head: sys/kern tests/sys/kern

[Mark Johnston]

Author: markj
Date: Tue Oct 16 20:06:56 2018
New Revision: 339390
URL: https://svnweb.freebsd.org/changeset/base/339390

Log:
  Reparent a child of pdfork(2) to its reaper when the procdesc is closed.
  
  Unconditionally reparenting to PID 1 breaks the procctl(2) reaper
  functionality.
  
  Add a regression test for this case.
  
  Reviewed by:	kib
  Approved by:	re (gjb)
  MFC after:	2 weeks
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D17589

Modified:
  head/sys/kern/sys_procdesc.c
  head/tests/sys/kern/reaper.c

Modified: head/sys/kern/sys_procdesc.c
==============================================================================
--- head/sys/kern/sys_procdesc.c	Tue Oct 16 19:26:04 2018	(r339389)
+++ head/sys/kern/sys_procdesc.c	Tue Oct 16 20:06:56 2018	(r339390)
@@ -312,8 +312,8 @@ procdesc_exit(struct proc *p)
 	pd = p->p_procdesc;
 
 	PROCDESC_LOCK(pd);
-	KASSERT((pd->pd_flags & PDF_CLOSED) == 0 || p->p_pptr == initproc,
-	    ("procdesc_exit: closed && parent not init"));
+	KASSERT((pd->pd_flags & PDF_CLOSED) == 0 || p->p_pptr == p->p_reaper,
+	    ("procdesc_exit: closed && parent not reaper"));
 
 	pd->pd_flags |= PDF_EXITED;
 	pd->pd_xstat = KW_EXITCODE(p->p_xexit, p->p_xsig);
@@ -361,7 +361,8 @@ procdesc_reap(struct proc *p)
 /*
  * procdesc_close() - last close on a process descriptor.  If the process is
  * still running, terminate with SIGKILL (unless PDF_DAEMON is set) and let
- * init(8) clean up the mess; if not, we have to clean up the zombie ourselves.
+ * its reaper clean up the mess; if not, we have to clean up the zombie
+ * ourselves.
  */
 static int
 procdesc_close(struct file *fp, struct thread *td)
@@ -410,12 +411,12 @@ procdesc_close(struct file *fp, struct thread *td)
 			procdesc_free(pd);
 
 			/*
-			 * Next, reparent it to init(8) so that there's someone
-			 * to pick up the pieces; finally, terminate with
-			 * prejudice.
+			 * Next, reparent it to its reaper (usually init(8)) so
+			 * that there's someone to pick up the pieces; finally,
+			 * terminate with prejudice.
 			 */
 			p->p_sigparent = SIGCHLD;
-			proc_reparent(p, initproc);
+			proc_reparent(p, p->p_reaper);
 			if ((pd->pd_flags & PDF_DAEMON) == 0)
 				kern_psignal(p, SIGKILL);
 			PROC_UNLOCK(p);

Modified: head/tests/sys/kern/reaper.c
==============================================================================
--- head/tests/sys/kern/reaper.c	Tue Oct 16 19:26:04 2018	(r339389)
+++ head/tests/sys/kern/reaper.c	Tue Oct 16 20:06:56 2018	(r339390)
@@ -28,6 +28,7 @@
 __FBSDID("$FreeBSD$");
 
 #include <sys/procctl.h>
+#include <sys/procdesc.h>
 #include <sys/wait.h>
 
 #include <atf-c.h>
@@ -740,6 +741,40 @@ ATF_TC_BODY(reaper_kill_subtree, tc)
 	ATF_REQUIRE_EQ(0, r);
 }
 
+ATF_TC_WITHOUT_HEAD(reaper_pdfork);
+ATF_TC_BODY(reaper_pdfork, tc)
+{
+	struct procctl_reaper_status st;
+	pid_t child, grandchild, parent, pid;
+	int pd, r, status;
+
+	parent = getpid();
+	r = procctl(P_PID, parent, PROC_REAP_ACQUIRE, NULL);
+	ATF_REQUIRE_EQ(r, 0);
+
+	child = pdfork(&pd, 0);
+	ATF_REQUIRE(child != -1);
+	if (child == 0) {
+		grandchild = pdfork(&pd, 0);
+		if (grandchild == -1)
+			_exit(1);
+		if (grandchild == 0)
+			pause();
+		_exit(0);
+	}
+	pid = waitpid(child, &status, 0);
+	ATF_REQUIRE_EQ(pid, child);
+	r = WIFEXITED(status) ? WEXITSTATUS(status) : -1;
+	ATF_REQUIRE_EQ(r, 0);
+
+	r = procctl(P_PID, parent, PROC_REAP_STATUS, &st);
+	ATF_REQUIRE_EQ(r, 0);
+	ATF_CHECK((st.rs_flags & REAPER_STATUS_OWNED) != 0);
+	ATF_CHECK(st.rs_reaper == parent);
+	ATF_CHECK(st.rs_children == 1);
+	ATF_CHECK(st.rs_descendants == 1);
+}
+
 ATF_TP_ADD_TCS(tp)
 {
 
@@ -754,5 +789,6 @@ ATF_TP_ADD_TCS(tp)
 	ATF_TP_ADD_TC(tp, reaper_kill_empty);
 	ATF_TP_ADD_TC(tp, reaper_kill_normal);
 	ATF_TP_ADD_TC(tp, reaper_kill_subtree);
+	ATF_TP_ADD_TC(tp, reaper_pdfork);
 	return (atf_no_error());
 }