d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

sleepq_catch_signals: do thread suspension before signal check

Browse Source 
Since locks are dropped when a thread suspends, it's possible for another
thread to deliver a signal to the suspended thread. If the thread awakens from
suspension without checking for signals, it may go to sleep despite having
a pending signal that should wake it up. Therefore the suspension check is
done first, so any signals sent while suspended will be caught in the
subsequent signal check.

Reviewed by:	kib
Approved by:	kib (mentor)
MFC after:	2 weeks
Sponsored by:	Dell EMC
Differential Revision:	https://reviews.freebsd.org/D9530
This commit is contained in:
Eric Badger 2017-02-14 17:13:23 +00:00
parent 111142bcf1
commit 28d2efa983
1 changed files with 57 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

101
sys/kern/subr_sleepqueue.c
View File

@ -430,6 +430,7 @@ sleepq_catch_signals(void *wchan, int pri)
struct sigacts *ps;
int sig, ret;
ret = 0;
td = curthread;
p = curproc;
sc = SC_LOOKUP(wchan);
@ -443,53 +444,65 @@ sleepq_catch_signals(void *wchan, int pri)
}
/*
* See if there are any pending signals for this thread. If not
* we can switch immediately. Otherwise do the signal processing
* directly.
* See if there are any pending signals or suspension requests for this
* thread. If not, we can switch immediately.
*/
thread_lock(td);
if ((td->td_flags & (TDF_NEEDSIGCHK | TDF_NEEDSUSPCHK)) == 0) {
sleepq_switch(wchan, pri);
return (0);
if ((td->td_flags & (TDF_NEEDSIGCHK | TDF_NEEDSUSPCHK)) != 0) {
thread_unlock(td);
mtx_unlock_spin(&sc->sc_lock);
CTR3(KTR_PROC, "sleepq catching signals: thread %p (pid %ld, %s)",
(void *)td, (long)p->p_pid, td->td_name);
PROC_LOCK(p);
/*
* Check for suspension first. Checking for signals and then
* suspending could result in a missed signal, since a signal
* can be delivered while this thread is suspended.
*/
if ((td->td_flags & TDF_NEEDSUSPCHK) != 0) {
ret = thread_suspend_check(1);
MPASS(ret == 0 || ret == EINTR || ret == ERESTART);
if (ret != 0) {
PROC_UNLOCK(p);
mtx_lock_spin(&sc->sc_lock);
thread_lock(td);
goto out;
}
}
if ((td->td_flags & TDF_NEEDSIGCHK) != 0) {
ps = p->p_sigacts;
mtx_lock(&ps->ps_mtx);
sig = cursig(td);
if (sig == -1) {
mtx_unlock(&ps->ps_mtx);
KASSERT((td->td_flags & TDF_SBDRY) != 0,
("lost TDF_SBDRY"));
KASSERT(TD_SBDRY_INTR(td),
("lost TDF_SERESTART of TDF_SEINTR"));
KASSERT((td->td_flags &
(TDF_SEINTR | TDF_SERESTART)) !=
(TDF_SEINTR | TDF_SERESTART),
("both TDF_SEINTR and TDF_SERESTART"));
ret = TD_SBDRY_ERRNO(td);
} else if (sig != 0) {
ret = SIGISMEMBER(ps->ps_sigintr, sig) ?
EINTR : ERESTART;
mtx_unlock(&ps->ps_mtx);
} else {
mtx_unlock(&ps->ps_mtx);
}
}
/*
* Lock the per-process spinlock prior to dropping the PROC_LOCK
* to avoid a signal delivery race. PROC_LOCK, PROC_SLOCK, and
* thread_lock() are currently held in tdsendsignal().
*/
PROC_SLOCK(p);
mtx_lock_spin(&sc->sc_lock);
PROC_UNLOCK(p);
thread_lock(td);
PROC_SUNLOCK(p);
}
thread_unlock(td);
mtx_unlock_spin(&sc->sc_lock);
CTR3(KTR_PROC, "sleepq catching signals: thread %p (pid %ld, %s)",
(void *)td, (long)p->p_pid, td->td_name);
PROC_LOCK(p);
ps = p->p_sigacts;
mtx_lock(&ps->ps_mtx);
sig = cursig(td);
if (sig == -1) {
mtx_unlock(&ps->ps_mtx);
KASSERT((td->td_flags & TDF_SBDRY) != 0, ("lost TDF_SBDRY"));
KASSERT(TD_SBDRY_INTR(td),
("lost TDF_SERESTART of TDF_SEINTR"));
KASSERT((td->td_flags & (TDF_SEINTR | TDF_SERESTART)) !=
(TDF_SEINTR | TDF_SERESTART),
("both TDF_SEINTR and TDF_SERESTART"));
ret = TD_SBDRY_ERRNO(td);
} else if (sig == 0) {
mtx_unlock(&ps->ps_mtx);
ret = thread_suspend_check(1);
MPASS(ret == 0 || ret == EINTR || ret == ERESTART);
} else {
if (SIGISMEMBER(ps->ps_sigintr, sig))
ret = EINTR;
else
ret = ERESTART;
mtx_unlock(&ps->ps_mtx);
}
/*
* Lock the per-process spinlock prior to dropping the PROC_LOCK
* to avoid a signal delivery race. PROC_LOCK, PROC_SLOCK, and
* thread_lock() are currently held in tdsendsignal().
*/
PROC_SLOCK(p);
mtx_lock_spin(&sc->sc_lock);
PROC_UNLOCK(p);
thread_lock(td);
PROC_SUNLOCK(p);
if (ret == 0) {
sleepq_switch(wchan, pri);
return (0);