From 044e8557cd32cecbec95d2b6ff1d516a37e48dcb Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Sun, 12 Feb 2006 06:24:27 +0000 Subject: [PATCH] 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) --- src/WINNT/afsd/cm_cell.c | 15 +++--- src/WINNT/afsd/smb.c | 99 +++++++++++++++++++++------------------- src/WINNT/afsd/smb.h | 14 +++--- 3 files changed, 65 insertions(+), 63 deletions(-) diff --git a/src/WINNT/afsd/cm_cell.c b/src/WINNT/afsd/cm_cell.c index a23e3b140..604a60d12 100644 --- 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 c573fd432..6b66335d2 100644 --- 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 866344b61..ed800b1fe 100644 --- 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) */ -- 2.39.5