windows-smb-misc-20090220

LICENSE MIT

Fix smb_FindFIDByScache() to avoid obtaining the smb_fid_t.mx and
smb_rctLock out of order.  Doing so requires obtaining references
on each smb_fid_t belonging to the smb_vc_t in order to prevent them
from being removed from the list while the list is being walked.

Reorder tests for CM_SCACHEFLAG_DELETED and smb_fid_t.scp to make
them more efficient and consistent.

When processing Tran2SetPathInfo do not fail because an smb_fid_t
cannot be found for the path object.  The PathInfo function is
being used because we do not have a file descriptor.  Most importantly
do not fail by returning success.

diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c
index 87a186b..ed8a943 100644 (file)
--- a/src/WINNT/afsd/smb.c
+++ b/src/WINNT/afsd/smb.c
@@ -1666,30 +1666,61 @@ smb_fid_t *smb_FindFID(smb_vc_t *vcp, unsigned short fid, int flags)
     return fidp;
 }
 
+
+/* Must not be called with scp->rw held because smb_ReleaseFID might be called */
 #ifdef DEBUG_SMB_REFCOUNT
 smb_fid_t *smb_FindFIDByScacheDbg(smb_vc_t *vcp, cm_scache_t * scp, char *file, long line)
 #else
 smb_fid_t *smb_FindFIDByScache(smb_vc_t *vcp, cm_scache_t * scp)
 #endif
 {
-    smb_fid_t *fidp = NULL;
-    int newFid = 0;
+    smb_fid_t *fidp = NULL, *nextp = NULL;
         
     if (!scp)
         return NULL;
 
+    /* 
+     * If the fidp->scp changes out from under us then
+     * we must not grab a refCount.  It means the *fidp
+     * was processed by smb_CloseFID() and the *fidp is 
+     * no longer valid for use.
+     */
     lock_ObtainWrite(&smb_rctLock);
-    for(fidp = vcp->fidsp; fidp; fidp = (smb_fid_t *) osi_QNext(&fidp->q)) {
+       for(fidp = vcp->fidsp, (fidp ? fidp->refCount++ : 0); fidp; fidp = nextp, nextp = NULL) {
+        nextp = (smb_fid_t *) osi_QNext(&fidp->q);
+        if (nextp)
+            nextp->refCount++;
+
         if (scp == fidp->scp) {
+            lock_ReleaseWrite(&smb_rctLock);
             lock_ObtainMutex(&fidp->mx);
+            lock_ObtainWrite(&smb_rctLock);
             if (scp == fidp->scp) {
-                fidp->refCount++;   
                 lock_ReleaseMutex(&fidp->mx);
                 break;
             }
             lock_ReleaseMutex(&fidp->mx);
         }
+
+        if (fidp->refCount > 1) {
+            fidp->refCount--;
+        } else {
+            lock_ReleaseWrite(&smb_rctLock);
+            smb_ReleaseFID(fidp);
+            lock_ObtainWrite(&smb_rctLock);
+        }
+    }
+
+    if (nextp) {
+        if (nextp->refCount > 1) {
+            nextp->refCount--;
+        } else {
+            lock_ReleaseWrite(&smb_rctLock);
+            smb_ReleaseFID(nextp);
+            lock_ObtainWrite(&smb_rctLock);
+        }
     }
+
 #ifdef DEBUG_SMB_REFCOUNT
     if (fidp) {
         afsi_log("%s:%d smb_FindFIDByScache fidp 0x%p ref %d", file, line, fidp, fidp->refCount);
@@ -1697,7 +1728,7 @@ smb_fid_t *smb_FindFIDByScache(smb_vc_t *vcp, cm_scache_t * scp)
       }
 #endif
     lock_ReleaseWrite(&smb_rctLock);
-    return fidp;
+    return (fidp);
 }
 
 #ifdef DEBUG_SMB_REFCOUNT
@@ -1715,8 +1746,8 @@ void smb_HoldFIDNoLock(smb_fid_t *fidp)
 }
 
 
-/* smb_ReleaseFID cannot be called while an cm_scache_t mutex lock is held */
-/* the sm_fid_t->mx and smb_rctLock must not be held */
+/* smb_ReleaseFID cannot be called while a cm_scache_t rwlock is held */
+/* the smb_fid_t->mx and smb_rctLock must not be held */
 #ifdef DEBUG_SMB_REFCOUNT
 void smb_ReleaseFIDDbg(smb_fid_t *fidp, char *file, long line)
 #else
@@ -3500,7 +3531,13 @@ long smb_ReceiveCoreReadRaw(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp
         goto send1;
 
     lock_ObtainMutex(&fidp->mx);
-    if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) {
+    if (!fidp->scp) {
+        lock_ReleaseMutex(&fidp->mx);
+        smb_ReleaseFID(fidp);
+        return CM_ERROR_BADFD;
+    }
+
+    if (fidp->scp->flags & CM_SCACHEFLAG_DELETED) {
         lock_ReleaseMutex(&fidp->mx);
         smb_CloseFID(vcp, fidp, NULL, 0);
         code = CM_ERROR_NOSUCHFILE;
@@ -6523,22 +6560,22 @@ long smb_ReceiveCoreFlush(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
     userp = smb_GetUserFromVCP(vcp, inp);
 
     lock_ObtainMutex(&fidp->mx);
-    if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) {
-        lock_ReleaseMutex(&fidp->mx);
+    if (!fidp->scp || (fidp->flags & SMB_FID_IOCTL)) {
         cm_ReleaseUser(userp);
-        smb_CloseFID(vcp, fidp, NULL, 0);
+        lock_ReleaseMutex(&fidp->mx);
         smb_ReleaseFID(fidp);
-        return CM_ERROR_NOSUCHFILE;
+        return CM_ERROR_BADFD;
     }
 
-    if (fidp->flags & SMB_FID_IOCTL) {
-        cm_ReleaseUser(userp);
+    if (fidp->scp->flags & CM_SCACHEFLAG_DELETED) {
         lock_ReleaseMutex(&fidp->mx);
+        cm_ReleaseUser(userp);
+        smb_CloseFID(vcp, fidp, NULL, 0);
         smb_ReleaseFID(fidp);
-        return CM_ERROR_BADFD;
+        return CM_ERROR_NOSUCHFILE;
     }
 
-    if (fidp->scp && (fidp->flags & SMB_FID_OPENWRITE) && smb_AsyncStore != 2) {
+    if ((fidp->flags & SMB_FID_OPENWRITE) && smb_AsyncStore != 2) {
         cm_scache_t * scp = fidp->scp;
         cm_HoldSCache(scp);
         lock_ReleaseMutex(&fidp->mx);
@@ -7319,14 +7356,7 @@ long smb_ReceiveCoreWrite(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
         return CM_ERROR_BADFD;
     }
         
-       lock_ObtainMutex(&fidp->mx);
-    if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) {
-        lock_ReleaseMutex(&fidp->mx);
-        smb_CloseFID(vcp, fidp, NULL, 0);
-        smb_ReleaseFID(fidp);
-        return CM_ERROR_NOSUCHFILE;
-    }
-
+    lock_ObtainMutex(&fidp->mx);
     if (fidp->flags & SMB_FID_IOCTL) {
        lock_ReleaseMutex(&fidp->mx);
         code = smb_IoctlWrite(fidp, vcp, inp, outp);
@@ -7341,6 +7371,13 @@ long smb_ReceiveCoreWrite(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
         return CM_ERROR_BADFD;
     }
 
+    if (fidp->scp->flags & CM_SCACHEFLAG_DELETED) {
+        lock_ReleaseMutex(&fidp->mx);
+        smb_CloseFID(vcp, fidp, NULL, 0);
+        smb_ReleaseFID(fidp);
+        return CM_ERROR_NOSUCHFILE;
+    }
+
     scp = fidp->scp;
     cm_HoldSCache(scp);
     lock_ReleaseMutex(&fidp->mx);
@@ -7452,7 +7489,13 @@ void smb_CompleteWriteRaw(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp,
     fidp = smb_FindFID(vcp, fd, 0);
 
     lock_ObtainMutex(&fidp->mx);
-    if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) {
+    if (!fidp->scp) {
+        lock_ReleaseMutex(&fidp->mx);
+        smb_ReleaseFID(fidp);
+        return;
+    }
+
+    if (fidp->scp->flags & CM_SCACHEFLAG_DELETED) {
         lock_ReleaseMutex(&fidp->mx);
         smb_CloseFID(vcp, fidp, NULL, 0);
         smb_ReleaseFID(fidp);
@@ -7565,18 +7608,19 @@ long smb_ReceiveCoreWriteRaw(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *out
         return CM_ERROR_BADFD;
 
     lock_ObtainMutex(&fidp->mx);
-    if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) {
+    if (!fidp->scp) {
         lock_ReleaseMutex(&fidp->mx);
-        smb_CloseFID(vcp, fidp, NULL, 0);
         smb_ReleaseFID(fidp);
-        return CM_ERROR_NOSUCHFILE;
+        return CM_ERROR_BADFD;
     }
 
-    if (!fidp->scp) {
+    if (fidp->scp->flags & CM_SCACHEFLAG_DELETED) {
         lock_ReleaseMutex(&fidp->mx);
+        smb_CloseFID(vcp, fidp, NULL, 0);
         smb_ReleaseFID(fidp);
-        return CM_ERROR_BADFDOP;
+        return CM_ERROR_NOSUCHFILE;
     }
+
     scp = fidp->scp;
     cm_HoldSCache(scp);
     lock_ReleaseMutex(&fidp->mx);
@@ -7721,14 +7765,7 @@ long smb_ReceiveCoreRead(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
     if (!fidp)
         return CM_ERROR_BADFD;
 
-       lock_ObtainMutex(&fidp->mx);
-    if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) {
-        lock_ReleaseMutex(&fidp->mx);
-        smb_CloseFID(vcp, fidp, NULL, 0);
-        smb_ReleaseFID(fidp);
-        return CM_ERROR_NOSUCHFILE;
-    }
-
+    lock_ObtainMutex(&fidp->mx);
     if (fidp->flags & SMB_FID_IOCTL) {
        lock_ReleaseMutex(&fidp->mx);
         code = smb_IoctlRead(fidp, vcp, inp, outp);
@@ -7739,8 +7776,16 @@ long smb_ReceiveCoreRead(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
     if (!fidp->scp) {
         lock_ReleaseMutex(&fidp->mx);
         smb_ReleaseFID(fidp);
-        return CM_ERROR_BADFDOP;
+        return CM_ERROR_BADFD;
+    }
+
+    if (fidp->scp->flags & CM_SCACHEFLAG_DELETED) {
+        lock_ReleaseMutex(&fidp->mx);
+        smb_CloseFID(vcp, fidp, NULL, 0);
+        smb_ReleaseFID(fidp);
+        return CM_ERROR_NOSUCHFILE;
     }
+
     scp = fidp->scp;
     cm_HoldSCache(scp);
     lock_ReleaseMutex(&fidp->mx);
@@ -8165,23 +8210,17 @@ long smb_ReceiveCoreSeek(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
        return CM_ERROR_BADFD;
 
     lock_ObtainMutex(&fidp->mx);
-    if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) {
-        lock_ReleaseMutex(&fidp->mx);
-        smb_CloseFID(vcp, fidp, NULL, 0);
-        smb_ReleaseFID(fidp);
-        return CM_ERROR_NOSUCHFILE;
-    }
-
-    if (fidp->flags & SMB_FID_IOCTL) {
+    if (!fidp->scp || (fidp->flags & SMB_FID_IOCTL)) {
        lock_ReleaseMutex(&fidp->mx);
        smb_ReleaseFID(fidp);
         return CM_ERROR_BADFD;
     }
 
-    if (!fidp->scp) {
+    if (fidp->scp->flags & CM_SCACHEFLAG_DELETED) {
         lock_ReleaseMutex(&fidp->mx);
+        smb_CloseFID(vcp, fidp, NULL, 0);
         smb_ReleaseFID(fidp);
-        return CM_ERROR_BADFDOP;
+        return CM_ERROR_NOSUCHFILE;
     }
 
     lock_ReleaseMutex(&fidp->mx);

diff --git a/src/WINNT/afsd/smb3.c b/src/WINNT/afsd/smb3.c
index dadaa91..fa27c50 100644 (file)
--- a/src/WINNT/afsd/smb3.c
+++ b/src/WINNT/afsd/smb3.c
@@ -3247,7 +3247,6 @@ long smb_ReceiveTran2SetPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet
     return CM_ERROR_BADOP;
 #else
     long code = 0;
-    smb_fid_t *fidp;
     unsigned short infoLevel;
     clientchar_t * pathp;
     smb_tran2Packet_t *outp;
@@ -3369,25 +3368,6 @@ long smb_ReceiveTran2SetPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet
         return 0;
     }
 
-    fidp = smb_FindFIDByScache(vcp, scp);
-    if (!fidp) {
-        cm_ReleaseSCache(scp);
-        cm_ReleaseUser(userp);
-       smb_SendTran2Error(vcp, p, opx, code);
-        return 0;
-    }
-
-    lock_ObtainMutex(&fidp->mx);
-    if (!(fidp->flags & SMB_FID_OPENWRITE)) {
-       lock_ReleaseMutex(&fidp->mx);
-        cm_ReleaseSCache(scp);
-        smb_ReleaseFID(fidp);
-        cm_ReleaseUser(userp);
-        smb_SendTran2Error(vcp, p, opx, CM_ERROR_NOACCESS);
-        return 0;
-    }
-    lock_ReleaseMutex(&fidp->mx);
-
     outp = smb_GetTran2ResponsePacket(vcp, p, opx, 2, 0);
 
     outp->totalParms = 2;
@@ -3410,10 +3390,6 @@ long smb_ReceiveTran2SetPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet
         }
        cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS);
 
-       lock_ReleaseWrite(&scp->rw);
-       lock_ObtainMutex(&fidp->mx);
-       lock_ObtainRead(&scp->rw);
-
         /* prepare for setattr call */
         attr.mask = CM_ATTRMASK_LENGTH;
         attr.length.LowPart = spi->u.QPstandardInfo.dataSize;
@@ -3422,7 +3398,6 @@ long smb_ReceiveTran2SetPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet
        if (spi->u.QPstandardInfo.lastWriteDateTime != 0) {
            smb_UnixTimeFromSearchTime(&attr.clientModTime, spi->u.QPstandardInfo.lastWriteDateTime);
             attr.mask |= CM_ATTRMASK_CLIENTMODTIME;
-            fidp->flags |= SMB_FID_MTIMESETDONE;
         }
                
         if (spi->u.QPstandardInfo.attributes != 0) {
@@ -3440,7 +3415,6 @@ long smb_ReceiveTran2SetPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet
             }
         }
         lock_ReleaseRead(&scp->rw);
-       lock_ReleaseMutex(&fidp->mx);
 
         /* call setattr */
         if (attr.mask)
@@ -3456,7 +3430,6 @@ long smb_ReceiveTran2SetPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet
   done:
     cm_ReleaseSCache(scp);
     cm_ReleaseUser(userp);
-    smb_ReleaseFID(fidp);
     if (code == 0) 
         smb_SendTran2Packet(vcp, outp, opx);
     else 
@@ -3653,7 +3626,7 @@ long smb_ReceiveTran2SetFileInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet
 
     lock_ObtainMutex(&fidp->mx);
     if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) {
-               lock_ReleaseMutex(&fidp->mx);
+        lock_ReleaseMutex(&fidp->mx);
         smb_SendTran2Error(vcp, p, opx, CM_ERROR_NOSUCHFILE);
         smb_CloseFID(vcp, fidp, NULL, 0);
         smb_ReleaseFID(fidp);
@@ -6683,7 +6656,7 @@ long smb_ReceiveV3ReadX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
         return CM_ERROR_BADFD;
     }
 
-       lock_ObtainMutex(&fidp->mx);
+    lock_ObtainMutex(&fidp->mx);
     if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) {
         lock_ReleaseMutex(&fidp->mx);
         smb_CloseFID(vcp, fidp, NULL, 0);