STABLE14-windows-off-to-the-races-20060211

several race conditions were introduced over the last couple of weeks.
let's fix them.

(cherry picked from commit d9d798f78617026349e3c087c714e474e9eb2b7f)

diff --git a/src/WINNT/afsd/cm_cell.c b/src/WINNT/afsd/cm_cell.c
index a23e3b140ecc9751d8a945e4d0252844436d84ab..604a60d12aa7580442b9ff1154f1e3560a2393f0 100644 (file)
--- a/src/WINNT/afsd/cm_cell.c
+++ b/src/WINNT/afsd/cm_cell.c
@@ -61,7 +61,7 @@ long cm_AddCellProc(void *rockp, struct sockaddr_in *addrp, char *namep)
  */
 cm_cell_t *cm_UpdateCell(cm_cell_t * cp)
 {
-    long code;
+    long code = 0;
 
     if (cp == NULL)
         return NULL;
@@ -84,8 +84,8 @@ cm_cell_t *cm_UpdateCell(cm_cell_t * cp)
         }
 
         code = cm_SearchCellFile(cp->name, NULL, cm_AddCellProc, cp);
-        if (code) {
 #ifdef AFS_AFSDB_ENV
+        if (code) {
             if (cm_dnsEnabled) {
                 int ttl;
 
@@ -102,19 +102,16 @@ cm_cell_t *cm_UpdateCell(cm_cell_t * cp)
                     * current entry alone 
                     */
                     cp->flags |= CM_CELLFLAG_VLSERVER_INVALID;
-                    cp = NULL;      /* return NULL to indicate failure */
                 }
-            } else 
+           }
+       } else 
 #endif /* AFS_AFSDB_ENV */
-            {
-                cp = NULL;          /* return NULL to indicate failure */
-            }
-        } else {
+        {
            cp->timeout = time(0) + 7200;
        }       
     }
     lock_ReleaseMutex(&cp->mx);
-    return cp;
+    return code ? NULL : cp;
 }
 
 /* load up a cell structure from the cell database, afsdcell.ini */

diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c
index c573fd432186f20079c6a0fe9eb1ebec027b1682..6b66335d2c369c380039e1788e41d7fb3a77574b 100644 (file)
--- a/src/WINNT/afsd/smb.c
+++ b/src/WINNT/afsd/smb.c
@@ -953,7 +953,7 @@ void smb_CleanupDeadVC(smb_vc_t *vcp)
     for (fidpIter = vcp->fidsp; fidpIter; fidpIter = fidpNext) {
         fidpNext = (smb_fid_t *) osi_QNext(&fidpIter->q);
 
-        if (fidpIter->flags & SMB_FID_DELETE)
+        if (fidpIter->delete)
             continue;
 
         fid = fidpIter->fid;
@@ -962,7 +962,6 @@ void smb_CleanupDeadVC(smb_vc_t *vcp)
        smb_HoldFIDNoLock(fidpIter);
        lock_ReleaseWrite(&smb_rctLock);
 
-       /* smb_CloseFID sets SMB_FID_DELETE on Success */
         smb_CloseFID(vcp, fidpIter, NULL, 0);
        smb_ReleaseFID(fidpIter);
 
@@ -972,19 +971,15 @@ void smb_CleanupDeadVC(smb_vc_t *vcp)
 
     for (tidpIter = vcp->tidsp; tidpIter; tidpIter = tidpNext) {
        tidpNext = tidpIter->nextp;
-       if (tidpIter->flags & SMB_TIDFLAG_DELETE)
+       if (tidpIter->delete)
            continue;
+       tidpIter->delete = 1;
 
        tid = tidpIter->tid;
        osi_Log2(smb_logp, "  Cleanup TID %d (tidp=0x%x)", tid, tidpIter);
 
        smb_HoldTIDNoLock(tidpIter);
        lock_ReleaseWrite(&smb_rctLock);
-
-       lock_ObtainMutex(&tidpIter->mx);
-       tidpIter->flags |= SMB_TIDFLAG_DELETE;
-       lock_ReleaseMutex(&tidpIter->mx);
-
        smb_ReleaseTID(tidpIter);
 
        lock_ObtainWrite(&smb_rctLock);
@@ -993,8 +988,9 @@ void smb_CleanupDeadVC(smb_vc_t *vcp)
 
     for (uidpIter = vcp->usersp; uidpIter; uidpIter = uidpNext) {
        uidpNext = uidpIter->nextp;
-       if (uidpIter->flags & SMB_USERFLAG_DELETE)
+       if (uidpIter->delete)
            continue;
+       uidpIter->delete = 1;
 
        uid = uidpIter->userID;
        osi_Log2(smb_logp, "  Cleanup UID %d (uidp=0x%x)", uid, uidpIter);
@@ -1003,10 +999,6 @@ void smb_CleanupDeadVC(smb_vc_t *vcp)
         * as the smb_vc_t already is holding a reference */
        lock_ReleaseWrite(&smb_rctLock);
 
-       lock_ObtainMutex(&uidpIter->mx);
-       uidpIter->flags |= SMB_USERFLAG_DELETE;
-       lock_ReleaseMutex(&uidpIter->mx);
-
        smb_ReleaseUID(uidpIter);
 
        lock_ObtainWrite(&smb_rctLock);
@@ -1071,7 +1063,7 @@ void smb_ReleaseTID(smb_tid_t *tidp)
     userp = NULL;
     lock_ObtainWrite(&smb_rctLock);
     osi_assert(tidp->refCount-- > 0);
-    if (tidp->refCount == 0 && (tidp->flags & SMB_TIDFLAG_DELETE)) {
+    if (tidp->refCount == 0 && (tidp->delete)) {
         ltpp = &tidp->vcp->tidsp;
         for(tp = *ltpp; tp; ltpp = &tp->nextp, tp = *ltpp) {
             if (tp == tidp) 
@@ -1395,30 +1387,29 @@ void smb_ReleaseFID(smb_fid_t *fidp)
     lock_ObtainWrite(&smb_rctLock);
     osi_assert(fidp->refCount-- > 0);
     lock_ObtainMutex(&fidp->mx);
-    if (fidp->refCount == 0 && (fidp->flags & SMB_FID_DELETE)) {
-        vcp = fidp->vcp;
-        fidp->vcp = NULL;
-        scp = fidp->scp;    /* release after lock is released */
-        fidp->scp = NULL;
-        userp = fidp->userp;
-        fidp->userp = NULL;
+    if (fidp->refCount == 0 && (fidp->delete)) {
+       vcp = fidp->vcp;
+       fidp->vcp = NULL;
+       scp = fidp->scp;    /* release after lock is released */
+       fidp->scp = NULL;
+       userp = fidp->userp;
+       fidp->userp = NULL;
 
        if (vcp->fidsp)
-         osi_QRemove((osi_queue_t **) &vcp->fidsp, &fidp->q);
-        thrd_CloseHandle(fidp->raw_write_event);
-
-        /* and see if there is ioctl stuff to free */
-        ioctlp = fidp->ioctlp;
-        if (ioctlp) {
-            if (ioctlp->prefix)
-                cm_FreeSpace(ioctlp->prefix);
-            if (ioctlp->inAllocp)
-                free(ioctlp->inAllocp);
-            if (ioctlp->outAllocp)
-                free(ioctlp->outAllocp);
-            free(ioctlp);
-        }       
-
+           osi_QRemove((osi_queue_t **) &vcp->fidsp, &fidp->q);
+       thrd_CloseHandle(fidp->raw_write_event);
+
+       /* and see if there is ioctl stuff to free */
+       ioctlp = fidp->ioctlp;
+       if (ioctlp) {
+           if (ioctlp->prefix)
+               cm_FreeSpace(ioctlp->prefix);
+           if (ioctlp->inAllocp)
+               free(ioctlp->inAllocp);
+           if (ioctlp->outAllocp)
+               free(ioctlp->outAllocp);
+           free(ioctlp);
+       }
        lock_ReleaseMutex(&fidp->mx);
        lock_FinalizeMutex(&fidp->mx);
         free(fidp);
@@ -3242,15 +3233,21 @@ long smb_ReceiveNegotiate(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp)
 
 void smb_CheckVCs(void)
 {
-    smb_vc_t * vcp;
+    smb_vc_t * vcp, *nextp;
     smb_packet_t * outp = GetPacket();
     smb_t *smbp;
             
-    for ( vcp=smb_allVCsp; vcp; vcp = vcp->nextp ) 
+    lock_ObtainWrite(&smb_rctLock);
+    for ( vcp=smb_allVCsp, nextp=NULL; vcp; vcp = nextp ) 
     {
+       nextp = vcp->nextp;
+
        if (vcp->flags & SMB_VCFLAG_ALREADYDEAD)
            continue;
 
+       smb_HoldVCNoLock(vcp);
+       if (nextp)
+           smb_HoldVCNoLock(nextp);
        smb_FormatResponsePacket(vcp, NULL, outp);
         smbp = (smb_t *)outp;
        outp->inCom = smbp->com = 0x2b /* Echo */;
@@ -3263,10 +3260,16 @@ void smb_CheckVCs(void)
 
        smb_SetSMBParm(outp, 0, 0);
        smb_SetSMBDataLength(outp, 0);
+       lock_ReleaseWrite(&smb_rctLock);
 
        smb_SendPacket(vcp, outp);
+       
+       lock_ObtainWrite(&smb_rctLock);
+       smb_ReleaseVCNoLock(vcp);
+       if (nextp)
+           smb_ReleaseVCNoLock(nextp);
     }
-
+    lock_ReleaseWrite(&smb_rctLock);
     smb_FreePacket(outp);
 }
 
@@ -4720,9 +4723,9 @@ long smb_ReceiveCoreTreeDisconnect(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_
     /* find the tree and free it */
     tidp = smb_FindTID(vcp, ((smb_t *)inp)->tid, 0);
     if (tidp) {
-        lock_ObtainMutex(&tidp->mx);
-        tidp->flags |= SMB_TIDFLAG_DELETE;
-        lock_ReleaseMutex(&tidp->mx);
+       lock_ObtainWrite(&smb_rctLock);
+        tidp->delete = 1;
+        lock_ReleaseWrite(&smb_rctLock);
         smb_ReleaseTID(tidp);
     }
 
@@ -5684,16 +5687,16 @@ long smb_CloseFID(smb_vc_t *vcp, smb_fid_t *fidp, cm_user_t *userp,
 
     cm_InitReq(&req);
 
-    lock_ObtainMutex(&fidp->mx);
-
-    if (fidp->flags & SMB_FID_DELETE) {
+    lock_ObtainWrite(&smb_rctLock);
+    if (fidp->delete) {
        osi_Log0(smb_logp, "  Fid already closed.");
-       lock_ReleaseMutex(&fidp->mx);
+       lock_ReleaseWrite(&smb_rctLock);
        return CM_ERROR_BADFD;
     }
+    fidp->delete = 1;
+    lock_ReleaseWrite(&smb_rctLock);
 
-    fidp->flags |= SMB_FID_DELETE;
-        
+    lock_ObtainMutex(&fidp->mx);
     /* Don't jump the gun on an async raw write */
     while (fidp->raw_writers) {
         lock_ReleaseMutex(&fidp->mx);

diff --git a/src/WINNT/afsd/smb.h b/src/WINNT/afsd/smb.h
index 866344b61235398ab9e247b08aa4fb18b5e11120..ed800b1feaa5c9ed2ea27dccc3ab9e52a0158a2a 100644 (file)
--- a/src/WINNT/afsd/smb.h
+++ b/src/WINNT/afsd/smb.h
@@ -227,11 +227,12 @@ typedef struct smb_vc {
 typedef struct smb_user {
     struct smb_user *nextp;            /* next sibling */
     unsigned long refCount;            /* ref count */
-    long flags;                                /* flags; locked by mx */
+    afs_uint32 flags;                  /* flags; locked by mx */
     osi_mutex_t mx;
     unsigned short userID;             /* the session identifier */
     struct smb_vc *vcp;                        /* back ptr to virtual circuit */
     struct smb_username *unp;           /* user name struct */
+    afs_uint32 delete;                 /* ok to del: locked by smb_rctLock */
 } smb_user_t;
 
 #define SMB_USERFLAG_DELETE        1   /* delete struct when ref count zero */
@@ -270,17 +271,17 @@ typedef struct smb_username {
 typedef struct smb_tid {
     struct smb_tid *nextp;             /* next sibling */
     unsigned long refCount;
-    long flags;
+    afs_uint32 flags;                  /* protected by mx */
     osi_mutex_t mx;                    /* for non-tree-related stuff */
     unsigned short tid;                        /* the tid */
     struct smb_vc *vcp;                        /* back ptr */
     struct cm_user *userp;             /* user logged in at the
                                         * tree connect level (base) */
     char *pathname;                    /* pathname derived from sharename */
+    afs_uint32 delete;                 /* ok to del: locked by smb_rctLock */
 } smb_tid_t;
 
-#define SMB_TIDFLAG_DELETE     1       /* delete struct when ref count zero */
-#define SMB_TIDFLAG_IPC        2 /* IPC$ */
+#define SMB_TIDFLAG_IPC                1       /* IPC$ */
 
 /* one per process ID */
 typedef struct smb_pid {
@@ -327,7 +328,7 @@ typedef struct smb_ioctl {
 typedef struct smb_fid {
     osi_queue_t q;
     unsigned long refCount;
-    unsigned long flags;
+    afs_uint32 flags;                  /* protected by mx */
     osi_mutex_t mx;                    /* for non-tree-related stuff */
     unsigned short fid;                        /* the file ID */
     struct smb_vc *vcp;                        /* back ptr */
@@ -350,11 +351,12 @@ typedef struct smb_fid {
     int prev_chunk;                    /* previous chunk read */
     int raw_writers;                   /* pending async raw writes */
     EVENT_HANDLE raw_write_event;      /* signal this when raw_writers zero */
+    afs_uint32 delete;                 /* ok to del: locked by smb_rctLock */
 } smb_fid_t;
 
 #define SMB_FID_OPENREAD               1       /* open for reading */
 #define SMB_FID_OPENWRITE              2       /* open for writing */
-#define SMB_FID_DELETE                 4       /* delete struct on ref count 0 */
+#define SMB_FID_UNUSED                  4       /* free for use */
 #define SMB_FID_IOCTL                  8       /* a file descriptor for the
                                                 * magic ioctl file */
 #define SMB_FID_OPENDELETE             0x10    /* open for deletion (NT) */