From b7a25e63b6fc922b197e9daf4c7f545c3560b8fa Mon Sep 17 00:00:00 2001 From: Konstantin Belousov Date: Thu, 28 Jul 2016 08:41:13 +0000 Subject: [PATCH] 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 --- bin/ps/ps.1 | 3 ++- sys/kern/kern_exit.c | 7 +++++-- sys/kern/kern_fork.c | 6 ++---- sys/kern/kern_sig.c | 46 +++++++++++++++++++++++++++++++++--------- sys/kern/sys_process.c | 32 ++++++++++++++++++++++------- sys/sys/proc.h | 3 +++ 6 files changed, 74 insertions(+), 23 deletions(-) diff --git a/bin/ps/ps.1 b/bin/ps/ps.1 index 3d7c7382cab..ea094bccbcb 100644 --- a/bin/ps/ps.1 +++ b/bin/ps/ps.1 @@ -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. diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c index 3b7b9daeafa..125f210ba34 100644 --- a/sys/kern/kern_exit.c +++ b/sys/kern/kern_exit.c @@ -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); diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c index 3cf4edc5722..4c21c0ba20d 100644 --- a/sys/kern/kern_fork.c +++ b/sys/kern/kern_fork.c @@ -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 { diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index 2ac6fcf9038..4aa064a3838 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -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 */ } diff --git a/sys/kern/sys_process.c b/sys/kern/sys_process.c index b081844a05f..7d2fd72120d 100644 --- a/sys/kern/sys_process.c +++ b/sys/kern/sys_process.c @@ -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, diff --git a/sys/sys/proc.h b/sys/sys/proc.h index b80ddf03eb1..61c78d9916f 100644 --- a/sys/sys/proc.h +++ b/sys/sys/proc.h @@ -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); -- 2.45.0