Only reparent a traced process to its old parent if the tracing process is

not the old parent. Otherwise, proc_reap() will leave the zombie in place
resulting in the process' status being returned twice to its parent.

Add test cases for PT_TRACE_ME and PT_ATTACH which are fixed by
this change.

Differential Revision:	https://reviews.freebsd.org/D2594
Reviewed by:	kib
MFC after:	2 weeks

3 files changed, 148 insertions, 2 deletions

diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c
index 0a601a1..e760bc8 100644
--- a/sys/kern/kern_exit.c
+++ b/sys/kern/kern_exit.c
@@ -847,13 +847,13 @@ proc_reap(struct thread *td, struct proc *p, int *status, int options)
 	PROC_LOCK(q);
 	sigqueue_take(p->p_ksi);
 	PROC_UNLOCK(q);
-	PROC_UNLOCK(p);
 
 	/*
 	 * If we got the child via a ptrace 'attach', we need to give it back
 	 * to the old parent.
 	 */
-	if (p->p_oppid != 0) {
+	if (p->p_oppid != 0 && p->p_oppid != p->p_pptr->p_pid) {
+		PROC_UNLOCK(p);
 		t = proc_realparent(p);
 		PROC_LOCK(t);
 		PROC_LOCK(p);
@@ -867,6 +867,8 @@ proc_reap(struct thread *td, struct proc *p, int *status, int options)
 		sx_xunlock(&proctree_lock);
 		return;
 	}
+	p->p_oppid = 0;
+	PROC_UNLOCK(p);
 
 	/*
 	 * Remove other references to this process to ensure we have an
diff --git a/tests/sys/kern/Makefile b/tests/sys/kern/Makefile
index d366aa1..bf6aa0d 100644
--- a/tests/sys/kern/Makefile
+++ b/tests/sys/kern/Makefile
@@ -3,6 +3,7 @@
 TESTSDIR=	${TESTSBASE}/sys/kern
 
 ATF_TESTS_C+=	kern_descrip_test
+ATF_TESTS_C+=	ptrace_test
 ATF_TESTS_C+=	unix_seqpacket_test
 TEST_METADATA.unix_seqpacket_test+=	timeout="15"
 
diff --git a/tests/sys/kern/ptrace_test.c b/tests/sys/kern/ptrace_test.c
new file mode 100644
index 0000000..28b0592
--- /dev/null
+++ b/tests/sys/kern/ptrace_test.c
@@ -0,0 +1,143 @@
+/*-
+ * Copyright (c) 2015 John Baldwin <jhb@FreeBSD.org>
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
+#include <sys/types.h>
+#include <sys/ptrace.h>
+#include <sys/wait.h>
+#include <errno.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <atf-c.h>
+
+/*
+ * Verify that a parent debugger process "sees" the exit of a debugged
+ * process exactly once when attached via PT_TRACE_ME.
+ */
+ATF_TC_WITHOUT_HEAD(ptrace__parent_wait_after_trace_me);
+ATF_TC_BODY(ptrace__parent_wait_after_trace_me, tc)
+{
+	pid_t child, wpid;
+	int status;
+
+	ATF_REQUIRE((child = fork()) != -1);
+	if (child == 0) {
+		/* Child process. */
+		ATF_REQUIRE(ptrace(PT_TRACE_ME, 0, NULL, 0) != -1);
+
+		/* Trigger a stop. */
+		raise(SIGSTOP);
+
+		exit(1);
+	}
+
+	/* Parent process. */
+
+	/* The first wait() should report the stop from SIGSTOP. */
+	wpid = waitpid(child, &status, 0);
+	ATF_REQUIRE(wpid == child);
+	ATF_REQUIRE(WIFSTOPPED(status));
+	ATF_REQUIRE(WSTOPSIG(status) == SIGSTOP);
+
+	/* Continue the child ignoring the SIGSTOP. */
+	ATF_REQUIRE(ptrace(PT_CONTINUE, child, (caddr_t)1, 0) != -1);
+
+	/* The second wait() should report the exit status. */
+	wpid = waitpid(child, &status, 0);
+	ATF_REQUIRE(wpid == child);
+	ATF_REQUIRE(WIFEXITED(status));
+	ATF_REQUIRE(WEXITSTATUS(status) == 1);
+
+	/* The child should no longer exist. */
+	wpid = waitpid(child, &status, 0);
+	ATF_REQUIRE(wpid == -1);
+	ATF_REQUIRE(errno == ECHILD);
+}
+
+/*
+ * Verify that a parent debugger process "sees" the exit of a debugged
+ * process exactly once when attached via PT_ATTACH.
+ */
+ATF_TC_WITHOUT_HEAD(ptrace__parent_wait_after_attach);
+ATF_TC_BODY(ptrace__parent_wait_after_attach, tc)
+{
+	pid_t child, wpid;
+	int cpipe[2], status;
+	char c;
+
+	ATF_REQUIRE(pipe(cpipe) == 0);
+	ATF_REQUIRE((child = fork()) != -1);
+	if (child == 0) {
+		/* Child process. */
+		close(cpipe[0]);
+
+		/* Wait for the parent to attach. */
+		ATF_REQUIRE(read(cpipe[1], &c, sizeof(c)) == 0);
+
+		exit(1);
+	}
+	close(cpipe[1]);
+
+	/* Parent process. */
+
+	/* Attach to the child process. */
+	ATF_REQUIRE(ptrace(PT_ATTACH, child, NULL, 0) == 0);
+
+	/* The first wait() should report the SIGSTOP from PT_ATTACH. */
+	wpid = waitpid(child, &status, 0);
+	ATF_REQUIRE(wpid == child);
+	ATF_REQUIRE(WIFSTOPPED(status));
+	ATF_REQUIRE(WSTOPSIG(status) == SIGSTOP);
+
+	/* Continue the child ignoring the SIGSTOP. */
+	ATF_REQUIRE(ptrace(PT_CONTINUE, child, (caddr_t)1, 0) != -1);
+
+	/* Signal the child to exit. */
+	close(cpipe[0]);
+
+	/* The second wait() should report the exit status. */
+	wpid = waitpid(child, &status, 0);
+	ATF_REQUIRE(wpid == child);
+	ATF_REQUIRE(WIFEXITED(status));
+	ATF_REQUIRE(WEXITSTATUS(status) == 1);
+
+	/* The child should no longer exist. */
+	wpid = waitpid(child, &status, 0);
+	ATF_REQUIRE(wpid == -1);
+	ATF_REQUIRE(errno == ECHILD);
+}
+
+ATF_TP_ADD_TCS(tp)
+{
+
+	ATF_TP_ADD_TC(tp, ptrace__parent_wait_after_trace_me);
+	ATF_TP_ADD_TC(tp, ptrace__parent_wait_after_attach);
+
+	return (atf_no_error());
+}