diff options
| author | Mark Johnston <markj@FreeBSD.org> | 2016-01-09 01:56:46 +0000 |
|---|---|---|
| committer | Mark Johnston <markj@FreeBSD.org> | 2016-01-09 01:56:46 +0000 |
| commit | 11f9ca696ee12e1732dd2f15dd8660e0689194b6 (patch) | |
| tree | f32fa1b0a9db03f6940f0fa00eaae32f3c3a6e34 /sys/kern/kern_condvar.c | |
| parent | fce8d0e350a77330a34844fee43103cf163377bb (diff) | |
| download | src-11f9ca696ee12e1732dd2f15dd8660e0689194b6.tar.gz src-11f9ca696ee12e1732dd2f15dd8660e0689194b6.zip | |
Prevent cv_waiters wraparound.
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
Notes
Notes:
svn path=/head/; revision=293458
Diffstat (limited to 'sys/kern/kern_condvar.c')
| -rw-r--r-- | sys/kern/kern_condvar.c | 33 |
1 files changed, 26 insertions, 7 deletions
diff --git a/sys/kern/kern_condvar.c b/sys/kern/kern_condvar.c index 2700a25d477c..95a6d09e1775 100644 --- a/sys/kern/kern_condvar.c +++ b/sys/kern/kern_condvar.c @@ -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> @@ -47,6 +48,17 @@ __FBSDID("$FreeBSD$"); #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. */ #define CV_ASSERT(cvp, lock, td) do { \ @@ -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) |