Windows: buf_CleanAsync scp->fid == bp->fid

If buf_CleanAsync or buf_CleanAsyncLocked are called
with a non-NULL cm_scache_t parameter, that status object's
fid must be the same as the associated cm_buf_t object.
If not, the wrong locks will be held.

If the cm_scache_t parameter is NULL and cm_FindSCache()
returns NULL, it means that the cm_scache_t object associated
with the bp->fid has been flushed from the cache.  cm_GetSCache()
must therefore be called to allocate a new status object for the
FID.  If the status object cannot be allocated, then any dirty
data stored in the buffer will be discarded.

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

diff --git a/src/WINNT/afsd/cm_buf.c b/src/WINNT/afsd/cm_buf.c
index 99efd4c..f698b4d 100644 (file)
--- a/src/WINNT/afsd/cm_buf.c
+++ b/src/WINNT/afsd/cm_buf.c
@@ -18,7 +18,6 @@
 #include <windows.h>
 #include <osi.h>
 #include <stdio.h>
-#include <assert.h>
 #include <strsafe.h>
 #include <math.h>
 
@@ -743,6 +742,9 @@ cm_buf_t *buf_FindAll(struct cm_scache *scp, osi_hyper_t *offsetp, afs_uint32 fl
  * if this buffer contains logged data.
  *
  * Returns non-zero if the buffer was dirty.
+ *
+ * 'scp' may or may not be NULL.  If it is not NULL, the FID for both cm_scache_t
+ * and cm_buf_t must match.
  */
 afs_uint32 buf_CleanAsyncLocked(cm_scache_t *scp, cm_buf_t *bp, cm_req_t *reqp,
                                 afs_uint32 flags, afs_uint32 *pisdirty)
@@ -753,35 +755,54 @@ afs_uint32 buf_CleanAsyncLocked(cm_scache_t *scp, cm_buf_t *bp, cm_req_t *reqp,
     int release_scp = 0;
 
     osi_assertx(bp->magic == CM_BUF_MAGIC, "invalid cm_buf_t magic");
+    osi_assertx(scp == NULL || cm_FidCmp(&scp->fid, &bp->fid) == 0, "scp fid != bp fid");
 
-    if (scp = cm_FindSCache(&bp->fid))
-        release_scp = 1;
-
-    if (!scp) {
-        osi_Log1(buf_logp, "buf_CleanAsyncLocked unable to start I/O - scp not found buf 0x%p", bp);
-        code = CM_ERROR_NOSUCHFILE;
+    /*
+     * If the matching cm_scache_t was not provided as a parameter
+     * we must either find one or allocate a new one.  It is possible
+     * that the cm_scache_t was recycled out of the cache even though
+     * a cm_buf_t with the same FID is in the cache.
+     */
+    if (scp == NULL) {
+        if ((scp = cm_FindSCache(&bp->fid)) ||
+            (cm_GetSCache(&bp->fid, &scp,
+                          bp->userp ? bp->userp : cm_rootUserp,
+                          reqp) == 0)) {
+            release_scp = 1;
+        }
     }
 
     while ((bp->flags & CM_BUF_DIRTY) == CM_BUF_DIRTY) {
         isdirty = 1;
         lock_ReleaseMutex(&bp->mx);
 
-        osi_Log2(buf_logp, "buf_CleanAsyncLocked starts I/O on scp 0x%p buf 0x%p", scp, bp);
+        if (!scp) {
+            /*
+             * If we didn't find a cm_scache_t object for bp->fid it means
+             * that we no longer have that FID in the cache.  It does not
+             * mean that the object does not exist in the cell.  That may
+             * in fact be the case but we don't know that until we attempt
+             * a FetchStatus on the FID.
+             */
+            osi_Log1(buf_logp, "buf_CleanAsyncLocked unable to start I/O - scp not found buf 0x%p", bp);
+            code = CM_ERROR_NOSUCHFILE;
+        } else {
+            osi_Log2(buf_logp, "buf_CleanAsyncLocked starts I/O on scp 0x%p buf 0x%p", scp, bp);
 
-        offset = bp->offset;
-        LargeIntegerAdd(offset, ConvertLongToLargeInteger(bp->dirty_offset));
-        code = (*cm_buf_opsp->Writep)(scp, &offset,
+            offset = bp->offset;
+            LargeIntegerAdd(offset, ConvertLongToLargeInteger(bp->dirty_offset));
+            code = (*cm_buf_opsp->Writep)(scp, &offset,
 #if 1
-                                       /* we might as well try to write all of the contiguous
-                                       * dirty buffers in one RPC
-                                       */
-                                       cm_chunkSize,
+                                          /* we might as well try to write all of the contiguous
+                                           * dirty buffers in one RPC
+                                           */
+                                          cm_chunkSize,
 #else
-                                       bp->dirty_length,
+                                          bp->dirty_length,
 #endif
-                                       flags, bp->userp, reqp);
-        osi_Log3(buf_logp, "buf_CleanAsyncLocked I/O on scp 0x%p buf 0x%p, done=%d", scp, bp, code);
-
+                                          flags, bp->userp, reqp);
+            osi_Log3(buf_logp, "buf_CleanAsyncLocked I/O on scp 0x%p buf 0x%p, done=%d", scp, bp, code);
+        }
         lock_ObtainMutex(&bp->mx);
        /* if the Write routine returns No Such File, clear the dirty flag
         * because we aren't going to be able to write this data to the file
@@ -1024,7 +1045,10 @@ long buf_GetNewLocked(struct cm_scache *scp, osi_hyper_t *offsetp, cm_req_t *req
                  * have the WRITING flag set, so we won't get
                  * back here.
                  */
-                buf_CleanAsync(scp, bp, reqp, 0, NULL);
+                if (cm_FidCmp(&scp->fid, &bp->fid) == 0)
+                    buf_CleanAsync(scp, bp, reqp, 0, NULL);
+                else
+                    buf_CleanAsync(NULL, bp, reqp, 0, NULL);
 
                 /* now put it back and go around again */
                 buf_Release(bp);