windows-off-to-the-races-20060211

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

diff --git a/src/WINNT/afsd/cm_cell.c b/src/WINNT/afsd/cm_cell.c
index a23e3b140ecc9751d8a945e4d0252844436d84ab..b74c1a8d87ad457912211a39874559019a00a45a 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;
 
@@ -97,24 +97,21 @@ cm_cell_t *cm_UpdateCell(cm_cell_t * cp)
 #ifdef DEBUG
                     fprintf(stderr, "cell %s: ttl=%d\n", cp->name, ttl);
 #endif
-                } else {
+               } else {
                     /* if we fail to find it this time, we'll just do nothing and leave the
-                    * current entry alone 
-                    */
+                     * 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 1d0d502a42649be5b385d47b9e5e2bc1a05aace2..9d04c4afb817020b901ae33179c0a0d485af17ad 100644 (file)
--- a/src/WINNT/afsd/smb.c
+++ b/src/WINNT/afsd/smb.c
@@ -1003,7 +1003,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;
@@ -1012,7 +1012,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);
 
@@ -1022,9 +1021,9 @@ 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);
@@ -1032,10 +1031,6 @@ void smb_CleanupDeadVC(smb_vc_t *vcp)
        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);
@@ -1044,18 +1039,14 @@ 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;
 
        /* do not add an additional reference count for the smb_user_t
         * 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);
@@ -1120,7 +1111,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) 
@@ -1447,7 +1438,7 @@ 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)) {
+    if (fidp->refCount == 0 && (fidp->delete)) {
         vcp = fidp->vcp;
         fidp->vcp = NULL;
         scp = fidp->scp;    /* release after lock is released */
@@ -1455,8 +1446,8 @@ void smb_ReleaseFID(smb_fid_t *fidp)
         userp = fidp->userp;
         fidp->userp = NULL;
 
-       if (vcp->fidsp)
-         osi_QRemove((osi_queue_t **) &vcp->fidsp, &fidp->q);
+       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 */
@@ -1470,7 +1461,6 @@ void smb_ReleaseFID(smb_fid_t *fidp)
                 free(ioctlp->outAllocp);
             free(ioctlp);
         }       
-
        lock_ReleaseMutex(&fidp->mx);
        lock_FinalizeMutex(&fidp->mx);
         free(fidp);
@@ -3245,15 +3235,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 */;
@@ -3266,10 +3262,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);
 }
 
@@ -4723,9 +4725,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);
     }
 
@@ -5687,16 +5689,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 5afe761d612bad98d8985b7bf36834b965b33c20..d2ebb5e3c5ea15842593f5c4445e5e909a4949b0 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) */