Author: markj Date: Sat Jan 9 01:56:46 2016 New Revision: 293458 URL: https://svnweb.freebsd.org/changeset/base/293458
Log: 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 Modified: head/sys/kern/kern_condvar.c Modified: head/sys/kern/kern_condvar.c ============================================================================== --- head/sys/kern/kern_condvar.c Sat Jan 9 01:43:53 2016 (r293457) +++ head/sys/kern/kern_condvar.c Sat Jan 9 01:56:46 2016 (r293458) @@ -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_obj 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 l 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 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 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, st 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) _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
