freebsd_network: Handle an edge-case race of callout_stop.

If callout_stop() runs at the same time the callout_thread is
about to process the callout, we can wind up with a situation where
the callout is "active" but has not yet run due to waiting on the mutex.

FreeBSD's documentation confirms that callout_stop must be called
(when "safe" is 0, i.e. not callout_drain) with the callout's lock
held, if it has one, and so we can prevent the callout from being
invoked at this point, too.

Additionally, fix return values of callout_reset.

May further help with #18315.
This commit is contained in:
Augustin Cavalier
2023-03-29 11:37:02 -04:00
parent d86a240aec
commit d2670b490d
+23 -5
View File
@@ -57,20 +57,29 @@ callout_thread(void* /*data*/)
// execute timer // execute timer
list_remove_item(&sTimers, c); list_remove_item(&sTimers, c);
c->c_due = -1; if (mutex == NULL)
c->c_due = -1;
sCurrentCallout = c; sCurrentCallout = c;
mutex_unlock(&sLock); mutex_unlock(&sLock);
if (mutex != NULL) if (mutex != NULL) {
mtx_lock(mutex); mtx_lock(mutex);
if (c->c_due < 0) {
mtx_unlock(mutex);
goto done;
}
c->c_due = -1;
}
c->c_func(c->c_arg); c->c_func(c->c_arg);
if (mutex != NULL if (mutex != NULL
&& (c->c_flags & CALLOUT_RETURNUNLOCKED) == 0) && (c->c_flags & CALLOUT_RETURNUNLOCKED) == 0)
mtx_unlock(mutex); mtx_unlock(mutex);
done:
if ((status = mutex_lock(&sLock)) != B_OK) if ((status = mutex_lock(&sLock)) != B_OK)
continue; continue;
@@ -176,7 +185,7 @@ callout_init_mtx(struct callout *c, struct mtx *mtx, int flags)
int int
callout_reset(struct callout *c, int _ticks, void (*func)(void *), void *arg) callout_reset(struct callout *c, int _ticks, void (*func)(void *), void *arg)
{ {
int canceled = callout_stop(c); int cancelled = callout_stop(c);
MutexLocker locker(sLock); MutexLocker locker(sLock);
@@ -197,7 +206,7 @@ callout_reset(struct callout *c, int _ticks, void (*func)(void *), void *arg)
release_sem(sWaitSem); release_sem(sWaitSem);
} }
return canceled; return (cancelled == -1) ? 0 : 1;
} }
@@ -220,13 +229,22 @@ _callout_stop_safe(struct callout *c, int safe)
int ret = -1; int ret = -1;
if (callout_active(c)) { if (callout_active(c)) {
ret = 0;
if (!safe && c->c_mtx != NULL && c->c_due > 0) {
mtx_assert(c->c_mtx, MA_OWNED);
// The callout is active, but c_due > 0 and we hold the locks: this
// means the callout thread has dequeued it and is waiting for c_mtx.
// Clear c_due to signal the callout thread.
c->c_due = -1;
ret = 1;
}
if (safe) { if (safe) {
locker.Unlock(); locker.Unlock();
while (callout_active(c)) while (callout_active(c))
snooze(100); snooze(100);
locker.Lock(); locker.Lock();
} }
ret = 0;
} }
if (c->c_due <= 0) if (c->c_due <= 0)