windows: ObjectInformationCB.ObjectReferenceCount

The ObjectInformationCB.ObjectReferenceCount is protected by
the VolumeCB->ObjectInfoTree.TreeLock.   When the TreeLock is
dropped the reference count can change.  Hold the TreeLock across
both ObjectReferenceCount == 0 tests and the associated tear down
or repeat the ObjectReferenceCount == 0 test after the TreeLock
is reacquired.

Change-Id: I069c22ae8f3a93fad3ef9a662df5b4903b317897
Reviewed-on: http://gerrit.openafs.org/6959
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/AFSCleanup.cpp b/src/WINNT/afsrdr/kernel/lib/AFSCleanup.cpp
index af4576b..77bf7e9 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSCleanup.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSCleanup.cpp
@@ -644,6 +644,7 @@ AFSCleanup( IN PDEVICE_OBJECT LibDeviceObject,
                 if( BooleanFlagOn( pFcb->Flags, AFS_FCB_FLAG_PURGE_ON_CLOSE))
                 {
                     InterlockedIncrement( &pObjectInfo->ObjectReferenceCount);
+
                     ClearFlag( pFcb->Flags, AFS_FCB_FLAG_PURGE_ON_CLOSE);
 
                     AFSPerformObjectInvalidate( pObjectInfo,

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp b/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp
index 4bd9ea6..fbdfa31 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp
@@ -1958,6 +1958,9 @@ AFSInvalidateCache( IN AFSInvalidateCacheCB *InvalidateCB)
             try_return( ntStatus);
         }
 
+        AFSAcquireShared( pVolumeCB->ObjectInfoTree.TreeLock,
+                          TRUE);
+
         if ( AFSIsVolumeFID( &InvalidateCB->FileID))
         {
 
@@ -1966,9 +1969,6 @@ AFSInvalidateCache( IN AFSInvalidateCacheCB *InvalidateCB)
         else
         {
 
-            AFSAcquireShared( pVolumeCB->ObjectInfoTree.TreeLock,
-                              TRUE);
-
             lCount = InterlockedDecrement( &pVolumeCB->VolumeReferenceCount);
 
             AFSDbgLogMsg( AFS_SUBSYSTEM_VOLUME_REF_COUNTING,
@@ -1982,9 +1982,6 @@ AFSInvalidateCache( IN AFSInvalidateCacheCB *InvalidateCB)
             ntStatus = AFSLocateHashEntry( pVolumeCB->ObjectInfoTree.TreeHead,
                                            ullIndex,
                                            (AFSBTreeEntry **)&pObjectInfo);
-
-            AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock);
-
         }
 
         if( pObjectInfo != NULL)
@@ -2003,6 +2000,8 @@ AFSInvalidateCache( IN AFSInvalidateCacheCB *InvalidateCB)
                           lCount);
         }
 
+        AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock);
+
         if( !NT_SUCCESS( ntStatus) ||
             pObjectInfo == NULL)
         {

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp b/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp
index 6ad1031..168d060 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp
@@ -1184,40 +1184,49 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
 
                             AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock);
 
+                            //
+                            // Dropping the TreeLock permits the
+                            // pCurrentObject->ObjectReferenceCount to change
+                            //
+
                             if( AFSAcquireExcl( pVolumeCB->ObjectInfoTree.TreeLock,
                                                 FALSE))
                             {
 
-                                if( pCurrentObject->Fcb != NULL)
+                                if ( pCurrentObject->ObjectReferenceCount == 0)
                                 {
 
-                                    AFSRemoveFcb( &pCurrentObject->Fcb);
-                                }
+                                    if( pCurrentObject->Fcb != NULL)
+                                    {
 
-                                if( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB != NULL)
-                                {
+                                        AFSRemoveFcb( &pCurrentObject->Fcb);
+                                    }
 
-                                    if( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->Fcb != NULL)
+                                    if( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB != NULL)
                                     {
 
-                                        AFSRemoveFcb( &pCurrentObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->Fcb);
-                                    }
+                                        if( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->Fcb != NULL)
+                                        {
 
-                                    AFSDeleteObjectInfo( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation);
+                                            AFSRemoveFcb( &pCurrentObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->Fcb);
+                                        }
 
-                                    ExDeleteResourceLite( &pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->NonPaged->Lock);
+                                        AFSDeleteObjectInfo( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation);
 
-                                    AFSExFreePool( pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->NonPaged);
+                                        ExDeleteResourceLite( &pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->NonPaged->Lock);
 
-                                    AFSExFreePool( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB);
-                                }
+                                        AFSExFreePool( pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->NonPaged);
 
-                                AFSDbgLogMsg( AFS_SUBSYSTEM_CLEANUP_PROCESSING,
-                                              AFS_TRACE_LEVEL_VERBOSE,
-                                              "AFSPrimaryVolumeWorkerThread Deleting deleted object %08lX\n",
-                                              pCurrentObject);
+                                        AFSExFreePool( pCurrentObject->Specific.Directory.PIOCtlDirectoryCB);
+                                    }
 
-                                AFSDeleteObjectInfo( pCurrentObject);
+                                    AFSDbgLogMsg( AFS_SUBSYSTEM_CLEANUP_PROCESSING,
+                                                  AFS_TRACE_LEVEL_VERBOSE,
+                                                  "AFSPrimaryVolumeWorkerThread Deleting deleted object %08lX\n",
+                                                  pCurrentObject);
+
+                                    AFSDeleteObjectInfo( pCurrentObject);
+                                }
 
                                 AFSConvertToShared( pVolumeCB->ObjectInfoTree.TreeLock);
 
@@ -1431,6 +1440,11 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
 
                                             AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock);
 
+                                            //
+                                            // Dropping the TreeLock permits the
+                                            // pCurrentObject->ObjectReferenceCount to change
+                                            //
+
                                             AFSCleanupFcb( pFcb,
                                                            TRUE);
 
@@ -1462,7 +1476,6 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
                                               pCurrentObject->FileId.Vnode,
                                               pCurrentObject->FileId.Unique);
 
-
                                 //
                                 // Clear our enumerated flag on this object so we retrieve info again on next access
                                 //
@@ -1470,6 +1483,8 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
                                 ClearFlag( pCurrentObject->Flags, AFS_OBJECT_FLAGS_DIRECTORY_ENUMERATED);
 
                                 AFSReleaseResource( pCurrentObject->Specific.Directory.DirectoryNodeHdr.TreeLock);
+
+                                AFSConvertToShared( pVolumeCB->ObjectInfoTree.TreeLock);
                             }
                             else
                             {
@@ -1480,8 +1495,6 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
 
                                 break;
                             }
-
-                            AFSConvertToShared( pVolumeCB->ObjectInfoTree.TreeLock);
                         }
                         else
                         {
@@ -1536,6 +1549,11 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
 
                                 AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock);
 
+                                //
+                                // Dropping the TreeLock permits the
+                                // pCurrentObject->ObjectReferenceCount to change
+                                //
+
                                 AFSCleanupFcb( pFcb,
                                                TRUE);
 
@@ -1572,6 +1590,11 @@ AFSPrimaryVolumeWorkerThread( IN PVOID Context)
 
                             AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock);
 
+                            //
+                            // Dropping the TreeLock permits the
+                            // pCurrentObject->ObjectReferenceCount to change
+                            //
+
                             AFSCleanupFcb( pCurrentObject->Fcb,
                                            FALSE);