From a33fef5e25ac16d545343f4fb500afc82bb082f3 Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Thu, 19 Nov 2020 18:37:28 +0000 Subject: [PATCH] callout(9): Fix a race between CPU migration and callout_drain() Suppose a running callout re-arms itself, and before the callout finishes running another CPU calls callout_drain() and goes to sleep. softclock_call_cc() will wake up the draining thread, which may not run immediately if there is a lot of CPU load. Furthermore, the callout is still in the callout wheel so it can continue to run and re-arm itself. Then, suppose that the callout migrates to another CPU before the draining thread gets a chance to run. The draining thread is in this loop in _callout_stop_safe(): while (cc_exec_curr(cc) == c) { CC_UNLOCK(cc); sleep(); CC_LOCK(cc); } but after the migration, cc points to the wrong CPU's callout state. Then the draining thread goes off and removes the callout from the wheel, but does so using the wrong lock and per-CPU callout state. Fix the problem by doing a re-lookup of the callout CPU after sleeping. Reported by: syzbot+79569cd4d76636b2cc1c@syzkaller.appspotmail.com Reported by: syzbot+1b27e0237aa22d8adffa@syzkaller.appspotmail.com Reported by: syzbot+e21aa5b85a9aff90ef3e@syzkaller.appspotmail.com Reviewed by: emaste, hselasky Tested by: pho MFC after: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D27266 --- sys/kern/kern_timeout.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sys/kern/kern_timeout.c b/sys/kern/kern_timeout.c index f242c92adf1..5f9fd7542a9 100644 --- a/sys/kern/kern_timeout.c +++ b/sys/kern/kern_timeout.c @@ -1145,7 +1145,7 @@ _callout_stop_safe(struct callout *c, int flags, callout_func_t *drain) * just wait for the current invocation to * finish. */ - while (cc_exec_curr(cc, direct) == c) { + if (cc_exec_curr(cc, direct) == c) { /* * Use direct calls to sleepqueue interface * instead of cv/msleep in order to avoid @@ -1193,7 +1193,7 @@ _callout_stop_safe(struct callout *c, int flags, callout_func_t *drain) /* Reacquire locks previously released. */ PICKUP_GIANT(); - CC_LOCK(cc); + goto again; } c->c_flags &= ~CALLOUT_ACTIVE; } else if (use_lock && -- 2.45.0