Page MenuHomeFreeBSD

callout(9): Fix a race between migration and callout_drain()
ClosedPublic

Authored by markj on Wed, Nov 18, 2:10 PM.

Details

Summary

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.
This is a minimal patch intended to be suitable for backporting.

Test Plan

syzkaller found this bug using kevent() and EVFILT_TIMER.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markj requested review of this revision.Wed, Nov 18, 2:10 PM
markj created this revision.
markj added reviewers: hselasky, rrs.
hselasky added inline comments.
sys/kern/kern_timeout.c
1164 ↗(On Diff #79699)

There is no need for a while () statement here. Change it into an if (). The block is always exited via a goto.

This revision is now accepted and ready to land.Wed, Nov 18, 2:23 PM
markj marked an inline comment as done.

while -> if

This revision now requires review to proceed.Wed, Nov 18, 2:30 PM
This revision is now accepted and ready to land.Wed, Nov 18, 5:58 PM