windows-callback-race-20050427

cm_EndCallbackGrantingCall contained a race condition due to the release
of the cm_callbackLock in the middle of the for() loop.  The race was
removed by optimizing out the call to cm_CallbackNotifyChange().  There
is no reason this needed to be called once per callback revoke in the
list.

diff --git a/src/WINNT/afsd/cm_callback.c b/src/WINNT/afsd/cm_callback.c
index 37771db..e572697 100644 (file)
--- a/src/WINNT/afsd/cm_callback.c
+++ b/src/WINNT/afsd/cm_callback.c
@@ -1499,6 +1499,7 @@ void cm_EndCallbackGrantingCall(cm_scache_t *scp, cm_callbackRequest_t *cbrp,
     cm_racingRevokes_t *nrevp;         /* where we'll be next */
     int freeFlag;
     cm_server_t * serverp = 0;
+    int discardScp = 0;
 
     lock_ObtainWrite(&cm_callbackLock);
     if (flags & CM_CALLBACK_MAINTAINCOUNT) {
@@ -1561,16 +1562,7 @@ void cm_EndCallbackGrantingCall(cm_scache_t *scp, cm_callbackRequest_t *cbrp,
                       scp,
                       cbrp->callbackCount, revp->callbackCount,
                       cm_callbackCount);
-            cm_DiscardSCache(scp);
-            /*
-             * Since we don't have a callback to preserve, it's
-             * OK to drop the lock and re-obtain it.
-             */
-            lock_ReleaseMutex(&scp->mx);
-            lock_ReleaseWrite(&cm_callbackLock);
-            cm_CallbackNotifyChange(scp);
-            lock_ObtainMutex(&scp->mx);
-            lock_ObtainWrite(&cm_callbackLock);
+            discardScp = 1;
         }
         if (freeFlag) 
             free(revp);
@@ -1582,6 +1574,13 @@ void cm_EndCallbackGrantingCall(cm_scache_t *scp, cm_callbackRequest_t *cbrp,
 
     lock_ReleaseWrite(&cm_callbackLock);
 
+    if ( discardScp ) {
+        cm_DiscardSCache(scp);
+        lock_ReleaseMutex(&scp->mx);
+        cm_CallbackNotifyChange(scp);
+        lock_ObtainMutex(&scp->mx);
+    }
+
     if ( serverp ) {
         lock_ObtainWrite(&cm_serverLock);
         cm_FreeServer(serverp);