From: Jeffrey Altman Date: Sat, 21 Feb 2009 04:26:43 +0000 (+0000) Subject: windows-smb-misc-20090220 X-Git-Tag: openafs-devel-1_5_61~514 X-Git-Url: https://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=8382f8ccb1c14f32318b3d1c587b417cce4c721c windows-smb-misc-20090220 LICENSE MIT Fix smb_FindFIDByScache() to avoid obtaining the smb_fid_t.mx and smb_rctLock out of order. Doing so requires obtaining references on each smb_fid_t belonging to the smb_vc_t in order to prevent them from being removed from the list while the list is being walked. Reorder tests for CM_SCACHEFLAG_DELETED and smb_fid_t.scp to make them more efficient and consistent. When processing Tran2SetPathInfo do not fail because an smb_fid_t cannot be found for the path object. The PathInfo function is being used because we do not have a file descriptor. Most importantly do not fail by returning success. --- diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c index 87a186b..ed8a943 100644 --- a/src/WINNT/afsd/smb.c +++ b/src/WINNT/afsd/smb.c @@ -1666,30 +1666,61 @@ smb_fid_t *smb_FindFID(smb_vc_t *vcp, unsigned short fid, int flags) return fidp; } + +/* Must not be called with scp->rw held because smb_ReleaseFID might be called */ #ifdef DEBUG_SMB_REFCOUNT smb_fid_t *smb_FindFIDByScacheDbg(smb_vc_t *vcp, cm_scache_t * scp, char *file, long line) #else smb_fid_t *smb_FindFIDByScache(smb_vc_t *vcp, cm_scache_t * scp) #endif { - smb_fid_t *fidp = NULL; - int newFid = 0; + smb_fid_t *fidp = NULL, *nextp = NULL; if (!scp) return NULL; + /* + * If the fidp->scp changes out from under us then + * we must not grab a refCount. It means the *fidp + * was processed by smb_CloseFID() and the *fidp is + * no longer valid for use. + */ lock_ObtainWrite(&smb_rctLock); - for(fidp = vcp->fidsp; fidp; fidp = (smb_fid_t *) osi_QNext(&fidp->q)) { + for(fidp = vcp->fidsp, (fidp ? fidp->refCount++ : 0); fidp; fidp = nextp, nextp = NULL) { + nextp = (smb_fid_t *) osi_QNext(&fidp->q); + if (nextp) + nextp->refCount++; + if (scp == fidp->scp) { + lock_ReleaseWrite(&smb_rctLock); lock_ObtainMutex(&fidp->mx); + lock_ObtainWrite(&smb_rctLock); if (scp == fidp->scp) { - fidp->refCount++; lock_ReleaseMutex(&fidp->mx); break; } lock_ReleaseMutex(&fidp->mx); } + + if (fidp->refCount > 1) { + fidp->refCount--; + } else { + lock_ReleaseWrite(&smb_rctLock); + smb_ReleaseFID(fidp); + lock_ObtainWrite(&smb_rctLock); + } + } + + if (nextp) { + if (nextp->refCount > 1) { + nextp->refCount--; + } else { + lock_ReleaseWrite(&smb_rctLock); + smb_ReleaseFID(nextp); + lock_ObtainWrite(&smb_rctLock); + } } + #ifdef DEBUG_SMB_REFCOUNT if (fidp) { afsi_log("%s:%d smb_FindFIDByScache fidp 0x%p ref %d", file, line, fidp, fidp->refCount); @@ -1697,7 +1728,7 @@ smb_fid_t *smb_FindFIDByScache(smb_vc_t *vcp, cm_scache_t * scp) } #endif lock_ReleaseWrite(&smb_rctLock); - return fidp; + return (fidp); } #ifdef DEBUG_SMB_REFCOUNT @@ -1715,8 +1746,8 @@ void smb_HoldFIDNoLock(smb_fid_t *fidp) } -/* smb_ReleaseFID cannot be called while an cm_scache_t mutex lock is held */ -/* the sm_fid_t->mx and smb_rctLock must not be held */ +/* smb_ReleaseFID cannot be called while a cm_scache_t rwlock is held */ +/* the smb_fid_t->mx and smb_rctLock must not be held */ #ifdef DEBUG_SMB_REFCOUNT void smb_ReleaseFIDDbg(smb_fid_t *fidp, char *file, long line) #else @@ -3500,7 +3531,13 @@ long smb_ReceiveCoreReadRaw(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp goto send1; lock_ObtainMutex(&fidp->mx); - if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) { + if (!fidp->scp) { + lock_ReleaseMutex(&fidp->mx); + smb_ReleaseFID(fidp); + return CM_ERROR_BADFD; + } + + if (fidp->scp->flags & CM_SCACHEFLAG_DELETED) { lock_ReleaseMutex(&fidp->mx); smb_CloseFID(vcp, fidp, NULL, 0); code = CM_ERROR_NOSUCHFILE; @@ -6523,22 +6560,22 @@ long smb_ReceiveCoreFlush(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) userp = smb_GetUserFromVCP(vcp, inp); lock_ObtainMutex(&fidp->mx); - if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) { - lock_ReleaseMutex(&fidp->mx); + if (!fidp->scp || (fidp->flags & SMB_FID_IOCTL)) { cm_ReleaseUser(userp); - smb_CloseFID(vcp, fidp, NULL, 0); + lock_ReleaseMutex(&fidp->mx); smb_ReleaseFID(fidp); - return CM_ERROR_NOSUCHFILE; + return CM_ERROR_BADFD; } - if (fidp->flags & SMB_FID_IOCTL) { - cm_ReleaseUser(userp); + if (fidp->scp->flags & CM_SCACHEFLAG_DELETED) { lock_ReleaseMutex(&fidp->mx); + cm_ReleaseUser(userp); + smb_CloseFID(vcp, fidp, NULL, 0); smb_ReleaseFID(fidp); - return CM_ERROR_BADFD; + return CM_ERROR_NOSUCHFILE; } - if (fidp->scp && (fidp->flags & SMB_FID_OPENWRITE) && smb_AsyncStore != 2) { + if ((fidp->flags & SMB_FID_OPENWRITE) && smb_AsyncStore != 2) { cm_scache_t * scp = fidp->scp; cm_HoldSCache(scp); lock_ReleaseMutex(&fidp->mx); @@ -7319,14 +7356,7 @@ long smb_ReceiveCoreWrite(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) return CM_ERROR_BADFD; } - lock_ObtainMutex(&fidp->mx); - if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) { - lock_ReleaseMutex(&fidp->mx); - smb_CloseFID(vcp, fidp, NULL, 0); - smb_ReleaseFID(fidp); - return CM_ERROR_NOSUCHFILE; - } - + lock_ObtainMutex(&fidp->mx); if (fidp->flags & SMB_FID_IOCTL) { lock_ReleaseMutex(&fidp->mx); code = smb_IoctlWrite(fidp, vcp, inp, outp); @@ -7341,6 +7371,13 @@ long smb_ReceiveCoreWrite(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) return CM_ERROR_BADFD; } + if (fidp->scp->flags & CM_SCACHEFLAG_DELETED) { + lock_ReleaseMutex(&fidp->mx); + smb_CloseFID(vcp, fidp, NULL, 0); + smb_ReleaseFID(fidp); + return CM_ERROR_NOSUCHFILE; + } + scp = fidp->scp; cm_HoldSCache(scp); lock_ReleaseMutex(&fidp->mx); @@ -7452,7 +7489,13 @@ void smb_CompleteWriteRaw(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp, fidp = smb_FindFID(vcp, fd, 0); lock_ObtainMutex(&fidp->mx); - if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) { + if (!fidp->scp) { + lock_ReleaseMutex(&fidp->mx); + smb_ReleaseFID(fidp); + return; + } + + if (fidp->scp->flags & CM_SCACHEFLAG_DELETED) { lock_ReleaseMutex(&fidp->mx); smb_CloseFID(vcp, fidp, NULL, 0); smb_ReleaseFID(fidp); @@ -7565,18 +7608,19 @@ long smb_ReceiveCoreWriteRaw(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *out return CM_ERROR_BADFD; lock_ObtainMutex(&fidp->mx); - if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) { + if (!fidp->scp) { lock_ReleaseMutex(&fidp->mx); - smb_CloseFID(vcp, fidp, NULL, 0); smb_ReleaseFID(fidp); - return CM_ERROR_NOSUCHFILE; + return CM_ERROR_BADFD; } - if (!fidp->scp) { + if (fidp->scp->flags & CM_SCACHEFLAG_DELETED) { lock_ReleaseMutex(&fidp->mx); + smb_CloseFID(vcp, fidp, NULL, 0); smb_ReleaseFID(fidp); - return CM_ERROR_BADFDOP; + return CM_ERROR_NOSUCHFILE; } + scp = fidp->scp; cm_HoldSCache(scp); lock_ReleaseMutex(&fidp->mx); @@ -7721,14 +7765,7 @@ long smb_ReceiveCoreRead(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) if (!fidp) return CM_ERROR_BADFD; - lock_ObtainMutex(&fidp->mx); - if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) { - lock_ReleaseMutex(&fidp->mx); - smb_CloseFID(vcp, fidp, NULL, 0); - smb_ReleaseFID(fidp); - return CM_ERROR_NOSUCHFILE; - } - + lock_ObtainMutex(&fidp->mx); if (fidp->flags & SMB_FID_IOCTL) { lock_ReleaseMutex(&fidp->mx); code = smb_IoctlRead(fidp, vcp, inp, outp); @@ -7739,8 +7776,16 @@ long smb_ReceiveCoreRead(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) if (!fidp->scp) { lock_ReleaseMutex(&fidp->mx); smb_ReleaseFID(fidp); - return CM_ERROR_BADFDOP; + return CM_ERROR_BADFD; + } + + if (fidp->scp->flags & CM_SCACHEFLAG_DELETED) { + lock_ReleaseMutex(&fidp->mx); + smb_CloseFID(vcp, fidp, NULL, 0); + smb_ReleaseFID(fidp); + return CM_ERROR_NOSUCHFILE; } + scp = fidp->scp; cm_HoldSCache(scp); lock_ReleaseMutex(&fidp->mx); @@ -8165,23 +8210,17 @@ long smb_ReceiveCoreSeek(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) return CM_ERROR_BADFD; lock_ObtainMutex(&fidp->mx); - if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) { - lock_ReleaseMutex(&fidp->mx); - smb_CloseFID(vcp, fidp, NULL, 0); - smb_ReleaseFID(fidp); - return CM_ERROR_NOSUCHFILE; - } - - if (fidp->flags & SMB_FID_IOCTL) { + if (!fidp->scp || (fidp->flags & SMB_FID_IOCTL)) { lock_ReleaseMutex(&fidp->mx); smb_ReleaseFID(fidp); return CM_ERROR_BADFD; } - if (!fidp->scp) { + if (fidp->scp->flags & CM_SCACHEFLAG_DELETED) { lock_ReleaseMutex(&fidp->mx); + smb_CloseFID(vcp, fidp, NULL, 0); smb_ReleaseFID(fidp); - return CM_ERROR_BADFDOP; + return CM_ERROR_NOSUCHFILE; } lock_ReleaseMutex(&fidp->mx); diff --git a/src/WINNT/afsd/smb3.c b/src/WINNT/afsd/smb3.c index dadaa91..fa27c50 100644 --- a/src/WINNT/afsd/smb3.c +++ b/src/WINNT/afsd/smb3.c @@ -3247,7 +3247,6 @@ long smb_ReceiveTran2SetPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet return CM_ERROR_BADOP; #else long code = 0; - smb_fid_t *fidp; unsigned short infoLevel; clientchar_t * pathp; smb_tran2Packet_t *outp; @@ -3369,25 +3368,6 @@ long smb_ReceiveTran2SetPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet return 0; } - fidp = smb_FindFIDByScache(vcp, scp); - if (!fidp) { - cm_ReleaseSCache(scp); - cm_ReleaseUser(userp); - smb_SendTran2Error(vcp, p, opx, code); - return 0; - } - - lock_ObtainMutex(&fidp->mx); - if (!(fidp->flags & SMB_FID_OPENWRITE)) { - lock_ReleaseMutex(&fidp->mx); - cm_ReleaseSCache(scp); - smb_ReleaseFID(fidp); - cm_ReleaseUser(userp); - smb_SendTran2Error(vcp, p, opx, CM_ERROR_NOACCESS); - return 0; - } - lock_ReleaseMutex(&fidp->mx); - outp = smb_GetTran2ResponsePacket(vcp, p, opx, 2, 0); outp->totalParms = 2; @@ -3410,10 +3390,6 @@ long smb_ReceiveTran2SetPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet } cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS); - lock_ReleaseWrite(&scp->rw); - lock_ObtainMutex(&fidp->mx); - lock_ObtainRead(&scp->rw); - /* prepare for setattr call */ attr.mask = CM_ATTRMASK_LENGTH; attr.length.LowPart = spi->u.QPstandardInfo.dataSize; @@ -3422,7 +3398,6 @@ long smb_ReceiveTran2SetPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet if (spi->u.QPstandardInfo.lastWriteDateTime != 0) { smb_UnixTimeFromSearchTime(&attr.clientModTime, spi->u.QPstandardInfo.lastWriteDateTime); attr.mask |= CM_ATTRMASK_CLIENTMODTIME; - fidp->flags |= SMB_FID_MTIMESETDONE; } if (spi->u.QPstandardInfo.attributes != 0) { @@ -3440,7 +3415,6 @@ long smb_ReceiveTran2SetPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet } } lock_ReleaseRead(&scp->rw); - lock_ReleaseMutex(&fidp->mx); /* call setattr */ if (attr.mask) @@ -3456,7 +3430,6 @@ long smb_ReceiveTran2SetPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet done: cm_ReleaseSCache(scp); cm_ReleaseUser(userp); - smb_ReleaseFID(fidp); if (code == 0) smb_SendTran2Packet(vcp, outp, opx); else @@ -3653,7 +3626,7 @@ long smb_ReceiveTran2SetFileInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet lock_ObtainMutex(&fidp->mx); if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) { - lock_ReleaseMutex(&fidp->mx); + lock_ReleaseMutex(&fidp->mx); smb_SendTran2Error(vcp, p, opx, CM_ERROR_NOSUCHFILE); smb_CloseFID(vcp, fidp, NULL, 0); smb_ReleaseFID(fidp); @@ -6683,7 +6656,7 @@ long smb_ReceiveV3ReadX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) return CM_ERROR_BADFD; } - lock_ObtainMutex(&fidp->mx); + lock_ObtainMutex(&fidp->mx); if (fidp->scp && (fidp->scp->flags & CM_SCACHEFLAG_DELETED)) { lock_ReleaseMutex(&fidp->mx); smb_CloseFID(vcp, fidp, NULL, 0);