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 1dd061ffc6e9657df38d5dcf48e9df2fa91285e6..cddd8fb0411942d86c96ff34bca8a5d4365cd5bf 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 85285dcbf3d56caf731f308cdf689ff9e61b3d27..f9edbbfcaa4bb022eb377eb34ff2c928ceec305a 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 39103e7c73def744c01395f6880c6f62d61b1521..aae6d8c9c479ae78c15f5d832fe1ebc137c2fd18 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 3524907aa0990cfabd81c7b0e511771f1d78822f..2a86401ed684eb4d71eb065775426bbd4bfe5243 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 aaae87dc8a559eb99d7657a1e4e5e49cc6f5db29..64f27607c60d84eea28f79ebd58b28b6e59d0c7f 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 2310268e549afdb7b691527b96fd5e717e148fe6..490438d0d75f04f2198dfcfaa51eb6ee282f736c 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 8e6b8ab95bea6e9635e6a60bc087cb23c971c772..50aee252de5492d369b4a9920403d49c461d6ab9 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,