Race in kevent

[Harti Brandt]

On Wed, 9 Jul 2003, Eric Jacobs wrote:

EJ>On Wed, 9 Jul 2003 15:28:38 +0200 (CEST)
EJ>
EJ>I didn't think of it in my original post, but perhaps we need a
EJ>"thissoftcheck" pointer that works analogously to "nextsoftcheck",
EJ>except that instead of being advanced to the next entry in the queue,
EJ>it is simply zeroed out when the entry is removed. softclock() could
EJ>detect this pointer being zeroed out just before it goes to call the
EJ>callout or timeout function and skip that invocation if that is the
EJ>case.
EJ>
EJ>This is definitely not a solution in the CALLOUT_MPSAFE case, however,
EJ>because it would make no sense to try to verify this pointer in the
EJ>unprotected area between the spin lock being dropped and the sleep
EJ>lock being picked up. In the !CALLOUT_MPSAFE case, we know what the
EJ>sleep mutex would be -- it is always Giant -- and so we can test the
EJ>pointer after that point.
EJ>
EJ>I am still not certain I am thinking clearly about this.

I think for the MPSAFE case we should just make the callout flags to be correct
(look at CALLOUT_ACTIVE) so that a caller has a chance to find out the current
state of the callout. Otherwise the user of an MPSAFE callout should avoid
races. This means: clearing CALLOUT_ACTIVE in softclock() after the callout
has returned and the lock has been re-aquired. Probably returning without
doing anything from callout_stop() when it finds the callout to be non-pending
but active.

For the !MPSAFE case, however, we should do something. I think your idea
would work. The only problem I can see are wrong uses of callouts allocated
by timeout() (for an example see dev/ray/if_ray.c). These should be fixed
in the callers of timeout(9).

Could you craft a patch? I would try it.

harti
-- 
harti brandt,
http://www.fokus.fraunhofer.de/research/cc/cats/employees/hartmut.brandt/private
brandt at fokus.fraunhofer.de, harti at freebsd.org