Date: Tue, 26 May 2015 10:29:38 +0000 (UTC) From: John Baldwin <jhb@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r283562 - in head: sys/kern tests/sys/kern Message-ID: <201505261029.t4QATcJh060476@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jhb Date: Tue May 26 10:29:37 2015 New Revision: 283562 URL: https://svnweb.freebsd.org/changeset/base/283562 Log: Do not allow a process to reap an orphan (a child currently being traced by another process such as a debugger). The parent process does need to check for matching orphan pids to avoid returning ECHILD if an orphan has exited, but it should not return the exited status for the child until after the debugger has detached from the orphan process either explicitly or implicitly via wait(). Add two tests for for this case: one where the debugger is the direct child (thus the parent has a non-empty children list) and one where the debugger is not a direct child (so the only "child" of the parent is the orphan). Differential Revision: https://reviews.freebsd.org/D2644 Reviewed by: kib MFC after: 2 weeks Modified: head/sys/kern/kern_exit.c head/tests/sys/kern/ptrace_test.c Modified: head/sys/kern/kern_exit.c ============================================================================== --- head/sys/kern/kern_exit.c Tue May 26 09:38:16 2015 (r283561) +++ head/sys/kern/kern_exit.c Tue May 26 10:29:37 2015 (r283562) @@ -955,7 +955,8 @@ proc_reap(struct thread *td, struct proc static int proc_to_reap(struct thread *td, struct proc *p, idtype_t idtype, id_t id, - int *status, int options, struct __wrusage *wrusage, siginfo_t *siginfo) + int *status, int options, struct __wrusage *wrusage, siginfo_t *siginfo, + int check_only) { struct proc *q; struct rusage *rup; @@ -1093,7 +1094,7 @@ proc_to_reap(struct thread *td, struct p calccru(p, &rup->ru_utime, &rup->ru_stime); } - if (p->p_state == PRS_ZOMBIE) { + if (p->p_state == PRS_ZOMBIE && !check_only) { PROC_SLOCK(p); proc_reap(td, p, status, options); return (-1); @@ -1187,7 +1188,7 @@ loop: sx_xlock(&proctree_lock); LIST_FOREACH(p, &q->p_children, p_sibling) { ret = proc_to_reap(td, p, idtype, id, status, options, - wrusage, siginfo); + wrusage, siginfo, 0); if (ret == 0) continue; else if (ret == 1) @@ -1289,15 +1290,17 @@ loop: * for. By maintaining a list of orphans we allow the parent * to successfully wait until the child becomes a zombie. */ - LIST_FOREACH(p, &q->p_orphans, p_orphan) { - ret = proc_to_reap(td, p, idtype, id, status, options, - wrusage, siginfo); - if (ret == 0) - continue; - else if (ret == 1) - nfound++; - else - return (0); + if (nfound == 0) { + LIST_FOREACH(p, &q->p_orphans, p_orphan) { + ret = proc_to_reap(td, p, idtype, id, NULL, options, + NULL, NULL, 1); + if (ret != 0) { + KASSERT(ret != -1, ("reaped an orphan (pid %d)", + (int)td->td_retval[0])); + nfound++; + break; + } + } } if (nfound == 0) { sx_xunlock(&proctree_lock); Modified: head/tests/sys/kern/ptrace_test.c ============================================================================== --- head/tests/sys/kern/ptrace_test.c Tue May 26 09:38:16 2015 (r283561) +++ head/tests/sys/kern/ptrace_test.c Tue May 26 10:29:37 2015 (r283562) @@ -29,6 +29,8 @@ __FBSDID("$FreeBSD$"); #include <sys/types.h> #include <sys/ptrace.h> +#include <sys/sysctl.h> +#include <sys/user.h> #include <sys/wait.h> #include <errno.h> #include <signal.h> @@ -133,11 +135,255 @@ ATF_TC_BODY(ptrace__parent_wait_after_at ATF_REQUIRE(errno == ECHILD); } +/* + * Verify that a parent process "sees" the exit of a debugged process only + * after the debugger has seen it. + */ +ATF_TC_WITHOUT_HEAD(ptrace__parent_sees_exit_after_child_debugger); +ATF_TC_BODY(ptrace__parent_sees_exit_after_child_debugger, tc) +{ + pid_t child, debugger, wpid; + int cpipe[2], dpipe[2], status; + char c; + + ATF_REQUIRE(pipe(cpipe) == 0); + ATF_REQUIRE((child = fork()) != -1); + + if (child == 0) { + /* Child process. */ + close(cpipe[0]); + + /* Wait for parent to be ready. */ + ATF_REQUIRE(read(cpipe[1], &c, sizeof(c)) == sizeof(c)); + + exit(1); + } + close(cpipe[1]); + + ATF_REQUIRE(pipe(dpipe) == 0); + ATF_REQUIRE((debugger = fork()) != -1); + + if (debugger == 0) { + /* Debugger process. */ + close(dpipe[0]); + + ATF_REQUIRE(ptrace(PT_ATTACH, child, NULL, 0) != -1); + + wpid = waitpid(child, &status, 0); + ATF_REQUIRE(wpid == child); + ATF_REQUIRE(WIFSTOPPED(status)); + ATF_REQUIRE(WSTOPSIG(status) == SIGSTOP); + + ATF_REQUIRE(ptrace(PT_CONTINUE, child, (caddr_t)1, 0) != -1); + + /* Signal parent that debugger is attached. */ + ATF_REQUIRE(write(dpipe[1], &c, sizeof(c)) == sizeof(c)); + + /* Wait for parent's failed wait. */ + ATF_REQUIRE(read(dpipe[1], &c, sizeof(c)) == 0); + + wpid = waitpid(child, &status, 0); + ATF_REQUIRE(wpid == child); + ATF_REQUIRE(WIFEXITED(status)); + ATF_REQUIRE(WEXITSTATUS(status) == 1); + + exit(0); + } + close(dpipe[1]); + + /* Parent process. */ + + /* Wait for the debugger to attach to the child. */ + ATF_REQUIRE(read(dpipe[0], &c, sizeof(c)) == sizeof(c)); + + /* Release the child. */ + ATF_REQUIRE(write(cpipe[0], &c, sizeof(c)) == sizeof(c)); + ATF_REQUIRE(read(cpipe[0], &c, sizeof(c)) == 0); + close(cpipe[0]); + + /* + * Wait for the child to exit. This is kind of gross, but + * there is not a better way. + */ + for (;;) { + struct kinfo_proc kp; + size_t len; + int mib[4]; + + mib[0] = CTL_KERN; + mib[1] = KERN_PROC; + mib[2] = KERN_PROC_PID; + mib[3] = child; + len = sizeof(kp); + if (sysctl(mib, nitems(mib), &kp, &len, NULL, 0) == -1) { + /* The KERN_PROC_PID sysctl fails for zombies. */ + ATF_REQUIRE(errno == ESRCH); + break; + } + usleep(5000); + } + + /* + * This wait should return an empty pid. The parent should + * see the child as non-exited until the debugger sees the + * exit. + */ + wpid = waitpid(child, &status, WNOHANG); + ATF_REQUIRE(wpid == 0); + + /* Signal the debugger to wait for the child. */ + close(dpipe[0]); + + /* Wait for the debugger. */ + wpid = waitpid(debugger, &status, 0); + ATF_REQUIRE(wpid == debugger); + ATF_REQUIRE(WIFEXITED(status)); + ATF_REQUIRE(WEXITSTATUS(status) == 0); + + /* The child process should now be ready. */ + wpid = waitpid(child, &status, WNOHANG); + ATF_REQUIRE(wpid == child); + ATF_REQUIRE(WIFEXITED(status)); + ATF_REQUIRE(WEXITSTATUS(status) == 1); +} + +/* + * Verify that a parent process "sees" the exit of a debugged process + * only after a non-direct-child debugger has seen it. In particular, + * various wait() calls in the parent must avoid failing with ESRCH by + * checking the parent's orphan list for the debugee. + */ +ATF_TC_WITHOUT_HEAD(ptrace__parent_sees_exit_after_unrelated_debugger); +ATF_TC_BODY(ptrace__parent_sees_exit_after_unrelated_debugger, tc) +{ + pid_t child, debugger, fpid, wpid; + int cpipe[2], dpipe[2], status; + char c; + + ATF_REQUIRE(pipe(cpipe) == 0); + ATF_REQUIRE((child = fork()) != -1); + + if (child == 0) { + /* Child process. */ + close(cpipe[0]); + + /* Wait for parent to be ready. */ + ATF_REQUIRE(read(cpipe[1], &c, sizeof(c)) == sizeof(c)); + + exit(1); + } + close(cpipe[1]); + + ATF_REQUIRE(pipe(dpipe) == 0); + ATF_REQUIRE((debugger = fork()) != -1); + + if (debugger == 0) { + /* Debugger parent. */ + + /* + * Fork again and drop the debugger parent so that the + * debugger is not a child of the main parent. + */ + ATF_REQUIRE((fpid = fork()) != -1); + if (fpid != 0) + exit(2); + + /* Debugger process. */ + close(dpipe[0]); + + ATF_REQUIRE(ptrace(PT_ATTACH, child, NULL, 0) != -1); + + wpid = waitpid(child, &status, 0); + ATF_REQUIRE(wpid == child); + ATF_REQUIRE(WIFSTOPPED(status)); + ATF_REQUIRE(WSTOPSIG(status) == SIGSTOP); + + ATF_REQUIRE(ptrace(PT_CONTINUE, child, (caddr_t)1, 0) != -1); + + /* Signal parent that debugger is attached. */ + ATF_REQUIRE(write(dpipe[1], &c, sizeof(c)) == sizeof(c)); + + /* Wait for parent's failed wait. */ + ATF_REQUIRE(read(dpipe[1], &c, sizeof(c)) == 0); + + wpid = waitpid(child, &status, 0); + ATF_REQUIRE(wpid == child); + ATF_REQUIRE(WIFEXITED(status)); + ATF_REQUIRE(WEXITSTATUS(status) == 1); + + exit(0); + } + + /* Parent process. */ + + /* Wait for the debugger parent process to exit. */ + wpid = waitpid(debugger, &status, 0); + ATF_REQUIRE(wpid == debugger); + ATF_REQUIRE(WIFEXITED(status)); + ATF_REQUIRE(WEXITSTATUS(status) == 2); + + /* A WNOHANG wait here should see the non-exited child. */ + wpid = waitpid(child, &status, WNOHANG); + ATF_REQUIRE(wpid == 0); + + /* Wait for the debugger to attach to the child. */ + ATF_REQUIRE(read(dpipe[0], &c, sizeof(c)) == sizeof(c)); + + /* Release the child. */ + ATF_REQUIRE(write(cpipe[0], &c, sizeof(c)) == sizeof(c)); + ATF_REQUIRE(read(cpipe[0], &c, sizeof(c)) == 0); + close(cpipe[0]); + + /* + * Wait for the child to exit. This is kind of gross, but + * there is not a better way. + */ + for (;;) { + struct kinfo_proc kp; + size_t len; + int mib[4]; + + mib[0] = CTL_KERN; + mib[1] = KERN_PROC; + mib[2] = KERN_PROC_PID; + mib[3] = child; + len = sizeof(kp); + if (sysctl(mib, nitems(mib), &kp, &len, NULL, 0) == -1) { + /* The KERN_PROC_PID sysctl fails for zombies. */ + ATF_REQUIRE(errno == ESRCH); + break; + } + usleep(5000); + } + + /* + * This wait should return an empty pid. The parent should + * see the child as non-exited until the debugger sees the + * exit. + */ + wpid = waitpid(child, &status, WNOHANG); + ATF_REQUIRE(wpid == 0); + + /* Signal the debugger to wait for the child. */ + close(dpipe[0]); + + /* Wait for the debugger. */ + ATF_REQUIRE(read(dpipe[1], &c, sizeof(c)) == 0); + + /* The child process should now be ready. */ + wpid = waitpid(child, &status, WNOHANG); + ATF_REQUIRE(wpid == child); + ATF_REQUIRE(WIFEXITED(status)); + ATF_REQUIRE(WEXITSTATUS(status) == 1); +} + 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); + ATF_TP_ADD_TC(tp, ptrace__parent_sees_exit_after_child_debugger); + ATF_TP_ADD_TC(tp, ptrace__parent_sees_exit_after_unrelated_debugger); return (atf_no_error()); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201505261029.t4QATcJh060476>