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.

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>
(cherry picked from commit 22cba8e9702f3673c335bf834a9ee2c5e5fd9b6e)

Change-Id: I82c6480859a85e00e8602421204dac9a9ce588ed
Reviewed-on: http://gerrit.openafs.org/6823
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com>
Tested-by: Jeffrey Altman <jaltman@secure-endpoints.com>
This commit is contained in:
Jeffrey Altman 2012-01-19 15:25:44 -05:00 committed by Jeffrey Altman
parent e42640ec10
commit 25b96acb42

View File

@ -179,33 +179,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)) {
osi_QAddH( (osi_queue_t **) &cm_data.buf_freeListp,
(osi_queue_t **) &cm_data.buf_freeListEndp,
&bp->q);
_InterlockedOr(&bp->qFlags, CM_BUF_QINLRU);
}
lock_ReleaseWrite(&buf_globalLock);
}
lock_ObtainRead(&buf_globalLock);
buf_ReleaseLocked(bp, FALSE);
lock_ReleaseRead(&buf_globalLock);
}
long