windows-protect-against-vcp-undercount-20060408

An undercount has been detected of the smb_vc_t objects stored
in the smb_allVCsp list.  Unfortunately, we have yet to be able
to find the cause of the undercount so this patch adds logic to
protect against the side effects until such time as the cause
can be identified.

diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c
index 58bd190fc371ee271d1264939110399b32d64866..a3af1f045c02384bf294c6e3289677aeac3a8721 100644 (file)
--- a/src/WINNT/afsd/smb.c
+++ b/src/WINNT/afsd/smb.c
@@ -858,6 +858,10 @@ smb_vc_t *smb_FindVC(unsigned short lsn, int flags, int lana)
 
     lock_ObtainWrite(&smb_rctLock);
     for (vcp = smb_allVCsp; vcp; vcp=vcp->nextp) {
+       if (vcp->magic != SMB_VC_MAGIC)
+           osi_panic("afsd: invalid smb_vc_t detected in smb_allVCsp", 
+                      __FILE__, __LINE__);
+
         if (lsn == vcp->lsn && lana == vcp->lana) {
             smb_HoldVCNoLock(vcp);
             break;
@@ -869,6 +873,7 @@ smb_vc_t *smb_FindVC(unsigned short lsn, int flags, int lana)
        lock_ObtainWrite(&smb_globalLock);
         vcp->vcID = ++numVCs;
        lock_ReleaseWrite(&smb_globalLock);
+       vcp->magic = SMB_VC_MAGIC;
         vcp->refCount = 2;     /* smb_allVCsp and caller */
         vcp->tidCounter = 1;
         vcp->fidCounter = 1;
@@ -936,24 +941,40 @@ int smb_IsStarMask(char *maskp)
 void smb_ReleaseVCInternal(smb_vc_t *vcp)
 {
     smb_vc_t **vcpp;
+    smb_vc_t * avcp;
 
-#ifdef DEBUG
-    osi_assert(vcp->refCount-- != 0);
-#else
     vcp->refCount--;
-#endif
 
     if (vcp->refCount == 0) {
+      if (vcp->flags & SMB_VCFLAG_ALREADYDEAD) {
        /* remove VCP from smb_deadVCsp */
        for (vcpp = &smb_deadVCsp; *vcpp; vcpp = &((*vcpp)->nextp)) {
-           if (*vcpp == vcp) {
-               *vcpp = vcp->nextp;
-               break;
-           }
+         if (*vcpp == vcp) {
+           *vcpp = vcp->nextp;
+           break;
+         }
        } 
        lock_FinalizeMutex(&vcp->mx);
        memset(vcp,0,sizeof(smb_vc_t));
        free(vcp);
+      } else {
+       for (avcp = smb_allVCsp; avcp; avcp = avcp->nextp) {
+         if (avcp == vcp)
+           break;
+       }
+       osi_Log3(smb_logp,"VCP not dead and %sin smb_allVCsp vcp %x ref %d",
+                avcp?"not ":"",vcp, vcp->refCount);
+#ifdef DEBUG
+       GenerateMiniDump(NULL);
+#endif
+       /* This is a wrong.  However, I suspect that there is an undercount
+        * and I don't want to release 1.4.1 in a state that will allow
+        * smb_vc_t objects to be deallocated while still in the
+        * smb_allVCsp list.  The list is supposed to keep a reference
+        * to the smb_vc_t.  Put it back.
+        */
+       vcp->refCount++;
+      }
     }
 }
 
@@ -1000,6 +1021,20 @@ void smb_CleanupDeadVC(smb_vc_t *vcp)
     osi_Log1(smb_logp, "Cleaning up dead vcp 0x%x", vcp);
 
     lock_ObtainWrite(&smb_rctLock);
+    /* remove VCP from smb_allVCsp */
+    for (vcpp = &smb_allVCsp; *vcpp; vcpp = &((*vcpp)->nextp)) {
+       if ((*vcpp)->magic != SMB_VC_MAGIC)
+           osi_panic("afsd: invalid smb_vc_t detected in smb_allVCsp", 
+                      __FILE__, __LINE__);
+        if (*vcpp == vcp) {
+            *vcpp = vcp->nextp;
+            vcp->nextp = smb_deadVCsp;
+            smb_deadVCsp = vcp;
+           /* Hold onto the reference until we are done with this function */
+            break;
+        }
+    }
+
     for (fidpIter = vcp->fidsp; fidpIter; fidpIter = fidpNext) {
         fidpNext = (smb_fid_t *) osi_QNext(&fidpIter->q);
 
@@ -1053,20 +1088,11 @@ void smb_CleanupDeadVC(smb_vc_t *vcp)
        uidpNext = vcp->usersp;
     }
 
-    /* remove VCP from smb_allVCsp */
-    for (vcpp = &smb_allVCsp; *vcpp; vcpp = &((*vcpp)->nextp)) {
-       if (*vcpp == vcp) {
-           *vcpp = vcp->nextp;
-           vcp->nextp = smb_deadVCsp;
-           smb_deadVCsp = vcp;
-           /* We intentionally do not keep a reference to the 
-            * vcp once it is placed on the deadVCsp list.  This
-            * allows the refcount to reach 0 so we can delete
-            * it. */
-           smb_ReleaseVCNoLock(vcp);
-           break;
-       }
-    } 
+
+    /* The vcp is now on the deadVCsp list.  We intentionally drop the
+     * reference so that the refcount can reach 0 and we can delete it */
+    smb_ReleaseVCNoLock(vcp);
+    
     lock_ReleaseWrite(&smb_rctLock);
     osi_Log1(smb_logp, "Finished cleaning up dead vcp 0x%x", vcp);
 }
@@ -3255,6 +3281,10 @@ void smb_CheckVCs(void)
     lock_ObtainWrite(&smb_rctLock);
     for ( vcp=smb_allVCsp, nextp=NULL; vcp; vcp = nextp ) 
     {
+       if (vcp->magic != SMB_VC_MAGIC)
+           osi_panic("afsd: invalid smb_vc_t detected in smb_allVCsp", 
+                      __FILE__, __LINE__);
+
        nextp = vcp->nextp;
 
        if (vcp->flags & SMB_VCFLAG_ALREADYDEAD)
@@ -8804,7 +8834,11 @@ void smb_Shutdown(void)
         smb_fid_t *fidp;
         smb_tid_t *tidp;
      
-        for (fidp = vcp->fidsp; fidp; fidp = (smb_fid_t *) osi_QNext(&fidp->q))
+       if (vcp->magic != SMB_VC_MAGIC)
+           osi_panic("afsd: invalid smb_vc_t detected in smb_allVCsp", 
+                      __FILE__, __LINE__);
+
+       for (fidp = vcp->fidsp; fidp; fidp = (smb_fid_t *) osi_QNext(&fidp->q))
         {
             if (fidp->scp != NULL) {
                 cm_scache_t * scp;

diff --git a/src/WINNT/afsd/smb.h b/src/WINNT/afsd/smb.h
index 7537bae8a5635e12620da4eeb2fc9826f44d6d21..30a66a1c29e586144b436fada625f1fb2964a345 100644 (file)
--- a/src/WINNT/afsd/smb.h
+++ b/src/WINNT/afsd/smb.h
@@ -192,6 +192,7 @@ typedef struct myncb {
 /* one per virtual circuit */
 typedef struct smb_vc {
     struct smb_vc *nextp;              /* not used */
+    afs_uint32 magic;                  /* a magic value to detect bad entries */
     unsigned long refCount;            /* the reference count */
     long flags;                                /* the flags, if any; locked by mx */
     osi_mutex_t mx;                    /* the mutex */
@@ -213,6 +214,7 @@ typedef struct smb_vc {
     unsigned short session;            /* This is the Session Index associated with the NCBs */
 } smb_vc_t;
 
+#define SMB_VC_MAGIC ('S' | 'C'<<8 | 'A'<<16 | 'C'<<24)
                                        /* have we negotiated ... */
 #define SMB_VCFLAG_USEV3       1       /* ... version 3 of the protocol */
 #define SMB_VCFLAG_USECORE     2       /* ... the core protocol */