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.

diff --git a/src/WINNT/afsd/afslogon.c b/src/WINNT/afsd/afslogon.c
index 9752eb6405c0b5ff57f9ef7bb7c5c028402d16eb..5fb3cca8e3c8b57508a22383f0c869b739fcf59e 100644 (file)
--- 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 96e183b5cb54caa4fe0500619f1cd4fea5159096..99e1708508d3f5c69d240ac3a0c37e22c737cf8d 100644 (file)
--- 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 c4a1d781df66923a0269812b99f03aeab97f0650..e922adb640171561d61d64e57bb7ab0c5e8afa46 100644 (file)
--- 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 f1828f9c984029f522ed1f016731dd30e854cd40..83aea1203fc0ce328da843edbbd46dfdf23a6646 100644 (file)
--- 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;
 }