Date: Wed, 13 Jul 2016 07:54:39 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Mark Johnston <markj@FreeBSD.org> Cc: freebsd-current@FreeBSD.org Subject: Re: ptrace attach in multi-threaded processes Message-ID: <20160713045439.GT38613@kib.kiev.ua> In-Reply-To: <20160713040210.GA89573@wkstn-mjohnston.west.isilon.com> References: <20160712011938.GA51319@wkstn-mjohnston.west.isilon.com> <20160712055753.GI38613@kib.kiev.ua> <20160712170502.GA71220@wkstn-mjohnston.west.isilon.com> <20160712175150.GP38613@kib.kiev.ua> <20160712182414.GC71220@wkstn-mjohnston.west.isilon.com> <20160713033036.GR38613@kib.kiev.ua> <20160713040210.GA89573@wkstn-mjohnston.west.isilon.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Jul 12, 2016 at 09:02:10PM -0700, Mark Johnston wrote:
> Hm, the only SIGSTOP in my scenario is the one generated by PT_ATTACH.
> The problem occurs when this SIGSTOP races with *any* other signal that's
> being delivered to the process and for which the process has registered
> a handler. For instance, SIGHUP after a log rotation.
>
> If a real SIGSTOP races with PT_ATTACH, then I would indeed expect to
> find the process in a stopped state after the detach. Does this make
> more sense?
I finally see. Might be something like the patch below is a step in
the desired direction. Idea is in the proc_next_xthread(): p_xthread
should be set to the next thread with a pending signal. Do you have a
test case that demonstrates the issue ?
diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c
index 2a5e6de..1106f3a 100644
--- a/sys/kern/kern_sig.c
+++ b/sys/kern/kern_sig.c
@@ -2525,22 +2525,21 @@ ptracestop(struct thread *td, int sig)
PROC_SUNLOCK(p);
return (sig);
}
- /*
- * Just make wait() to work, the last stopped thread
- * will win.
- */
- 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_STOPATFORK) != 0) {
- td->td_dbgflags &= ~TDB_STOPATFORK;
- cv_broadcast(&p->p_dbgwait);
+ if (p->p_xthread == NULL)
+ p->p_xthread = td;
+ if (p->p_xthread == td) {
+ p->p_xsig = sig;
+ 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);
+ }
}
stopme:
thread_suspend_switch(td, p);
if (p->p_xthread == td)
- p->p_xthread = NULL;
+ proc_next_xthread(p);
if (!(p->p_flag & P_TRACED))
break;
if (td->td_dbgflags & TDB_SUSPEND) {
diff --git a/sys/kern/sys_process.c b/sys/kern/sys_process.c
index a6037e3..3d9950b 100644
--- a/sys/kern/sys_process.c
+++ b/sys/kern/sys_process.c
@@ -1057,7 +1057,7 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
proctree_locked = 0;
}
p->p_xsig = data;
- p->p_xthread = NULL;
+ proc_next_xthread(p);
if ((p->p_flag & (P_STOPPED_SIG | P_STOPPED_TRACE)) != 0) {
/* deliver or queue signal */
td2->td_dbgflags &= ~TDB_XSIG;
@@ -1065,7 +1065,8 @@ 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 |
+ TDB_XSIG);
}
/*
* unsuspend all threads, to not let a thread run,
@@ -1376,9 +1377,24 @@ stopevent(struct proc *p, unsigned int event, unsigned int val)
do {
if (event != S_EXIT)
p->p_xsig = val;
- p->p_xthread = NULL;
+ proc_next_xthread(p);
p->p_stype = event; /* Which event caused the stop? */
wakeup(&p->p_stype); /* Wake up any PIOCWAIT'ing procs */
msleep(&p->p_step, &p->p_mtx, PWAIT, "stopevent", 0);
} while (p->p_step);
}
+
+void
+proc_next_xthread(struct proc *p)
+{
+ struct thread *td;
+
+ PROC_LOCK_ASSERT(p, MA_OWNED);
+ FOREACH_THREAD_IN_PROC(p, td) {
+ if (td == p->p_xthread)
+ continue;
+ if ((td->td_dbgflags & TDB_XSIG) != 0)
+ break;
+ }
+ p->p_xthread = td;
+}
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index f533db6..a3132d9 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -999,6 +999,7 @@ int proc_getenvv(struct thread *td, struct proc *p, struct sbuf *sb);
void procinit(void);
void proc_linkup0(struct proc *p, struct thread *td);
void proc_linkup(struct proc *p, struct thread *td);
+void proc_next_xthread(struct proc *p);
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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160713045439.GT38613>