Date: Thu, 28 Jul 2016 08:41:13 +0000 (UTC) From: Konstantin Belousov <kib@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r303423 - in head: bin/ps sys/kern sys/sys Message-ID: <201607280841.u6S8fDj1057663@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201607280841.u6S8fDj1057663>