Windows: prevent race assigning Fcb in AFSInitFcb()

AFSInitFcb() is executed when the ObjectInformation->Fcb pointer
is NULL.  More than one thread can make that determination at the
same time.  Use InterlockedCompareExchangePointer() to detect
a race and permit cleanup to be performed.

Remove the output parameter of AFSInitFcb() to avoid a double
assignment.

Change-Id: I3870cccd5cd5e95134446523cce3547a2135d5e3
Reviewed-on: http://gerrit.openafs.org/6562
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 9814049..5c2f9a2 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp
@@ -1896,10 +1896,12 @@ AFSProcessCreate( IN PIRP               Irp,
             // Allocate and initialize the Fcb for the file.
             //
 
-            ntStatus = AFSInitFcb( pDirEntry,
-                                   Fcb);
+            ntStatus = AFSInitFcb( pDirEntry);
 
-            if( !NT_SUCCESS( ntStatus))
+            *Fcb = pObjectInfo->Fcb;
+
+            if( !NT_SUCCESS( ntStatus) &&
+                ntStatus != STATUS_REPARSE)
             {
 
                 AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING,
@@ -1912,7 +1914,13 @@ AFSProcessCreate( IN PIRP               Irp,
                 try_return( ntStatus);
             }
 
-            bAllocatedFcb = TRUE;
+            if ( ntStatus != STATUS_REPARSE)
+            {
+
+                bAllocatedFcb = TRUE;
+            }
+
+            ntStatus = STATUS_SUCCESS;
         }
 
         bReleaseFcb = TRUE;
@@ -2252,10 +2260,12 @@ AFSOpenTargetDirectory( IN PIRP Irp,
             // Allocate and initialize the Fcb for the file.
             //
 
-            ntStatus = AFSInitFcb( ParentDirectoryCB,
-                                   Fcb);
+            ntStatus = AFSInitFcb( ParentDirectoryCB);
 
-            if( !NT_SUCCESS( ntStatus))
+            *Fcb = pParentObject->Fcb;
+
+            if( !NT_SUCCESS( ntStatus) &&
+                ntStatus != STATUS_REPARSE)
             {
 
                 AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING,
@@ -2268,7 +2278,13 @@ AFSOpenTargetDirectory( IN PIRP Irp,
                 try_return( ntStatus);
             }
 
-            bAllocatedFcb = TRUE;
+            if ( ntStatus == STATUS_REPARSE)
+            {
+
+                bAllocatedFcb = TRUE;
+            }
+
+            ntStatus = STATUS_SUCCESS;
         }
 
         bReleaseFcb = TRUE;
@@ -2579,10 +2595,10 @@ AFSProcessOpen( IN PIRP Irp,
         if( pObjectInfo->Fcb == NULL)
         {
 
-            ntStatus = AFSInitFcb( DirectoryCB,
-                                   &pObjectInfo->Fcb);
+            ntStatus = AFSInitFcb( DirectoryCB);
 
-            if( !NT_SUCCESS( ntStatus))
+            if( !NT_SUCCESS( ntStatus) &&
+                ntStatus != STATUS_REPARSE)
             {
 
                 AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING,
@@ -2595,7 +2611,13 @@ AFSProcessOpen( IN PIRP Irp,
                 try_return( ntStatus);
             }
 
-            bAllocatedFcb = TRUE;
+            if ( ntStatus != STATUS_REPARSE)
+            {
+
+                bAllocatedFcb = TRUE;
+            }
+
+            ntStatus = STATUS_SUCCESS;
         }
         else
         {
@@ -3068,10 +3090,12 @@ AFSProcessOverwriteSupersede( IN PDEVICE_OBJECT DeviceObject,
         if( pObjectInfo->Fcb == NULL)
         {
 
-            ntStatus = AFSInitFcb( DirectoryCB,
-                                   Fcb);
+            ntStatus = AFSInitFcb( DirectoryCB);
 
-            if( !NT_SUCCESS( ntStatus))
+            *Fcb = pObjectInfo->Fcb;
+
+            if( !NT_SUCCESS( ntStatus) &&
+                ntStatus != STATUS_REPARSE)
             {
 
                 AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING,
@@ -3084,7 +3108,13 @@ AFSProcessOverwriteSupersede( IN PDEVICE_OBJECT DeviceObject,
                 try_return( ntStatus);
             }
 
-            bAllocatedFcb = TRUE;
+            if ( ntStatus != STATUS_REPARSE)
+            {
+
+                bAllocatedFcb = TRUE;
+            }
+
+            ntStatus = STATUS_SUCCESS;
         }
         else
         {
@@ -3476,10 +3506,12 @@ AFSOpenIOCtlFcb( IN PIRP Irp,
             // Allocate and initialize the Fcb for the file.
             //
 
-            ntStatus = AFSInitFcb( pParentObjectInfo->Specific.Directory.PIOCtlDirectoryCB,
-                                   Fcb);
+            ntStatus = AFSInitFcb( pParentObjectInfo->Specific.Directory.PIOCtlDirectoryCB);
 
-            if( !NT_SUCCESS( ntStatus))
+            *Fcb = pParentObjectInfo->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->Fcb;
+
+            if( !NT_SUCCESS( ntStatus) &&
+                ntStatus != STATUS_REPARSE)
             {
 
                 AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING,
@@ -3491,7 +3523,13 @@ AFSOpenIOCtlFcb( IN PIRP Irp,
                 try_return( ntStatus);
             }
 
-            bAllocatedFcb = TRUE;
+            if ( ntStatus != STATUS_REPARSE)
+            {
+
+                bAllocatedFcb = TRUE;
+            }
+
+            ntStatus = STATUS_SUCCESS;
         }
         else
         {
@@ -3734,10 +3772,12 @@ AFSOpenSpecialShareFcb( IN PIRP Irp,
             // Allocate and initialize the Fcb for the file.
             //
 
-            ntStatus = AFSInitFcb( DirectoryCB,
-                                   Fcb);
+            ntStatus = AFSInitFcb( DirectoryCB);
 
-            if( !NT_SUCCESS( ntStatus))
+            *Fcb = DirectoryCB->ObjectInformation->Fcb;
+
+            if( !NT_SUCCESS( ntStatus) &&
+                ntStatus != STATUS_REPARSE)
             {
 
                 AFSDbgLogMsg( AFS_SUBSYSTEM_PIPE_PROCESSING,
@@ -3749,7 +3789,13 @@ AFSOpenSpecialShareFcb( IN PIRP Irp,
                 try_return( ntStatus);
             }
 
-            bAllocateFcb = TRUE;
+            if ( ntStatus != STATUS_REPARSE)
+            {
+
+                bAllocateFcb = TRUE;
+            }
+
+            ntStatus = STATUS_SUCCESS;
         }
         else
         {

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSFcbSupport.cpp b/src/WINNT/afsrdr/kernel/lib/AFSFcbSupport.cpp
index 73b9702..0ea85fa 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSFcbSupport.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSFcbSupport.cpp
@@ -51,8 +51,7 @@
 //
 
 NTSTATUS
-AFSInitFcb( IN AFSDirectoryCB  *DirEntry,
-            IN OUT AFSFcb     **Fcb)
+AFSInitFcb( IN AFSDirectoryCB  *DirEntry)
 {
 
     NTSTATUS ntStatus = STATUS_SUCCESS;
@@ -128,6 +127,7 @@ AFSInitFcb( IN AFSDirectoryCB  *DirEntry,
                        sizeof( AFSNonPagedFcb));
 
         pNPFcb->Size = sizeof( AFSNonPagedFcb);
+
         pNPFcb->Type = AFS_NON_PAGED_FCB;
 
         //
@@ -168,14 +168,6 @@ AFSInitFcb( IN AFSDirectoryCB  *DirEntry,
         pFcb->NPFcb = pNPFcb;
 
         //
-        // Initialize some fields in the Fcb
-        //
-
-        pFcb->ObjectInformation = pObjectInfo;
-
-        pObjectInfo->Fcb = pFcb;
-
-        //
         // Set type specific information
         //
 
@@ -280,26 +272,52 @@ AFSInitFcb( IN AFSDirectoryCB  *DirEntry,
         }
 
         //
-        // And return the Fcb
+        // Initialize some fields in the Fcb
         //
 
-        *Fcb = pFcb;
+        if ( InterlockedCompareExchangePointer( (PVOID *)&pObjectInfo->Fcb, pFcb, NULL) != NULL)
+        {
+
+            AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING,
+                          AFS_TRACE_LEVEL_WARNING,
+                          "AFSInitFcb Raced Fcb %08lX pFcb %08lX Name %wZ\n",
+                          pObjectInfo->Fcb,
+                          pFcb,
+                          &DirEntry->NameInformation.FileName);
+
+            AFSDbgLogMsg( AFS_SUBSYSTEM_LOCK_PROCESSING,
+                          AFS_TRACE_LEVEL_VERBOSE,
+                          "AFSInitFcb Acquiring Fcb lock %08lX EXCL %08lX\n",
+                          &pObjectInfo->Fcb->NPFcb->Resource,
+                          PsGetCurrentThread());
+
+            AFSAcquireExcl( &pObjectInfo->Fcb->NPFcb->Resource,
+                            TRUE);
+
+            try_return( ntStatus = STATUS_REPARSE);
+        }
+
+        pFcb->ObjectInformation = pObjectInfo;
 
         AFSDbgLogMsg( AFS_SUBSYSTEM_FCB_REF_COUNTING,
                       AFS_TRACE_LEVEL_VERBOSE,
                       "AFSInitFcb Initialized Fcb %08lX Name %wZ\n",
-                      pFcb,
+                      &pObjectInfo->Fcb,
                       &DirEntry->NameInformation.FileName);
 
 try_exit:
 
-        if( !NT_SUCCESS( ntStatus))
+        if( ntStatus != STATUS_SUCCESS)
         {
 
-            AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING,
-                          AFS_TRACE_LEVEL_ERROR,
-                          "AFSInitFcb Failed to initialize fcb Status %08lX\n",
-                          ntStatus);
+            if ( !NT_SUCCESS( ntStatus))
+            {
+
+                AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING,
+                              AFS_TRACE_LEVEL_ERROR,
+                              "AFSInitFcb Failed to initialize fcb Status %08lX\n",
+                              ntStatus);
+            }
 
             if( pFcb != NULL)
             {
@@ -330,12 +348,6 @@ try_exit:
 
                 AFSExFreePool( pNPFcb);
             }
-
-            if( Fcb != NULL)
-            {
-
-                *Fcb = NULL;
-            }
         }
     }
 

diff --git a/src/WINNT/afsrdr/kernel/lib/Include/AFSCommon.h b/src/WINNT/afsrdr/kernel/lib/Include/AFSCommon.h
index 9b57ff9..197bb6b 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/Include/AFSCommon.h
+++ b/src/WINNT/afsrdr/kernel/lib/Include/AFSCommon.h
@@ -494,8 +494,7 @@ AFSClose( IN PDEVICE_OBJECT DeviceObject,
 //
 
 NTSTATUS
-AFSInitFcb( IN AFSDirectoryCB   *DirEntry,
-            IN OUT AFSFcb     **Fcb);
+AFSInitFcb( IN AFSDirectoryCB   *DirEntry);
 
 NTSTATUS
 AFSInitVolume( IN GUID *AuthGroup,