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
This commit is contained in:
Konstantin Belousov 2016-07-28 08:41:13 +00:00
parent 7d8ee480c4
commit b7a25e63b6
6 changed files with 74 additions and 23 deletions

View File

@ -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.

View File

@ -476,9 +476,12 @@ exit1(struct thread *td, int rval, int signo)
*/
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);

View File

@ -1074,15 +1074,13 @@ fork_return(struct thread *td, struct trapframe *frame)
* 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 {

View File

@ -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 */
}

View File

@ -692,6 +692,17 @@ sys_ptrace(struct thread *td, struct ptrace_args *uap)
#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, pid_t pid, void *addr, int data)
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, pid_t pid, void *addr, int data)
* 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, pid_t pid, void *addr, int data)
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, pid_t pid, void *addr, int data)
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,

View File

@ -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 thread *td);
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);