Windows: AFSDeleteDirEntry set input to NULL

AFSDeleteDirEntry() frees the memory allocated to the DirectoryCB.
To ensure that an invalid memory pointer is not accidentally used
by the caller after the memory is freed, use
InterlockedCompareExchangePointer() to set the input parameter to
NULL prior to destroying the DirectoryCB.

Change-Id: I2e92d4277d1f9baee164bfb941821aa11a1ad738
Reviewed-on: http://gerrit.openafs.org/9721
Reviewed-by: Peter Scott <pscott@kerneldrivers.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Tested-by: Jeffrey Altman <jaltman@your-file-system.com>

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSClose.cpp b/src/WINNT/afsrdr/kernel/lib/AFSClose.cpp
index 1dd061f..cddd8fb 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSClose.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSClose.cpp
@@ -418,7 +418,7 @@ AFSClose( IN PDEVICE_OBJECT LibDeviceObject,
                             //
 
                             AFSDeleteDirEntry( pParentObjectInfo,
-                                               pDirCB);
+                                               &pDirCB);
 
                             AFSAcquireShared( &pObjectInfo->NonPagedInfo->ObjectInfoLock,
                                               TRUE);

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSCommSupport.cpp b/src/WINNT/afsrdr/kernel/lib/AFSCommSupport.cpp
index 85285dc..f9edbbf 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSCommSupport.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSCommSupport.cpp
@@ -364,7 +364,7 @@ AFSEnumerateDirectory( IN GUID *AuthGroup,
                                           pCurrentDirEntry->FileId.Unique));
 
                             AFSDeleteDirEntry( ObjectInfoCB,
-                                               pDirNode);
+                                               &pDirNode);
                         }
                         else
                         {
@@ -534,7 +534,7 @@ AFSEnumerateDirectory( IN GUID *AuthGroup,
                         //
 
                         AFSDeleteDirEntry( ObjectInfoCB,
-                                           pDirNode);
+                                           &pDirNode);
 
                         pCurrentDirEntry = (AFSDirEnumEntry *)((char *)pCurrentDirEntry + ulEntryLength);
 
@@ -1201,7 +1201,7 @@ AFSVerifyDirectoryContent( IN AFSObjectInfoCB *ObjectInfoCB,
                                       pCurrentDirEntry->FileId.Unique));
 
                         AFSDeleteDirEntry( ObjectInfoCB,
-                                           pDirNode);
+                                           &pDirNode);
                     }
                     else
                     {
@@ -1373,7 +1373,7 @@ AFSVerifyDirectoryContent( IN AFSObjectInfoCB *ObjectInfoCB,
                         //
 
                         AFSDeleteDirEntry( ObjectInfoCB,
-                                           pDirNode);
+                                           &pDirNode);
 
                         pCurrentDirEntry = (AFSDirEnumEntry *)((char *)pCurrentDirEntry + ulEntryLength);
 
@@ -1732,7 +1732,7 @@ AFSNotifyFileCreate( IN GUID            *AuthGroup,
                                       pResultCB->DirEnum.FileId.Unique));
 
                         AFSDeleteDirEntry( ParentObjectInfo,
-                                           pDirNode);
+                                           &pDirNode);
                     }
                     else
                     {
@@ -2341,7 +2341,7 @@ AFSNotifyHardLink( IN AFSObjectInfoCB *ObjectInfo,
                                       pResultCB->DirEnum.FileId.Unique));
 
                         AFSDeleteDirEntry( TargetParentObjectInfo,
-                                           pDirNode);
+                                           &pDirNode);
                     }
                     else
                     {

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp b/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp
index 39103e7..aae6d8c 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp
@@ -3316,7 +3316,7 @@ AFSSetRenameInfo( IN PIRP Irp)
                               &pTargetDirEntry->NameInformation.FileName));
 
                 AFSDeleteDirEntry( pTargetParentObject,
-                                   pTargetDirEntry);
+                                   &pTargetDirEntry);
             }
 
             pTargetDirEntry = NULL;

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp b/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp
index 3524907..2a86401 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp
@@ -3582,7 +3582,7 @@ AFSValidateDirectoryCache( IN AFSObjectInfoCB *ObjectInfo,
                                   &pCurrentDirEntry->NameInformation.FileName));
 
                     AFSDeleteDirEntry( ObjectInfo,
-                                       pCurrentDirEntry);
+                                       &pCurrentDirEntry);
                 }
                 else
                 {
@@ -3715,7 +3715,7 @@ AFSValidateDirectoryCache( IN AFSObjectInfoCB *ObjectInfo,
                               ObjectInfo->FileId.Unique));
 
                 AFSDeleteDirEntry( ObjectInfo,
-                                   pCurrentDirEntry);
+                                   &pCurrentDirEntry);
             }
             else
             {
@@ -4909,7 +4909,7 @@ AFSResetDirectoryContent( IN AFSObjectInfoCB *ObjectInfoCB)
                               &pCurrentDirEntry->NameInformation.FileName));
 
                 AFSDeleteDirEntry( ObjectInfoCB,
-                                   pCurrentDirEntry);
+                                   &pCurrentDirEntry);
             }
             else
             {

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSNameSupport.cpp b/src/WINNT/afsrdr/kernel/lib/AFSNameSupport.cpp
index aaae87d..64f2760 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSNameSupport.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSNameSupport.cpp
@@ -1986,7 +1986,7 @@ AFSLocateNameEntry( IN GUID *AuthGroup,
                     //
 
                     AFSDeleteDirEntry( pParentObjectInfo,
-                                       pDirEntry);
+                                       &pDirEntry);
 
                     AFSAcquireShared( &pCurrentObject->NonPagedInfo->ObjectInfoLock,
                                       TRUE);
@@ -2477,7 +2477,7 @@ AFSCreateDirEntry( IN GUID            *AuthGroup,
                                   lCount));
 
                     AFSDeleteDirEntry( ParentObjectInfo,
-                                       pDirNode);
+                                       &pDirNode);
 
                     lCount = InterlockedIncrement( &pExistingDirNode->DirOpenReferenceCount);
 
@@ -2521,7 +2521,7 @@ AFSCreateDirEntry( IN GUID            *AuthGroup,
                                   pDirNode->ObjectInformation->FileId.Unique));
 
                     AFSDeleteDirEntry( ParentObjectInfo,
-                                       pExistingDirNode);
+                                       &pExistingDirNode);
                 }
                 else
                 {
@@ -2755,72 +2755,83 @@ AFSInsertDirectoryNode( IN AFSObjectInfoCB *ParentObjectInfo,
 
 void
 AFSDeleteDirEntry( IN AFSObjectInfoCB *ParentObjectInfo,
-                   IN AFSDirectoryCB *DirEntry)
+                   IN AFSDirectoryCB **ppDirEntry)
 {
 
     LONG lCount;
+    AFSDirectoryCB *pDirEntry;
 
     __Enter
     {
 
+        pDirEntry = (AFSDirectoryCB *) InterlockedCompareExchangePointer( (PVOID *)ppDirEntry,
+                                                                          NULL,
+                                                                          *ppDirEntry);
+
+        if ( pDirEntry == NULL)
+        {
+
+            try_return( NOTHING);
+        }
+
         AFSDbgTrace(( AFS_SUBSYSTEM_CLEANUP_PROCESSING | AFS_SUBSYSTEM_DIRENTRY_REF_COUNTING,
                       AFS_TRACE_LEVEL_VERBOSE,
-                      "AFSDeleteDirEntry Deleting dir entry in parent %p Entry %p object %p %wZ RefCount %d\n",
+                      "AFSDeletepDirEntry Deleting dir entry in parent %p Entry %p object %p %wZ RefCount %d\n",
                       ParentObjectInfo,
-                      DirEntry,
-                      DirEntry->ObjectInformation,
-                      &DirEntry->NameInformation.FileName,
-                      DirEntry->DirOpenReferenceCount));
+                      pDirEntry,
+                      pDirEntry->ObjectInformation,
+                      &pDirEntry->NameInformation.FileName,
+                      pDirEntry->DirOpenReferenceCount));
 
-        ASSERT( DirEntry->DirOpenReferenceCount == 0);
+        ASSERT( pDirEntry->DirOpenReferenceCount == 0);
 
         AFSRemoveDirNodeFromParent( ParentObjectInfo,
-                                    DirEntry,
+                                    pDirEntry,
                                     TRUE);
 
         //
         // Free up the name buffer if it was reallocated
         //
 
-        if( BooleanFlagOn( DirEntry->Flags, AFS_DIR_RELEASE_NAME_BUFFER))
+        if( BooleanFlagOn( pDirEntry->Flags, AFS_DIR_RELEASE_NAME_BUFFER))
         {
 
-            AFSExFreePoolWithTag( DirEntry->NameInformation.FileName.Buffer, 0);
+            AFSExFreePoolWithTag( pDirEntry->NameInformation.FileName.Buffer, 0);
         }
 
-        if( BooleanFlagOn( DirEntry->Flags, AFS_DIR_RELEASE_TARGET_NAME_BUFFER))
+        if( BooleanFlagOn( pDirEntry->Flags, AFS_DIR_RELEASE_TARGET_NAME_BUFFER))
         {
 
-            AFSExFreePoolWithTag( DirEntry->NameInformation.TargetName.Buffer, 0);
+            AFSExFreePoolWithTag( pDirEntry->NameInformation.TargetName.Buffer, 0);
         }
 
-        if ( DirEntry->ObjectInformation != NULL)
+        if ( pDirEntry->ObjectInformation != NULL)
         {
 
-            if( BooleanFlagOn( DirEntry->Flags, AFS_DIR_ENTRY_DELETED) &&
-                DirEntry->ObjectInformation->Links == 0)
+            if( BooleanFlagOn( pDirEntry->Flags, AFS_DIR_ENTRY_DELETED) &&
+                pDirEntry->ObjectInformation->Links == 0)
             {
 
-                SetFlag( DirEntry->ObjectInformation->Flags, AFS_OBJECT_FLAGS_DELETED);
+                SetFlag( pDirEntry->ObjectInformation->Flags, AFS_OBJECT_FLAGS_DELETED);
             }
 
             //
             // Dereference the object for this dir entry
             //
 
-            lCount = AFSObjectInfoDecrement( DirEntry->ObjectInformation,
+            lCount = AFSObjectInfoDecrement( pDirEntry->ObjectInformation,
                                              AFS_OBJECT_REFERENCE_DIRENTRY);
 
             AFSDbgTrace(( AFS_SUBSYSTEM_OBJECT_REF_COUNTING,
                           AFS_TRACE_LEVEL_VERBOSE,
-                          "AFSDeleteDirEntry Decrement count on object %p Cnt %d\n",
-                          DirEntry->ObjectInformation,
+                          "AFSDeletepDirEntry Decrement count on object %p Cnt %d\n",
+                          pDirEntry->ObjectInformation,
                           lCount));
         }
 
-        ExDeleteResourceLite( &DirEntry->NonPaged->Lock);
+        ExDeleteResourceLite( &pDirEntry->NonPaged->Lock);
 
-        AFSExFreePoolWithTag( DirEntry->NonPaged, AFS_DIR_ENTRY_NP_TAG);
+        AFSExFreePoolWithTag( pDirEntry->NonPaged, AFS_DIR_ENTRY_NP_TAG);
 
         //
         // Free up the dir entry
@@ -2828,10 +2839,14 @@ AFSDeleteDirEntry( IN AFSObjectInfoCB *ParentObjectInfo,
 
         AFSDbgTrace(( AFS_SUBSYSTEM_DIRENTRY_ALLOCATION,
                       AFS_TRACE_LEVEL_VERBOSE,
-                      "AFSDeleteDirEntry AFS_DIR_ENTRY_TAG deallocating %p\n",
-                      DirEntry));
+                      "AFSDeletepDirEntry AFS_DIR_ENTRY_TAG deallocating %p\n",
+                      pDirEntry));
+
+        AFSExFreePoolWithTag( pDirEntry, AFS_DIR_ENTRY_TAG);
 
-        AFSExFreePoolWithTag( DirEntry, AFS_DIR_ENTRY_TAG);
+try_exit:
+
+        NOTHING;
     }
 }
 
@@ -4325,7 +4340,7 @@ AFSCheckCellName( IN GUID *AuthGroup,
                 {
 
                     AFSDeleteDirEntry( &AFSGlobalRoot->ObjectInformation,
-                                       pDirNode);
+                                       &pDirNode);
 
                     AFSReleaseResource( AFSGlobalRoot->ObjectInformation.Specific.Directory.DirectoryNodeHdr.TreeLock);
 

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp b/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp
index 2310268..490438d 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp
@@ -1322,7 +1322,7 @@ AFSExamineObjectInfo( IN AFSObjectInfoCB * pCurrentObject,
                                       pCurrentDirEntry->ObjectInformation));
 
                         AFSDeleteDirEntry( pCurrentObject,
-                                           pCurrentDirEntry);
+                                           &pCurrentDirEntry);
 
                         pCurrentDirEntry = pNextDirEntry;
                     }

diff --git a/src/WINNT/afsrdr/kernel/lib/Include/AFSCommon.h b/src/WINNT/afsrdr/kernel/lib/Include/AFSCommon.h
index 8e6b8ab..50aee25 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/Include/AFSCommon.h
+++ b/src/WINNT/afsrdr/kernel/lib/Include/AFSCommon.h
@@ -591,7 +591,7 @@ AFSInsertDirectoryNode( IN AFSObjectInfoCB *ParentObjectInfo,
 
 void
 AFSDeleteDirEntry( IN AFSObjectInfoCB *ParentObjectInfo,
-                   IN AFSDirectoryCB *DirEntry);
+                   IN AFSDirectoryCB **ppDirEntry);
 
 NTSTATUS
 AFSRemoveDirNodeFromParent( IN AFSObjectInfoCB *ParentObjectInfo,