Windows: Hold Fcb references prior to service call

If the Fcb reference count hits 0 while the service is called
it is possible that the Fcb can be garbage collected prior to
the completion of the call.

Change-Id: I32c3c5e3debb246fe63ac6f6cc5625b493ee47a9
Reviewed-on: http://gerrit.openafs.org/6660
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com>
Tested-by: Jeffrey Altman <jaltman@secure-endpoints.com>

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp b/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp
index c9b14ba2db9b00a15e26b28fb1af5ef290926cc8..63fae848dbd2ddcd295c7f55c4d35f1a606d692d 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp
@@ -1900,8 +1900,7 @@ AFSProcessCreate( IN PIRP               Irp,
 
             *Fcb = pObjectInfo->Fcb;
 
-            if( !NT_SUCCESS( ntStatus) &&
-                ntStatus != STATUS_REPARSE)
+            if( !NT_SUCCESS( ntStatus))
             {
 
                 AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING,
@@ -1923,6 +1922,18 @@ AFSProcessCreate( IN PIRP               Irp,
             ntStatus = STATUS_SUCCESS;
         }
 
+        //
+        // Increment the open count on this Fcb
+        //
+
+        lCount = InterlockedIncrement( &(*Fcb)->OpenReferenceCount);
+
+        AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
+                      AFS_TRACE_LEVEL_VERBOSE,
+                      "AFSProcessCreate Increment count on Fcb %08lX Cnt %d\n",
+                      *Fcb,
+                      lCount);
+
         bReleaseFcb = TRUE;
 
         //
@@ -2035,18 +2046,6 @@ AFSProcessCreate( IN PIRP               Irp,
                           pFileObject,
                           &(*Fcb)->ShareAccess);
 
-        //
-        // Increment the open count on this Fcb
-        //
-
-        lCount = InterlockedIncrement( &(*Fcb)->OpenReferenceCount);
-
-        AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
-                      AFS_TRACE_LEVEL_VERBOSE,
-                      "AFSProcessCreate Increment count on Fcb %08lX Cnt %d\n",
-                      *Fcb,
-                      lCount);
-
         lCount = InterlockedIncrement( &(*Fcb)->OpenHandleCount);
 
         AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
@@ -2113,6 +2112,21 @@ try_exit:
         if( bReleaseFcb)
         {
 
+            if( !NT_SUCCESS( ntStatus))
+            {
+                //
+                // Decrement the open count on this Fcb
+                //
+
+                lCount = InterlockedDecrement( &(*Fcb)->OpenReferenceCount);
+
+                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
+                              AFS_TRACE_LEVEL_VERBOSE,
+                              "AFSProcessCreate Decrement count on Fcb %08lX Cnt %d\n",
+                              *Fcb,
+                              lCount);
+            }
+
             AFSReleaseResource( &(*Fcb)->NPFcb->Resource);
         }
 
@@ -2264,8 +2278,7 @@ AFSOpenTargetDirectory( IN PIRP Irp,
 
             *Fcb = pParentObject->Fcb;
 
-            if( !NT_SUCCESS( ntStatus) &&
-                ntStatus != STATUS_REPARSE)
+            if( !NT_SUCCESS( ntStatus))
             {
 
                 AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING,
@@ -2287,6 +2300,18 @@ AFSOpenTargetDirectory( IN PIRP Irp,
             ntStatus = STATUS_SUCCESS;
         }
 
+        //
+        // Increment the open count on this Fcb
+        //
+
+        lCount = InterlockedIncrement( &pParentObject->Fcb->OpenReferenceCount);
+
+        AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
+                      AFS_TRACE_LEVEL_VERBOSE,
+                      "AFSOpenTargetDirectory Increment count on Fcb %08lX Cnt %d\n",
+                      pParentObject->Fcb,
+                      lCount);
+
         bReleaseFcb = TRUE;
 
         //
@@ -2397,18 +2422,6 @@ AFSOpenTargetDirectory( IN PIRP Irp,
                               &pParentObject->Fcb->ShareAccess);
         }
 
-        //
-        // Increment the open count on this Fcb
-        //
-
-        lCount = InterlockedIncrement( &pParentObject->Fcb->OpenReferenceCount);
-
-        AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
-                      AFS_TRACE_LEVEL_VERBOSE,
-                      "AFSOpenTargetDirectory Increment count on Fcb %08lX Cnt %d\n",
-                      pParentObject->Fcb,
-                      lCount);
-
         lCount = InterlockedIncrement( &pParentObject->Fcb->OpenHandleCount);
 
         AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
@@ -2446,6 +2459,21 @@ try_exit:
         if( bReleaseFcb)
         {
 
+            if( !NT_SUCCESS( ntStatus))
+            {
+                //
+                // Decrement the open count on this Fcb
+                //
+
+                lCount = InterlockedDecrement( &pParentObject->Fcb->OpenReferenceCount);
+
+                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
+                              AFS_TRACE_LEVEL_VERBOSE,
+                              "AFSOpenTargetDirectory Decrement count on Fcb %08lX Cnt %d\n",
+                              pParentObject->Fcb,
+                              lCount);
+            }
+
             AFSReleaseResource( &pParentObject->Fcb->NPFcb->Resource);
         }
 
@@ -2597,8 +2625,7 @@ AFSProcessOpen( IN PIRP Irp,
 
             ntStatus = AFSInitFcb( DirectoryCB);
 
-            if( !NT_SUCCESS( ntStatus) &&
-                ntStatus != STATUS_REPARSE)
+            if( !NT_SUCCESS( ntStatus))
             {
 
                 AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING,
@@ -2626,20 +2653,20 @@ AFSProcessOpen( IN PIRP Irp,
                             TRUE);
         }
 
-        bReleaseFcb = TRUE;
-
         //
-        // Reference the Fcb so it won't go away while we call into the service for processing
+        // Increment the open count on this Fcb
         //
 
         lCount = InterlockedIncrement( &pObjectInfo->Fcb->OpenReferenceCount);
 
         AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
                       AFS_TRACE_LEVEL_VERBOSE,
-                      "AFSProcessOpen Increment count on Fcb %08lX Cnt %d\n",
+                      "AFSProcessOpen Increment2 count on Fcb %08lX Cnt %d\n",
                       pObjectInfo->Fcb,
                       lCount);
 
+        bReleaseFcb = TRUE;
+
         //
         // Check access on the entry
         //
@@ -2869,18 +2896,6 @@ AFSProcessOpen( IN PIRP Irp,
                               &pObjectInfo->Fcb->ShareAccess);
         }
 
-        //
-        // Increment the open count on this Fcb
-        //
-
-        lCount = InterlockedIncrement( &pObjectInfo->Fcb->OpenReferenceCount);
-
-        AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
-                      AFS_TRACE_LEVEL_VERBOSE,
-                      "AFSProcessOpen Increment2 count on Fcb %08lX Cnt %d\n",
-                      pObjectInfo->Fcb,
-                      lCount);
-
         lCount = InterlockedIncrement( &pObjectInfo->Fcb->OpenHandleCount);
 
         AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
@@ -2945,17 +2960,20 @@ try_exit:
         if( bReleaseFcb)
         {
 
-            //
-            // Remove the reference we added initially
-            //
+            if( !NT_SUCCESS( ntStatus))
+            {
+                //
+                // Decrement the open count on this Fcb
+                //
 
-            lCount = InterlockedDecrement( &pObjectInfo->Fcb->OpenReferenceCount);
+                lCount = InterlockedDecrement( &pObjectInfo->Fcb->OpenReferenceCount);
 
-            AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
-                          AFS_TRACE_LEVEL_VERBOSE,
-                          "AFSProcessOpen Decrement count on Fcb %08lX Cnt %d\n",
-                          pObjectInfo->Fcb,
-                          lCount);
+                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
+                              AFS_TRACE_LEVEL_VERBOSE,
+                              "AFSProcessOpen Decrement2 count on Fcb %08lX Cnt %d\n",
+                              pObjectInfo->Fcb,
+                              lCount);
+            }
 
             AFSReleaseResource( pObjectInfo->Fcb->Header.Resource);
         }
@@ -3094,8 +3112,7 @@ AFSProcessOverwriteSupersede( IN PDEVICE_OBJECT DeviceObject,
 
             *Fcb = pObjectInfo->Fcb;
 
-            if( !NT_SUCCESS( ntStatus) &&
-                ntStatus != STATUS_REPARSE)
+            if( !NT_SUCCESS( ntStatus))
             {
 
                 AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING,
@@ -3123,20 +3140,20 @@ AFSProcessOverwriteSupersede( IN PDEVICE_OBJECT DeviceObject,
                             TRUE);
         }
 
-        bReleaseFcb = TRUE;
-
         //
-        // Reference the Fcb so it won't go away while processing the request
+        // Increment the open count on this Fcb.
         //
 
         lCount = InterlockedIncrement( &pObjectInfo->Fcb->OpenReferenceCount);
 
         AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
                       AFS_TRACE_LEVEL_VERBOSE,
-                      "AFSProcessOverwriteSupersede Increment count on Fcb %08lX Cnt %d\n",
+                      "AFSProcessOverwriteSupersede Increment2 count on Fcb %08lX Cnt %d\n",
                       pObjectInfo->Fcb,
                       lCount);
 
+        bReleaseFcb = TRUE;
+
         //
         // Check access on the entry
         //
@@ -3344,18 +3361,6 @@ AFSProcessOverwriteSupersede( IN PDEVICE_OBJECT DeviceObject,
             Irp->IoStatus.Information = FILE_OVERWRITTEN;
         }
 
-        //
-        // Increment the open count on this Fcb.
-        //
-
-        lCount = InterlockedIncrement( &pObjectInfo->Fcb->OpenReferenceCount);
-
-        AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
-                      AFS_TRACE_LEVEL_VERBOSE,
-                      "AFSProcessOverwriteSupersede Increment2 count on Fcb %08lX Cnt %d\n",
-                      pObjectInfo->Fcb,
-                      lCount);
-
         lCount = InterlockedIncrement( &pObjectInfo->Fcb->OpenHandleCount);
 
         AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
@@ -3397,17 +3402,20 @@ try_exit:
         if( bReleaseFcb)
         {
 
-            //
-            // Remove the reference we added above to prevent tear down
-            //
+            if( !NT_SUCCESS( ntStatus))
+            {
+                //
+                // Decrement the open count on this Fcb.
+                //
 
-            lCount = InterlockedDecrement( &pObjectInfo->Fcb->OpenReferenceCount);
+                lCount = InterlockedDecrement( &pObjectInfo->Fcb->OpenReferenceCount);
 
-            AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
-                          AFS_TRACE_LEVEL_VERBOSE,
-                          "AFSProcessOverwriteSupersede Decrement count on Fcb %08lX Cnt %d\n",
-                          pObjectInfo->Fcb,
-                          lCount);
+                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
+                              AFS_TRACE_LEVEL_VERBOSE,
+                              "AFSProcessOverwriteSupersede Decrement2 count on Fcb %08lX Cnt %d\n",
+                              pObjectInfo->Fcb,
+                              lCount);
+            }
 
             AFSReleaseResource( pObjectInfo->Fcb->Header.Resource);
         }
@@ -3510,8 +3518,7 @@ AFSOpenIOCtlFcb( IN PIRP Irp,
 
             *Fcb = pParentObjectInfo->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->Fcb;
 
-            if( !NT_SUCCESS( ntStatus) &&
-                ntStatus != STATUS_REPARSE)
+            if( !NT_SUCCESS( ntStatus))
             {
 
                 AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING,
@@ -3540,6 +3547,18 @@ AFSOpenIOCtlFcb( IN PIRP Irp,
                             TRUE);
         }
 
+        //
+        // Increment the open reference and handle on the node
+        //
+
+        lCount = InterlockedIncrement( &(*Fcb)->OpenReferenceCount);
+
+        AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
+                      AFS_TRACE_LEVEL_VERBOSE,
+                      "AFSOpenIOCtlFcb Increment count on Fcb %08lX Cnt %d\n",
+                      (*Fcb),
+                      lCount);
+
         bReleaseFcb = TRUE;
 
         //
@@ -3631,17 +3650,9 @@ AFSOpenIOCtlFcb( IN PIRP Irp,
                       lCount);
 
         //
-        // Increment the open reference and handle on the node
+        // Increment the handle on the node
         //
 
-        lCount = InterlockedIncrement( &(*Fcb)->OpenReferenceCount);
-
-        AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
-                      AFS_TRACE_LEVEL_VERBOSE,
-                      "AFSOpenIOCtlFcb Increment count on Fcb %08lX Cnt %d\n",
-                      (*Fcb),
-                      lCount);
-
         lCount = InterlockedIncrement( &(*Fcb)->OpenHandleCount);
 
         AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
@@ -3700,6 +3711,21 @@ try_exit:
         if( bReleaseFcb)
         {
 
+            if( !NT_SUCCESS( ntStatus))
+            {
+                //
+                // Decrement the open reference and handle on the node
+                //
+
+                lCount = InterlockedDecrement( &(*Fcb)->OpenReferenceCount);
+
+                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
+                              AFS_TRACE_LEVEL_VERBOSE,
+                              "AFSOpenIOCtlFcb Decrement count on Fcb %08lX Cnt %d\n",
+                              (*Fcb),
+                              lCount);
+            }
+
             AFSReleaseResource( &(*Fcb)->NPFcb->Resource);
         }
 
@@ -3776,8 +3802,7 @@ AFSOpenSpecialShareFcb( IN PIRP Irp,
 
             *Fcb = DirectoryCB->ObjectInformation->Fcb;
 
-            if( !NT_SUCCESS( ntStatus) &&
-                ntStatus != STATUS_REPARSE)
+            if( !NT_SUCCESS( ntStatus))
             {
 
                 AFSDbgLogMsg( AFS_SUBSYSTEM_PIPE_PROCESSING,
@@ -3806,6 +3831,18 @@ AFSOpenSpecialShareFcb( IN PIRP Irp,
                             TRUE);
         }
 
+        //
+        // Increment the open count on this Fcb
+        //
+
+        lCount = InterlockedIncrement( &(*Fcb)->OpenReferenceCount);
+
+        AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
+                      AFS_TRACE_LEVEL_VERBOSE,
+                      "AFSOpenSpecialShareFcb Increment count on Fcb %08lX Cnt %d\n",
+                      (*Fcb),
+                      lCount);
+
         bReleaseFcb = TRUE;
 
         //
@@ -3873,18 +3910,6 @@ AFSOpenSpecialShareFcb( IN PIRP Irp,
             try_return( ntStatus);
         }
 
-        //
-        // Increment the open count on this Fcb
-        //
-
-        lCount = InterlockedIncrement( &(*Fcb)->OpenReferenceCount);
-
-        AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
-                      AFS_TRACE_LEVEL_VERBOSE,
-                      "AFSOpenSpecialShareFcb Increment count on Fcb %08lX Cnt %d\n",
-                      (*Fcb),
-                      lCount);
-
         lCount = InterlockedIncrement( &(*Fcb)->OpenHandleCount);
 
         AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
@@ -3924,6 +3949,21 @@ try_exit:
         if( bReleaseFcb)
         {
 
+            if( !NT_SUCCESS( ntStatus))
+            {
+                //
+                // Decrement the open count on this Fcb
+                //
+
+                lCount = InterlockedDecrement( &(*Fcb)->OpenReferenceCount);
+
+                AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
+                              AFS_TRACE_LEVEL_VERBOSE,
+                              "AFSOpenSpecialShareFcb Decrement count on Fcb %08lX Cnt %d\n",
+                              (*Fcb),
+                              lCount);
+            }
+
             AFSReleaseResource( &(*Fcb)->NPFcb->Resource);
         }