From 21c576178b01382f411892c318130df429e38de8 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Thu, 26 Jan 2006 06:09:47 +0000 Subject: [PATCH] STABLE14-windows-smb_fid_t-audit-20060125 Further testing revealed that some smb_vc_t objects could not be freed because the associated smb_fid_t objects never reached a zero refcount. Additional auditing uncovered cases in which there were holds not being released and others in which they were released to many times. This patch fixes the problems and improves auditability by modifying the behavior of the smb_IoctlXXX() functions to not release a reference that was obtained by the caller. Now the caller releases the reference. --- src/WINNT/afsd/afslogon.c | 4 +-- src/WINNT/afsd/smb.c | 12 ++++++--- src/WINNT/afsd/smb3.c | 50 ++++++++++++++++++++++++++++---------- src/WINNT/afsd/smb_ioctl.c | 8 ------ 4 files changed, 47 insertions(+), 27 deletions(-) diff --git a/src/WINNT/afsd/afslogon.c b/src/WINNT/afsd/afslogon.c index 9752eb640..5fb3cca8e 100644 --- a/src/WINNT/afsd/afslogon.c +++ b/src/WINNT/afsd/afslogon.c @@ -256,7 +256,7 @@ BOOL IsServiceRunning (void) CloseServiceHandle (hManager); } - DebugEvent("AFS AfsLogon - Test Service Running","Return Code[%x] ?Running[%d]",Status.dwCurrentState,(Status.dwCurrentState == SERVICE_RUNNING)); + DebugEvent("AFS AfsLogon - Test Service Running Return Code[%x] ?Running[%d]",Status.dwCurrentState,(Status.dwCurrentState == SERVICE_RUNNING)); return (Status.dwCurrentState == SERVICE_RUNNING); } @@ -278,7 +278,7 @@ BOOL IsServiceStartPending (void) CloseServiceHandle (hManager); } - DebugEvent("AFS AfsLogon - Test Service Start Pending","Return Code[%x] ?Start Pending[%d]",Status.dwCurrentState,(Status.dwCurrentState == SERVICE_START_PENDING)); + DebugEvent("AFS AfsLogon - Test Service Start Pending Return Code[%x] ?Start Pending[%d]",Status.dwCurrentState,(Status.dwCurrentState == SERVICE_START_PENDING)); return (Status.dwCurrentState == SERVICE_START_PENDING); } diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c index 96e183b5c..99e170850 100644 --- a/src/WINNT/afsd/smb.c +++ b/src/WINNT/afsd/smb.c @@ -6248,9 +6248,11 @@ long smb_ReceiveCoreWrite(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) return CM_ERROR_BADFD; } - if (fidp->flags & SMB_FID_IOCTL) - return smb_IoctlWrite(fidp, vcp, inp, outp); - + if (fidp->flags & SMB_FID_IOCTL) { + code = smb_IoctlWrite(fidp, vcp, inp, outp); + smb_ReleaseFID(fidp); + return code; + } userp = smb_GetUser(vcp, inp); /* special case: 0 bytes transferred means truncate to this position */ @@ -6582,7 +6584,9 @@ long smb_ReceiveCoreRead(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) } if (fidp->flags & SMB_FID_IOCTL) { - return smb_IoctlRead(fidp, vcp, inp, outp); + code = smb_IoctlRead(fidp, vcp, inp, outp); + smb_ReleaseFID(fidp); + return code; } { diff --git a/src/WINNT/afsd/smb3.c b/src/WINNT/afsd/smb3.c index c4a1d781d..e922adb64 100644 --- a/src/WINNT/afsd/smb3.c +++ b/src/WINNT/afsd/smb3.c @@ -4857,6 +4857,8 @@ long smb_ReceiveV3LockingX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) fidp = smb_FindFID(vcp, fid, 0); if (!fidp || (fidp->flags & SMB_FID_IOCTL)) { osi_Log0(smb_logp, "smb_ReceiveV3Locking BadFD"); + if (fidp) + smb_ReleaseFID(fidp); return CM_ERROR_BADFD; } /* set inp->fid so that later read calls in same msg can find fid */ @@ -5086,6 +5088,8 @@ long smb_ReceiveV3GetAttributes(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t * fidp = smb_FindFID(vcp, fid, 0); if (!fidp || (fidp->flags & SMB_FID_IOCTL)) { + if (fidp) + smb_ReleaseFID(fidp); return CM_ERROR_BADFD; } @@ -5150,6 +5154,8 @@ long smb_ReceiveV3SetAttributes(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t * fidp = smb_FindFID(vcp, fid, 0); if (!fidp || (fidp->flags & SMB_FID_IOCTL)) { + if (fidp) + smb_ReleaseFID(fidp); return CM_ERROR_BADFD; } @@ -5235,7 +5241,9 @@ long smb_ReceiveV3ReadX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) inp->fid = fd; if (fidp->flags & SMB_FID_IOCTL) { - return smb_IoctlV3Read(fidp, vcp, inp, outp); + code = smb_IoctlV3Read(fidp, vcp, inp, outp); + smb_ReleaseFID(fidp); + return code; } userp = smb_GetUser(vcp, inp); @@ -5472,6 +5480,7 @@ long smb_ReceiveNTCreateX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) } if (baseFid == 0) { + baseFidp = NULL; baseDirp = cm_data.rootSCachep; code = smb_LookupTIDPath(vcp, ((smb_t *)inp)->tid, &tidPathp); if (code == CM_ERROR_TIDIPC) { @@ -5532,6 +5541,8 @@ long smb_ReceiveNTCreateX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) cm_ReleaseSCache(dscp); cm_ReleaseUser(userp); free(realPathp); + if (baseFidp) + smb_ReleaseFID(baseFidp); if ( WANTS_DFS_PATHNAMES(inp) ) return CM_ERROR_PATH_NOT_COVERED; else @@ -5548,6 +5559,8 @@ long smb_ReceiveNTCreateX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) cm_ReleaseSCache(dscp); cm_ReleaseUser(userp); free(realPathp); + if (baseFidp) + smb_ReleaseFID(baseFidp); return CM_ERROR_EXISTS; } } @@ -5561,6 +5574,8 @@ long smb_ReceiveNTCreateX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) cm_ReleaseSCache(scp); cm_ReleaseUser(userp); free(realPathp); + if (baseFidp) + smb_ReleaseFID(baseFidp); if ( WANTS_DFS_PATHNAMES(inp) ) return CM_ERROR_PATH_NOT_COVERED; else @@ -5597,6 +5612,8 @@ long smb_ReceiveNTCreateX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) cm_ReleaseSCache(dscp); cm_ReleaseUser(userp); free(realPathp); + if (baseFidp) + smb_ReleaseFID(baseFidp); if ( WANTS_DFS_PATHNAMES(inp) ) return CM_ERROR_PATH_NOT_COVERED; else @@ -5613,7 +5630,7 @@ long smb_ReceiveNTCreateX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) treeStartp = realPathp + (tp - spacep->data); if (*tp && !smb_IsLegalFilename(tp)) { - if (baseFid != 0) + if (baseFidp) smb_ReleaseFID(baseFidp); cm_ReleaseUser(userp); free(realPathp); @@ -5629,7 +5646,7 @@ long smb_ReceiveNTCreateX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) /* we might have scp and we might have dscp */ - if (baseFid != 0) + if (baseFidp) smb_ReleaseFID(baseFidp); if (code) { @@ -5691,7 +5708,7 @@ long smb_ReceiveNTCreateX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) /* we have scp and dscp */ } else { /* we have scp but not dscp */ - if (baseFid != 0) + if (baseFidp) smb_ReleaseFID(baseFidp); } @@ -6196,6 +6213,7 @@ long smb_ReceiveNTTranCreate(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *out } if (baseFid == 0) { + baseFidp = NULL; baseDirp = cm_data.rootSCachep; code = smb_LookupTIDPath(vcp, ((smb_t *)inp)->tid, &tidPathp); if (code == CM_ERROR_TIDIPC) { @@ -6214,7 +6232,7 @@ long smb_ReceiveNTTranCreate(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *out } else { baseFidp = smb_FindFID(vcp, baseFid, 0); if (!baseFidp) { - osi_Log1(smb_logp, "NTTranCreate Invalid fid [%d]", baseFid); + osi_Log1(smb_logp, "NTTranCreate Invalid fid [%d]", baseFid); free(realPathp); cm_ReleaseUser(userp); return CM_ERROR_INVAL; @@ -6253,6 +6271,8 @@ long smb_ReceiveNTTranCreate(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *out cm_ReleaseSCache(dscp); cm_ReleaseUser(userp); free(realPathp); + if (baseFidp) + smb_ReleaseFID(baseFidp); if ( WANTS_DFS_PATHNAMES(inp) ) return CM_ERROR_PATH_NOT_COVERED; else @@ -6269,6 +6289,8 @@ long smb_ReceiveNTTranCreate(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *out cm_ReleaseSCache(dscp); cm_ReleaseUser(userp); free(realPathp); + if (baseFidp) + smb_ReleaseFID(baseFidp); return CM_ERROR_EXISTS; } } @@ -6282,6 +6304,8 @@ long smb_ReceiveNTTranCreate(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *out cm_ReleaseSCache(scp); cm_ReleaseUser(userp); free(realPathp); + if (baseFidp) + smb_ReleaseFID(baseFidp); if ( WANTS_DFS_PATHNAMES(inp) ) return CM_ERROR_PATH_NOT_COVERED; else @@ -6304,6 +6328,8 @@ long smb_ReceiveNTTranCreate(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *out cm_ReleaseSCache(dscp); cm_ReleaseUser(userp); free(realPathp); + if (baseFidp) + smb_ReleaseFID(baseFidp); if ( WANTS_DFS_PATHNAMES(inp) ) return CM_ERROR_PATH_NOT_COVERED; else @@ -6315,10 +6341,8 @@ long smb_ReceiveNTTranCreate(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *out cm_FreeSpace(spacep); - if (baseFid != 0) { + if (baseFidp) smb_ReleaseFID(baseFidp); - baseFidp = 0; - } if (code) { cm_ReleaseUser(userp); @@ -6326,8 +6350,10 @@ long smb_ReceiveNTTranCreate(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *out return code; } - if (!lastNamep) lastNamep = realPathp; - else lastNamep++; + if (!lastNamep) + lastNamep = realPathp; + else + lastNamep++; if (!smb_IsLegalFilename(lastNamep)) return CM_ERROR_BADNTFILENAME; @@ -6349,10 +6375,8 @@ long smb_ReceiveNTTranCreate(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *out } } } else { - if (baseFid != 0) { + if (baseFidp) smb_ReleaseFID(baseFidp); - baseFidp = 0; - } cm_FreeSpace(spacep); } diff --git a/src/WINNT/afsd/smb_ioctl.c b/src/WINNT/afsd/smb_ioctl.c index f1828f9c9..83aea1203 100644 --- a/src/WINNT/afsd/smb_ioctl.c +++ b/src/WINNT/afsd/smb_ioctl.c @@ -231,7 +231,6 @@ long smb_IoctlRead(smb_fid_t *fidp, smb_vc_t *vcp, smb_packet_t *inp, iop->outCopied += count; cm_ReleaseUser(userp); - smb_ReleaseFID(fidp); return 0; } @@ -274,7 +273,6 @@ long smb_IoctlWrite(smb_fid_t *fidp, smb_vc_t *vcp, smb_packet_t *inp, smb_packe smb_SetSMBDataLength(outp, 0); } - smb_ReleaseFID(fidp); return code; } @@ -315,7 +313,6 @@ long smb_IoctlV3Read(smb_fid_t *fidp, smb_vc_t *vcp, smb_packet_t *inp, smb_pack if (uidp) smb_ReleaseUID(uidp); cm_ReleaseUser(userp); - smb_ReleaseFID(fidp); return CM_ERROR_NOSUCHPATH; } @@ -326,7 +323,6 @@ long smb_IoctlV3Read(smb_fid_t *fidp, smb_vc_t *vcp, smb_packet_t *inp, smb_pack } if (code) { cm_ReleaseUser(userp); - smb_ReleaseFID(fidp); return code; } @@ -366,7 +362,6 @@ long smb_IoctlV3Read(smb_fid_t *fidp, smb_vc_t *vcp, smb_packet_t *inp, smb_pack /* and cleanup things */ cm_ReleaseUser(userp); - smb_ReleaseFID(fidp); return 0; } @@ -421,14 +416,12 @@ long smb_IoctlReadRaw(smb_fid_t *fidp, smb_vc_t *vcp, smb_packet_t *inp, code = smb_LookupTIDPath(vcp, ((smb_t *)inp)->tid, &iop->tidPathp); if(code) { cm_ReleaseUser(userp); - smb_ReleaseFID(fidp); return CM_ERROR_NOSUCHPATH; } code = smb_IoctlPrepareRead(fidp, iop, userp); if (code) { cm_ReleaseUser(userp); - smb_ReleaseFID(fidp); return code; } @@ -457,7 +450,6 @@ long smb_IoctlReadRaw(smb_fid_t *fidp, smb_vc_t *vcp, smb_packet_t *inp, osi_Log1(afsd_logp, "ReadRaw send failure code %d", code); cm_ReleaseUser(userp); - smb_ReleaseFID(fidp); return 0; } -- 2.39.5