d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix a race where timeout/untimeout could cause crashes for Giant locked

Browse Source 
code.

The bug:

There exists a race condition for timeout/untimeout(9) due to the
way that the softclock thread dequeues timeouts.

The softclock thread sets the c_func and c_arg of the callout to
NULL while holding the callout lock but not Giant.  It then drops
the callout lock and acquires Giant.

It is at this point where untimeout(9) on another cpu/thread could
be called.

Since c_arg and c_func are cleared, untimeout(9) does not touch the
callout and returns as if the callout is canceled.

The softclock then tries to acquire Giant and likely blocks due to
the other cpu/thread holding it.

The other cpu/thread then likely deallocates the backing store that
c_arg points to and finishes working and hence drops Giant.

Softclock resumes and acquires giant and calls the function with
the now free'd c_arg and we have corruption/crash.

The fix:

We need to track curr_callout even for timeout(9) (LOCAL_ALLOC)
callouts.  We need to free the callout after the softclock processes
it to deal with the race here.

Obtained from: Juniper Networks, iedowse
Reviewed by: jhb, iedowse
MFC After: 2 weeks.
This commit is contained in:
Alfred Perlstein 2008-03-22 07:29:45 +00:00
parent 20b94d8035
commit 435cdf88ea
1 changed files with 19 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
sys/kern/kern_timeout.c
View File

@ -230,11 +230,8 @@ softclock(void *dummy)
c_arg = c->c_arg;
c_flags = c->c_flags;
if (c->c_flags & CALLOUT_LOCAL_ALLOC) {
c->c_func = NULL;
c->c_flags = CALLOUT_LOCAL_ALLOC;
SLIST_INSERT_HEAD(&callfree, c,
c_links.sle);
curr_callout = NULL;
curr_callout = c;
} else {
c->c_flags =
(c->c_flags & ~CALLOUT_PENDING);
@ -299,6 +296,24 @@ softclock(void *dummy)
class->lc_unlock(c_lock);
skip:
mtx_lock_spin(&callout_lock);
/*
* If the current callout is locally
* allocated (from timeout(9))
* then put it on the freelist.
*
* Note: we need to check the cached
* copy of c_flags because if it was not
* local, then it's not safe to deref the
* callout pointer.
*/
if (c_flags & CALLOUT_LOCAL_ALLOC) {
KASSERT(c->c_flags ==
CALLOUT_LOCAL_ALLOC,
("corrupted callout"));
c->c_func = NULL;
SLIST_INSERT_HEAD(&callfree, c,
c_links.sle);
}
curr_callout = NULL;
if (callout_wait) {
/*