Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 25 Mar 2017 20:14:08 +0000 (UTC)
From:      Eric Badger <badger@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org
Subject:   svn commit: r315963 - in stable: 10/sys/kern 10/tests/sys/kern 11/sys/kern 11/tests/sys/kern
Message-ID:  <201703252014.v2PKE8Cl065223@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: badger
Date: Sat Mar 25 20:14:08 2017
New Revision: 315963
URL: https://svnweb.freebsd.org/changeset/base/315963

Log:
  MFC r315412, r314852:
  
  r315412:
      Don't clear p_ptevents on normal SIGKILL delivery
  
      The ptrace() user has the option of discarding the signal. In such a
      case, p_ptevents should not be modified. If the ptrace() user decides to
      send a SIGKILL, ptevents will be cleared in ptracestop(). procfs events
      do not have the capability to discard the signal, so continue to clear
      the mask in that case.
  
  r314852:
      don't stop in issignal() if P_SINGLE_EXIT is set
  
      Suppose a traced process is stopped in ptracestop() due to receipt of a
      SIGSTOP signal, and is awaiting orders from the tracing process on how
      to handle the signal. Before sending any such orders, the tracing
      process exits. This should kill the traced process. But suppose a second
      thread handles the SIGKILL and proceeds to exit1(), calling
      thread_single(). The first thread will now awaken and will have a chance
      to check once more if it should go to sleep due to the SIGSTOP.  It must
      not sleep after P_SINGLE_EXIT has been set; this would prevent the
      SIGKILL from taking effect, leaving a stopped orphan behind after the
      tracing process dies.
  
      Also add new tests for this condition.
  
  Sponsored by:	Dell EMC

Modified:
  stable/10/sys/kern/kern_sig.c
  stable/10/tests/sys/kern/ptrace_test.c
Directory Properties:
  stable/10/   (props changed)

Changes in other areas also in this revision:
Modified:
  stable/11/sys/kern/kern_sig.c
  stable/11/tests/sys/kern/ptrace_test.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/10/sys/kern/kern_sig.c
==============================================================================
--- stable/10/sys/kern/kern_sig.c	Sat Mar 25 19:46:53 2017	(r315962)
+++ stable/10/sys/kern/kern_sig.c	Sat Mar 25 20:14:08 2017	(r315963)
@@ -2197,11 +2197,9 @@ tdsendsignal(struct proc *p, struct thre
 	if (action == SIG_HOLD &&
 	    !((prop & SA_CONT) && (p->p_flag & P_STOPPED_SIG)))
 		return (ret);
-	/*
-	 * SIGKILL: Remove procfs STOPEVENTs and ptrace events.
-	 */
+
+	/* SIGKILL: Remove procfs STOPEVENTs. */
 	if (sig == SIGKILL) {
-		p->p_ptevents = 0;
 		/* from procfs_ioctl.c: PIOCBIC */
 		p->p_stops = 0;
 		/* from procfs_ioctl.c: PIOCCONT */
@@ -2824,14 +2822,15 @@ issignal(struct thread *td)
 				break;		/* == ignore */
 			}
 			/*
-			 * If there is a pending stop signal to process
-			 * with default action, stop here,
-			 * then clear the signal.  However,
-			 * if process is member of an orphaned
-			 * process group, ignore tty stop signals.
+			 * If there is a pending stop signal to process with
+			 * default action, stop here, then clear the signal.
+			 * Traced or exiting processes should ignore stops.
+			 * Additionally, a member of an orphaned process group
+			 * should ignore tty stops.
 			 */
 			if (prop & SA_STOP) {
-				if (p->p_flag & (P_TRACED|P_WEXIT) ||
+				if (p->p_flag &
+				    (P_TRACED | P_WEXIT | P_SINGLE_EXIT) ||
 				    (p->p_pgrp->pg_jobc == 0 &&
 				     prop & SA_TTYSTOP))
 					break;	/* == ignore */

Modified: stable/10/tests/sys/kern/ptrace_test.c
==============================================================================
--- stable/10/tests/sys/kern/ptrace_test.c	Sat Mar 25 19:46:53 2017	(r315962)
+++ stable/10/tests/sys/kern/ptrace_test.c	Sat Mar 25 20:14:08 2017	(r315963)
@@ -31,6 +31,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/cpuset.h>
 #include <sys/event.h>
 #include <sys/time.h>
+#include <sys/procctl.h>
 #include <sys/ptrace.h>
 #include <sys/queue.h>
 #include <sys/runq.h>
@@ -2732,6 +2733,212 @@ ATF_TC_BODY(ptrace__PT_CONTINUE_with_sig
 	ATF_REQUIRE(errno == ECHILD);
 }
 
+static void *
+raise_sigstop_thread(void *arg __unused)
+{
+
+	raise(SIGSTOP);
+	return NULL;
+}
+
+static void *
+sleep_thread(void *arg __unused)
+{
+
+	sleep(60);
+	return NULL;
+}
+
+static void
+terminate_with_pending_sigstop(bool sigstop_from_main_thread)
+{
+	pid_t fpid, wpid;
+	int status, i;
+	cpuset_t setmask;
+	cpusetid_t setid;
+	pthread_t t;
+
+	/*
+	 * Become the reaper for this process tree. We need to be able to check
+	 * that both child and grandchild have died.
+	 */
+	ATF_REQUIRE(procctl(P_PID, getpid(), PROC_REAP_ACQUIRE, NULL) == 0);
+
+	fpid = fork();
+	ATF_REQUIRE(fpid >= 0);
+	if (fpid == 0) {
+		fpid = fork();
+		CHILD_REQUIRE(fpid >= 0);
+		if (fpid == 0) {
+			trace_me();
+
+			/* Pin to CPU 0 to serialize thread execution. */
+			CPU_ZERO(&setmask);
+			CPU_SET(0, &setmask);
+			CHILD_REQUIRE(cpuset(&setid) == 0);
+			CHILD_REQUIRE(cpuset_setaffinity(CPU_LEVEL_CPUSET,
+			    CPU_WHICH_CPUSET, setid,
+			    sizeof(setmask), &setmask) == 0);
+
+			if (sigstop_from_main_thread) {
+				/*
+				 * We expect the SIGKILL sent when our parent
+				 * dies to be delivered to the new thread.
+				 * Raise the SIGSTOP in this thread so the
+				 * threads compete.
+				 */
+				CHILD_REQUIRE(pthread_create(&t, NULL,
+				    sleep_thread, NULL) == 0);
+				raise(SIGSTOP);
+			} else {
+				/*
+				 * We expect the SIGKILL to be delivered to
+				 * this thread. After creating the new thread,
+				 * just get off the CPU so the other thread can
+				 * raise the SIGSTOP.
+				 */
+				CHILD_REQUIRE(pthread_create(&t, NULL,
+				    raise_sigstop_thread, NULL) == 0);
+				sleep(60);
+			}
+
+			exit(0);
+		}
+		/* First stop is trace_me() immediately after fork. */
+		wpid = waitpid(fpid, &status, 0);
+		CHILD_REQUIRE(wpid == fpid);
+		CHILD_REQUIRE(WIFSTOPPED(status));
+		CHILD_REQUIRE(WSTOPSIG(status) == SIGSTOP);
+
+		CHILD_REQUIRE(ptrace(PT_CONTINUE, fpid, (caddr_t)1, 0) == 0);
+
+		/* Second stop is from the raise(SIGSTOP). */
+		wpid = waitpid(fpid, &status, 0);
+		CHILD_REQUIRE(wpid == fpid);
+		CHILD_REQUIRE(WIFSTOPPED(status));
+		CHILD_REQUIRE(WSTOPSIG(status) == SIGSTOP);
+
+		/*
+		 * Terminate tracing process without detaching. Our child
+		 * should be killed.
+		 */
+		exit(0);
+	}
+
+	/*
+	 * We should get a normal exit from our immediate child and a SIGKILL
+	 * exit from our grandchild. The latter case is the interesting one.
+	 * Our grandchild should not have stopped due to the SIGSTOP that was
+	 * left dangling when its parent died.
+	 */
+	for (i = 0; i < 2; ++i) {
+		wpid = wait(&status);
+		if (wpid == fpid)
+			ATF_REQUIRE(WIFEXITED(status) &&
+			    WEXITSTATUS(status) == 0);
+		else
+			ATF_REQUIRE(WIFSIGNALED(status) &&
+			    WTERMSIG(status) == SIGKILL);
+	}
+}
+
+/*
+ * These two tests ensure that if the tracing process exits without detaching
+ * just after the child received a SIGSTOP, the child is cleanly killed and
+ * doesn't go to sleep due to the SIGSTOP. The parent's death will send a
+ * SIGKILL to the child. If the SIGKILL and the SIGSTOP are handled by
+ * different threads, the SIGKILL must win.  There are two variants of this
+ * test, designed to catch the case where the SIGKILL is delivered to the
+ * younger thread (the first test) and the case where the SIGKILL is delivered
+ * to the older thread (the second test). This behavior has changed in the
+ * past, so make no assumption.
+ */
+ATF_TC_WITHOUT_HEAD(ptrace__parent_terminate_with_pending_sigstop1);
+ATF_TC_BODY(ptrace__parent_terminate_with_pending_sigstop1, tc)
+{
+
+	terminate_with_pending_sigstop(true);
+}
+ATF_TC_WITHOUT_HEAD(ptrace__parent_terminate_with_pending_sigstop2);
+ATF_TC_BODY(ptrace__parent_terminate_with_pending_sigstop2, tc)
+{
+
+	terminate_with_pending_sigstop(false);
+}
+
+/*
+ * Verify that after ptrace() discards a SIGKILL signal, the event mask
+ * is not modified.
+ */
+ATF_TC_WITHOUT_HEAD(ptrace__event_mask_sigkill_discard);
+ATF_TC_BODY(ptrace__event_mask_sigkill_discard, tc)
+{
+	struct ptrace_lwpinfo pl;
+	pid_t fpid, wpid;
+	int status, event_mask, new_event_mask;
+
+	ATF_REQUIRE((fpid = fork()) != -1);
+	if (fpid == 0) {
+		trace_me();
+		raise(SIGSTOP);
+		exit(0);
+	}
+
+	/* The first wait() should report the stop from trace_me(). */
+	wpid = waitpid(fpid, &status, 0);
+	ATF_REQUIRE(wpid == fpid);
+	ATF_REQUIRE(WIFSTOPPED(status));
+	ATF_REQUIRE(WSTOPSIG(status) == SIGSTOP);
+
+	/* Set several unobtrusive event bits. */
+	event_mask = PTRACE_EXEC | PTRACE_FORK | PTRACE_LWP;
+	ATF_REQUIRE(ptrace(PT_SET_EVENT_MASK, wpid, (caddr_t)&event_mask,
+	    sizeof(event_mask)) == 0);
+
+	/* Send a SIGKILL without using ptrace. */
+	ATF_REQUIRE(kill(fpid, SIGKILL) == 0);
+
+	/* Continue the child ignoring the SIGSTOP. */
+	ATF_REQUIRE(ptrace(PT_CONTINUE, fpid, (caddr_t)1, 0) == 0);
+
+	/* The next stop should be due to the SIGKILL. */
+	wpid = waitpid(fpid, &status, 0);
+	ATF_REQUIRE(wpid == fpid);
+	ATF_REQUIRE(WIFSTOPPED(status));
+	ATF_REQUIRE(WSTOPSIG(status) == SIGKILL);
+
+	ATF_REQUIRE(ptrace(PT_LWPINFO, wpid, (caddr_t)&pl, sizeof(pl)) != -1);
+	ATF_REQUIRE(pl.pl_flags & PL_FLAG_SI);
+	ATF_REQUIRE(pl.pl_siginfo.si_signo == SIGKILL);
+
+	/* Continue the child ignoring the SIGKILL. */
+	ATF_REQUIRE(ptrace(PT_CONTINUE, fpid, (caddr_t)1, 0) == 0);
+
+	/* The next wait() should report the stop from SIGSTOP. */
+	wpid = waitpid(fpid, &status, 0);
+	ATF_REQUIRE(wpid == fpid);
+	ATF_REQUIRE(WIFSTOPPED(status));
+	ATF_REQUIRE(WSTOPSIG(status) == SIGSTOP);
+
+	/* Check the current event mask. It should not have changed. */
+	new_event_mask = 0;
+	ATF_REQUIRE(ptrace(PT_GET_EVENT_MASK, wpid, (caddr_t)&new_event_mask,
+	    sizeof(new_event_mask)) == 0);
+	ATF_REQUIRE(event_mask == new_event_mask);
+
+	/* Continue the child to let it exit. */
+	ATF_REQUIRE(ptrace(PT_CONTINUE, fpid, (caddr_t)1, 0) == 0);
+
+	/* The last event should be for the child process's exit. */
+	wpid = waitpid(fpid, &status, 0);
+	ATF_REQUIRE(WIFEXITED(status));
+	ATF_REQUIRE(WEXITSTATUS(status) == 0);
+
+	wpid = wait(&status);
+	ATF_REQUIRE(wpid == -1);
+	ATF_REQUIRE(errno == ECHILD);
+}
+
 ATF_TP_ADD_TCS(tp)
 {
 
@@ -2775,6 +2982,9 @@ ATF_TP_ADD_TCS(tp)
 	ATF_TP_ADD_TC(tp, ptrace__PT_CONTINUE_with_signal_mix);
 	ATF_TP_ADD_TC(tp, ptrace__PT_CONTINUE_with_signal_kqueue);
 	ATF_TP_ADD_TC(tp, ptrace__PT_CONTINUE_with_signal_thread_sigmask);
+	ATF_TP_ADD_TC(tp, ptrace__parent_terminate_with_pending_sigstop1);
+	ATF_TP_ADD_TC(tp, ptrace__parent_terminate_with_pending_sigstop2);
+	ATF_TP_ADD_TC(tp, ptrace__event_mask_sigkill_discard);
 
 	return (atf_no_error());
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201703252014.v2PKE8Cl065223>