svn commit: r303423 - in head: bin/ps sys/kern sys/sys

Konstantin Belousov kib at FreeBSD.org
Thu Jul 28 08:41:15 UTC 2016


Author: kib
Date: Thu Jul 28 08:41:13 2016
New Revision: 303423
URL: https://svnweb.freebsd.org/changeset/base/303423

Log:
  When a debugger attaches to the process, SIGSTOP is sent to the
  target.  Due to a way issignal() selects the next signal to deliver
  and report, if the simultaneous or already pending another signal
  exists, that signal might be reported by the next waitpid(2) call.
  This causes minor annoyance for debuggers, which must be prepared to
  take any signal as the first event, then filter SIGSTOP later.
  
  More importantly, for tools like gcore(1), which attach and then
  detach without processing events, SIGSTOP might leak to be delivered
  after PT_DETACH.  This results in the process being unintentionally
  stopped after detach, which is fatal for automatic tools.
  
  The solution is to force SIGSTOP to be the first signal reported after
  the attach.  Attach code is modified to set P2_PTRACE_FSTP to indicate
  that the attaching ritual was not yet finished, and issignal() prefers
  SIGSTOP in that condition.  Also, the thread which handles
  P2_PTRACE_FSTP is made to guarantee to own p_xthread during the first
  waitpid(2).  All that ensures that SIGSTOP is consumed first.
  
  Additionally, if P2_PTRACE_FSTP is still set on detach, which means
  that waitpid(2) was not called at all, SIGSTOP is removed from the
  queue, ensuring that the process is resumed on detach.
  
  In issignal(), when acting on STOPing signals, remove the signal from
  queue before suspending.  Otherwise parallel attach could result in
  ptracestop() acting on that STOP as if it was the STOP signal from the
  attach.  Then SIGSTOP from attach leaks again.
  
  As a minor refactoring, some bits of the common attach code is moved
  to new helper proc_set_traced().
  
  Reported by:	markj
  Reviewed by:	jhb, markj
  Tested by:	pho
  Sponsored by:	The FreeBSD Foundation
  MFC after:	2 weeks
  Differential revision:	https://reviews.freebsd.org/D7256

Modified:
  head/bin/ps/ps.1
  head/sys/kern/kern_exit.c
  head/sys/kern/kern_fork.c
  head/sys/kern/kern_sig.c
  head/sys/kern/sys_process.c
  head/sys/sys/proc.h

Modified: head/bin/ps/ps.1
==============================================================================
--- head/bin/ps/ps.1	Thu Jul 28 06:46:10 2016	(r303422)
+++ head/bin/ps/ps.1	Thu Jul 28 08:41:13 2016	(r303423)
@@ -29,7 +29,7 @@
 .\"     @(#)ps.1	8.3 (Berkeley) 4/18/94
 .\" $FreeBSD$
 .\"
-.Dd December 1, 2015
+.Dd July 28, 2016
 .Dt PS 1
 .Os
 .Sh NAME
@@ -360,6 +360,7 @@ the include file
 .It Dv "P2_NOTRACE" Ta No "0x00000002" Ta "No ptrace(2) attach or coredumps"
 .It Dv "P2_NOTRACE_EXEC" Ta No "0x00000004" Ta "Keep P2_NOPTRACE on exec(2)"
 .It Dv "P2_AST_SU" Ta No "0x00000008" Ta "Handles SU ast for kthreads"
+.It Dv "P2_PTRACE_FSTP" Ta No "0x00000010" Ta "SIGSTOP from PT_ATTACH not yet handled"
 .El
 .It Cm label
 The MAC label of the process.

Modified: head/sys/kern/kern_exit.c
==============================================================================
--- head/sys/kern/kern_exit.c	Thu Jul 28 06:46:10 2016	(r303422)
+++ head/sys/kern/kern_exit.c	Thu Jul 28 08:41:13 2016	(r303423)
@@ -476,9 +476,12 @@ exit1(struct thread *td, int rval, int s
 			 */
 			clear_orphan(q);
 			q->p_flag &= ~(P_TRACED | P_STOPPED_TRACE);
+			q->p_flag2 &= ~P2_PTRACE_FSTP;
 			q->p_ptevents = 0;
-			FOREACH_THREAD_IN_PROC(q, tdt)
-				tdt->td_dbgflags &= ~TDB_SUSPEND;
+			FOREACH_THREAD_IN_PROC(q, tdt) {
+				tdt->td_dbgflags &= ~(TDB_SUSPEND | TDB_XSIG |
+				    TDB_FSTP);
+			}
 			kern_psignal(q, SIGKILL);
 		}
 		PROC_UNLOCK(q);

Modified: head/sys/kern/kern_fork.c
==============================================================================
--- head/sys/kern/kern_fork.c	Thu Jul 28 06:46:10 2016	(r303422)
+++ head/sys/kern/kern_fork.c	Thu Jul 28 08:41:13 2016	(r303423)
@@ -1074,15 +1074,13 @@ fork_return(struct thread *td, struct tr
 			 * parent's children, do it now.
 			 */
 			dbg = p->p_pptr->p_pptr;
-			p->p_flag |= P_TRACED;
-			p->p_ptevents = PTRACE_DEFAULT;
-			p->p_oppid = p->p_pptr->p_pid;
+			proc_set_traced(p);
 			CTR2(KTR_PTRACE,
 		    "fork_return: attaching to new child pid %d: oppid %d",
 			    p->p_pid, p->p_oppid);
 			proc_reparent(p, dbg);
 			sx_xunlock(&proctree_lock);
-			td->td_dbgflags |= TDB_CHILD | TDB_SCX;
+			td->td_dbgflags |= TDB_CHILD | TDB_SCX | TDB_FSTP;
 			ptracestop(td, SIGSTOP);
 			td->td_dbgflags &= ~(TDB_CHILD | TDB_SCX);
 		} else {

Modified: head/sys/kern/kern_sig.c
==============================================================================
--- head/sys/kern/kern_sig.c	Thu Jul 28 06:46:10 2016	(r303422)
+++ head/sys/kern/kern_sig.c	Thu Jul 28 08:41:13 2016	(r303423)
@@ -2526,14 +2526,26 @@ ptracestop(struct thread *td, int sig)
 			PROC_SUNLOCK(p);
 			return (sig);
 		}
+
 		/*
-		 * Just make wait() to work, the last stopped thread
-		 * will win.
+		 * Make wait(2) work.  Ensure that right after the
+		 * attach, the thread which was decided to become the
+		 * leader of attach gets reported to the waiter.
+		 * Otherwise, just avoid overwriting another thread's
+		 * assignment to p_xthread.  If another thread has
+		 * already set p_xthread, the current thread will get
+		 * a chance to report itself upon the next iteration.
 		 */
-		p->p_xsig = sig;
-		p->p_xthread = td;
-		p->p_flag |= (P_STOPPED_SIG|P_STOPPED_TRACE);
-		sig_suspend_threads(td, p, 0);
+		if ((td->td_dbgflags & TDB_FSTP) != 0 ||
+		    ((p->p_flag & P2_PTRACE_FSTP) == 0 &&
+		    p->p_xthread == NULL)) {
+			p->p_xsig = sig;
+			p->p_xthread = td;
+			td->td_dbgflags &= ~TDB_FSTP;
+			p->p_flag2 &= ~P2_PTRACE_FSTP;
+			p->p_flag |= P_STOPPED_SIG | P_STOPPED_TRACE;
+			sig_suspend_threads(td, p, 0);
+		}
 		if ((td->td_dbgflags & TDB_STOPATFORK) != 0) {
 			td->td_dbgflags &= ~TDB_STOPATFORK;
 			cv_broadcast(&p->p_dbgwait);
@@ -2726,7 +2738,20 @@ issignal(struct thread *td)
 			SIG_STOPSIGMASK(sigpending);
 		if (SIGISEMPTY(sigpending))	/* no signal to send */
 			return (0);
-		sig = sig_ffs(&sigpending);
+		if ((p->p_flag & (P_TRACED | P_PPTRACE)) == P_TRACED &&
+		    (p->p_flag2 & P2_PTRACE_FSTP) != 0 &&
+		    SIGISMEMBER(sigpending, SIGSTOP)) {
+			/*
+			 * If debugger just attached, always consume
+			 * SIGSTOP from ptrace(PT_ATTACH) first, to
+			 * execute the debugger attach ritual in
+			 * order.
+			 */
+			sig = SIGSTOP;
+			td->td_dbgflags |= TDB_FSTP;
+		} else {
+			sig = sig_ffs(&sigpending);
+		}
 
 		if (p->p_stops & S_SIG) {
 			mtx_unlock(&ps->ps_mtx);
@@ -2743,7 +2768,7 @@ issignal(struct thread *td)
 			sigqueue_delete(&p->p_sigqueue, sig);
 			continue;
 		}
-		if (p->p_flag & P_TRACED && (p->p_flag & P_PPTRACE) == 0) {
+		if ((p->p_flag & (P_TRACED | P_PPTRACE)) == P_TRACED) {
 			/*
 			 * If traced, always stop.
 			 * Remove old signal from queue before the stop.
@@ -2846,6 +2871,8 @@ issignal(struct thread *td)
 				mtx_unlock(&ps->ps_mtx);
 				WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK,
 				    &p->p_mtx.lock_object, "Catching SIGSTOP");
+				sigqueue_delete(&td->td_sigqueue, sig);
+				sigqueue_delete(&p->p_sigqueue, sig);
 				p->p_flag |= P_STOPPED_SIG;
 				p->p_xsig = sig;
 				PROC_SLOCK(p);
@@ -2853,7 +2880,7 @@ issignal(struct thread *td)
 				thread_suspend_switch(td, p);
 				PROC_SUNLOCK(p);
 				mtx_lock(&ps->ps_mtx);
-				break;
+				goto next;
 			} else if (prop & SA_IGNORE) {
 				/*
 				 * Except for SIGCONT, shouldn't get here.
@@ -2884,6 +2911,7 @@ issignal(struct thread *td)
 		}
 		sigqueue_delete(&td->td_sigqueue, sig);	/* take the signal! */
 		sigqueue_delete(&p->p_sigqueue, sig);
+next:;
 	}
 	/* NOTREACHED */
 }

Modified: head/sys/kern/sys_process.c
==============================================================================
--- head/sys/kern/sys_process.c	Thu Jul 28 06:46:10 2016	(r303422)
+++ head/sys/kern/sys_process.c	Thu Jul 28 08:41:13 2016	(r303423)
@@ -692,6 +692,17 @@ sys_ptrace(struct thread *td, struct ptr
 #define	PROC_WRITE(w, t, a)	proc_write_ ## w (t, a)
 #endif
 
+void
+proc_set_traced(struct proc *p)
+{
+
+	PROC_LOCK_ASSERT(p, MA_OWNED);
+	p->p_flag |= P_TRACED;
+	p->p_flag2 |= P2_PTRACE_FSTP;
+	p->p_ptevents = PTRACE_DEFAULT;
+	p->p_oppid = p->p_pptr->p_pid;
+}
+
 int
 kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
 {
@@ -899,11 +910,9 @@ kern_ptrace(struct thread *td, int req, 
 	switch (req) {
 	case PT_TRACE_ME:
 		/* set my trace flag and "owner" so it can read/write me */
-		p->p_flag |= P_TRACED;
-		p->p_ptevents = PTRACE_DEFAULT;
+		proc_set_traced(p);
 		if (p->p_flag & P_PPWAIT)
 			p->p_flag |= P_PPTRACE;
-		p->p_oppid = p->p_pptr->p_pid;
 		CTR1(KTR_PTRACE, "PT_TRACE_ME: pid %d", p->p_pid);
 		break;
 
@@ -918,9 +927,7 @@ kern_ptrace(struct thread *td, int req, 
 		 * The old parent is remembered so we can put things back
 		 * on a "detach".
 		 */
-		p->p_flag |= P_TRACED;
-		p->p_ptevents = PTRACE_DEFAULT;
-		p->p_oppid = p->p_pptr->p_pid;
+		proc_set_traced(p);
 		if (p->p_pptr != td->td_proc) {
 			proc_reparent(p, td->td_proc);
 		}
@@ -1088,6 +1095,17 @@ kern_ptrace(struct thread *td, int req, 
 				    p->p_pid, data);
 			p->p_oppid = 0;
 			p->p_ptevents = 0;
+			FOREACH_THREAD_IN_PROC(p, td3) {
+				if ((td3->td_dbgflags & TDB_FSTP) != 0) {
+					sigqueue_delete(&td3->td_sigqueue,
+					    SIGSTOP);
+				}
+				td3->td_dbgflags &= ~(TDB_XSIG | TDB_FSTP);
+			}
+			if ((p->p_flag2 & P2_PTRACE_FSTP) != 0) {
+				sigqueue_delete(&p->p_sigqueue, SIGSTOP);
+				p->p_flag2 &= ~P2_PTRACE_FSTP;
+			}
 
 			/* should we send SIGCHLD? */
 			/* childproc_continued(p); */
@@ -1108,7 +1126,7 @@ kern_ptrace(struct thread *td, int req, 
 
 			if (req == PT_DETACH) {
 				FOREACH_THREAD_IN_PROC(p, td3)
-					td3->td_dbgflags &= ~TDB_SUSPEND; 
+					td3->td_dbgflags &= ~TDB_SUSPEND;
 			}
 			/*
 			 * unsuspend all threads, to not let a thread run,

Modified: head/sys/sys/proc.h
==============================================================================
--- head/sys/sys/proc.h	Thu Jul 28 06:46:10 2016	(r303422)
+++ head/sys/sys/proc.h	Thu Jul 28 08:41:13 2016	(r303423)
@@ -423,6 +423,7 @@ do {									\
 #define	TDB_BORN	0x00000200 /* New LWP indicator for ptrace() */
 #define	TDB_EXIT	0x00000400 /* Exiting LWP indicator for ptrace() */
 #define	TDB_VFORK	0x00000800 /* vfork indicator for ptrace() */
+#define	TDB_FSTP	0x00001000 /* The thread is PT_ATTACH leader */
 
 /*
  * "Private" flags kept in td_pflags:
@@ -713,6 +714,7 @@ struct proc {
 #define	P2_NOTRACE	0x00000002	/* No ptrace(2) attach or coredumps. */
 #define	P2_NOTRACE_EXEC 0x00000004	/* Keep P2_NOPTRACE on exec(2). */
 #define	P2_AST_SU	0x00000008	/* Handles SU ast for kthreads. */
+#define	P2_PTRACE_FSTP	0x00000010 /* SIGSTOP from PT_ATTACH not yet handled. */
 
 /* Flags protected by proctree_lock, kept in p_treeflags. */
 #define	P_TREE_ORPHANED		0x00000001	/* Reparented, on orphan list */
@@ -1003,6 +1005,7 @@ void	proc_linkup(struct proc *p, struct 
 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);
+void	proc_set_traced(struct proc *p);
 struct	pstats *pstats_alloc(void);
 void	pstats_fork(struct pstats *src, struct pstats *dst);
 void	pstats_free(struct pstats *ps);


More information about the svn-src-all mailing list