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

Prevent cv_waiters wraparound.

Browse Source 
r282971 attempted to fix this problem by decrementing cv_waiters after
waking up from sleeping on a condition variable, but this can result in
a use-after-free if the CV is freed before all woken threads have had a
chance to run. Instead, avoid incrementing cv_waiters past INT_MAX, and
have cv_signal() explicitly check for sleeping threads once cv_waiters has
reached this bound.

Reviewed by:	jhb
MFC after:	2 weeks
Sponsored by:	EMC / Isilon Storage Division
Differential Revision:	https://reviews.freebsd.org/D4822
This commit is contained in:
markj 2016-01-09 01:56:46 +00:00
parent eeaf13194e
commit e38d62e90d
1 changed files with 26 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
sys/kern/kern_condvar.c
View File

@ -31,6 +31,7 @@ __FBSDID("$FreeBSD$");
#include <sys/param.h>
#include <sys/systm.h>
#include <sys/limits.h>
#include <sys/lock.h>
#include <sys/mutex.h>
#include <sys/proc.h>
@ -46,6 +47,17 @@ __FBSDID("$FreeBSD$");
#include <sys/ktrace.h>
#endif
/*
* A bound below which cv_waiters is valid. Once cv_waiters reaches this bound,
* cv_signal must manually check the wait queue for threads.
*/
#define CV_WAITERS_BOUND INT_MAX
#define CV_WAITERS_INC(cvp) do { \
if ((cvp)->cv_waiters < CV_WAITERS_BOUND) \
(cvp)->cv_waiters++; \
} while (0)
/*
* Common sanity checks for cv_wait* functions.
*/
@ -122,7 +134,7 @@ _cv_wait(struct cv *cvp, struct lock_object *lock)
sleepq_lock(cvp);
cvp->cv_waiters++;
CV_WAITERS_INC(cvp);
if (lock == &Giant.lock_object)
mtx_assert(&Giant, MA_OWNED);
DROP_GIANT();
@ -184,7 +196,7 @@ _cv_wait_unlock(struct cv *cvp, struct lock_object *lock)
sleepq_lock(cvp);
cvp->cv_waiters++;
CV_WAITERS_INC(cvp);
DROP_GIANT();
sleepq_add(cvp, lock, cvp->cv_description, SLEEPQ_CONDVAR, 0);
@ -240,7 +252,7 @@ _cv_wait_sig(struct cv *cvp, struct lock_object *lock)
sleepq_lock(cvp);
cvp->cv_waiters++;
CV_WAITERS_INC(cvp);
if (lock == &Giant.lock_object)
mtx_assert(&Giant, MA_OWNED);
DROP_GIANT();
@ -307,7 +319,7 @@ _cv_timedwait_sbt(struct cv *cvp, struct lock_object *lock, sbintime_t sbt,
sleepq_lock(cvp);
cvp->cv_waiters++;
CV_WAITERS_INC(cvp);
if (lock == &Giant.lock_object)
mtx_assert(&Giant, MA_OWNED);
DROP_GIANT();
@ -376,7 +388,7 @@ _cv_timedwait_sig_sbt(struct cv *cvp, struct lock_object *lock,
sleepq_lock(cvp);
cvp->cv_waiters++;
CV_WAITERS_INC(cvp);
if (lock == &Giant.lock_object)
mtx_assert(&Giant, MA_OWNED);
DROP_GIANT();
@ -422,8 +434,15 @@ cv_signal(struct cv *cvp)
wakeup_swapper = 0;
sleepq_lock(cvp);
if (cvp->cv_waiters > 0) {
cvp->cv_waiters--;
wakeup_swapper = sleepq_signal(cvp, SLEEPQ_CONDVAR, 0, 0);
if (cvp->cv_waiters == CV_WAITERS_BOUND &&
sleepq_lookup(cvp) == NULL) {
cvp->cv_waiters = 0;
} else {
if (cvp->cv_waiters < CV_WAITERS_BOUND)
cvp->cv_waiters--;
wakeup_swapper = sleepq_signal(cvp, SLEEPQ_CONDVAR, 0,
0);
}
}
sleepq_release(cvp);
if (wakeup_swapper)