From 86ec92d8077c7ed037079b15f29e6e68badad512 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Sun, 9 Apr 2006 05:56:36 +0000 Subject: [PATCH] STABLE14-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. (cherry picked from commit b9f22f0b7b9bf9aa746d3ef8ea63465b1cdadb97) --- src/WINNT/afsd/smb.c | 91 ++++++++++++++++++++++++++++++-------------- src/WINNT/afsd/smb.h | 2 + 2 files changed, 64 insertions(+), 29 deletions(-) diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c index b335ef225..efed6f12b 100644 --- a/src/WINNT/afsd/smb.c +++ b/src/WINNT/afsd/smb.c @@ -807,6 +807,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; @@ -818,6 +822,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; @@ -885,24 +890,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) { - /* remove VCP from smb_deadVCsp */ - for (vcpp = &smb_deadVCsp; *vcpp; vcpp = &((*vcpp)->nextp)) { - if (*vcpp == vcp) { - *vcpp = vcp->nextp; - break; - } - } - lock_FinalizeMutex(&vcp->mx); - memset(vcp,0,sizeof(smb_vc_t)); - free(vcp); + 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; + } + } + 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++; + } } } @@ -950,6 +971,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); @@ -1005,20 +1040,10 @@ 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); } @@ -3240,6 +3265,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) @@ -8991,7 +9020,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 ed800b1fe..acd5d6669 100644 --- 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 */ -- 2.39.5