Windows: cm_buf refcnt must hold buf_globalLock

An assertion in buf_Recycle() was being triggered when a cm_buf_t
object was supposed to be in the free buffer list but wasn't.
buf_Recycle() was racing with another thread.  The test for
refCount == 0 was performed while holding the buf_globalLock
exclusively but the InterlockedDecrement(refCount) in buf_Release()
was performed without holding buf_globalLock at all.  buf_globalLOck
must be held at least as a read lock.  Otherwise, the refCount can
reach 0 prior to the thread blocking for exclusive access to the
buf_globalLock.  This provides buf_Recycle() which is holding
buf_globalLock the opportunity to race.

The solution is to make sure that buf_Release() always holds
buf_globalLock as a read lock and then use buf_ReleaseLocked()
to perform the actual decrement and test.

Change-Id: Ieb67548a7e44fa5f06f9346f428b1edadfc80696
Reviewed-on: http://gerrit.openafs.org/6576
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com>
Tested-by: Jeffrey Altman <jaltman@secure-endpoints.com>

diff --git a/src/WINNT/afsd/cm_buf.c b/src/WINNT/afsd/cm_buf.c
index 7283271..4d8833e 100644 (file)
--- a/src/WINNT/afsd/cm_buf.c
+++ b/src/WINNT/afsd/cm_buf.c
@@ -188,34 +188,9 @@ void buf_ReleaseDbg(cm_buf_t *bp, char *file, long line)
 void buf_Release(cm_buf_t *bp)
 #endif
 {
-    afs_int32 refCount;
-
-    /* ensure that we're in the LRU queue if our ref count is 0 */
-    osi_assertx(bp->magic == CM_BUF_MAGIC,"incorrect cm_buf_t magic");
-
-    refCount = InterlockedDecrement(&bp->refCount);
-#ifdef DEBUG_REFCOUNT
-    osi_Log2(afsd_logp,"buf_Release bp 0x%p ref %d", bp, refCount);
-    afsi_log("%s:%d buf_ReleaseLocked bp 0x%p, ref %d", file, line, bp, refCount);
-#endif
-#ifdef DEBUG
-    if (refCount < 0)
-       osi_panic("buf refcount 0",__FILE__,__LINE__);;
-#else
-    osi_assertx(refCount >= 0, "cm_buf_t refCount == 0");
-#endif
-    if (refCount == 0) {
-        lock_ObtainWrite(&buf_globalLock);
-        if (bp->refCount == 0 &&
-            !(bp->qFlags & (CM_BUF_QINLRU|CM_BUF_QREDIR))) {
-            osi_QAddH( (osi_queue_t **) &cm_data.buf_freeListp,
-                       (osi_queue_t **) &cm_data.buf_freeListEndp,
-                       &bp->q);
-            _InterlockedOr(&bp->qFlags, CM_BUF_QINLRU);
-            buf_IncrementFreeCount();
-        }
-        lock_ReleaseWrite(&buf_globalLock);
-    }
+    lock_ObtainRead(&buf_globalLock);
+    buf_ReleaseLocked(bp, FALSE);
+    lock_ReleaseRead(&buf_globalLock);
 }
 
 long