From ec5fd2ce07d2e8adfa5c34d978bc786f36dac044 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Thu, 14 Mar 2013 12:30:36 -0400 Subject: [PATCH] Windows: Protect against DirEntry with NULL ObjInfo During cleanup protect AFSExamineObjectInfo() and AFSExamineDirectory() from DirectoryCB objects that have a NULL ObjectInformation pointer. Change-Id: Id46f6b53ec4861f5ac2d28b918d073201d2433ce Reviewed-on: http://gerrit.openafs.org/9603 Reviewed-by: Peter Scott Tested-by: BuildBot Reviewed-by: Jeffrey Altman Tested-by: Jeffrey Altman --- src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp | 262 +++++++++++----------- 1 file changed, 137 insertions(+), 125 deletions(-) diff --git a/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp b/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp index 8f7d0d8b3..3e8f61808 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSWorker.cpp @@ -961,112 +961,116 @@ AFSExamineDirectory( IN AFSObjectInfoCB * pCurrentObject, AFSDeleteDirEntry( pCurrentObject, pCurrentDirEntry); - // - // Acquire ObjectInfoLock shared here so as not to deadlock - // with an invalidation call from the service during AFSCleanupFcb - // - - lCount = AFSObjectInfoIncrement( pCurrentChildObject, - AFS_OBJECT_REFERENCE_WORKER); - - AFSDbgLogMsg( AFS_SUBSYSTEM_OBJECT_REF_COUNTING, - AFS_TRACE_LEVEL_VERBOSE, - "AFSExamineDirectory Increment count on object %p Cnt %d\n", - pCurrentChildObject, - lCount); - - if( lCount == 1 && - pCurrentChildObject->Fcb != NULL && - pCurrentChildObject->FileType == AFS_FILE_TYPE_FILE) + if ( pCurrentChildObject != NULL) { // - // We must not hold pVolumeCB->ObjectInfoTree.TreeLock exclusive - // across an AFSCleanupFcb call since it can deadlock with an - // invalidation call from the service. + // Acquire ObjectInfoLock shared here so as not to deadlock + // with an invalidation call from the service during AFSCleanupFcb // - AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock); - - // - // Cannot hold a TreeLock across an AFSCleanupFcb call - // as it can deadlock with an invalidation ioctl initiated - // from the service. - // - // Dropping the TreeLock permits the - // pCurrentObject->ObjectReferenceCount to change - // + lCount = AFSObjectInfoIncrement( pCurrentChildObject, + AFS_OBJECT_REFERENCE_WORKER); - ntStatus = AFSCleanupFcb( pCurrentChildObject->Fcb, - TRUE); + AFSDbgLogMsg( AFS_SUBSYSTEM_OBJECT_REF_COUNTING, + AFS_TRACE_LEVEL_VERBOSE, + "AFSExamineDirectory Increment count on object %p Cnt %d\n", + pCurrentChildObject, + lCount); - if ( ntStatus == STATUS_RETRY) + if( lCount == 1 && + pCurrentChildObject->Fcb != NULL && + pCurrentChildObject->FileType == AFS_FILE_TYPE_FILE) { - bFcbBusy = TRUE; - } + // + // We must not hold pVolumeCB->ObjectInfoTree.TreeLock exclusive + // across an AFSCleanupFcb call since it can deadlock with an + // invalidation call from the service. + // - AFSAcquireExcl( pVolumeCB->ObjectInfoTree.TreeLock, - TRUE); - } + AFSReleaseResource( pVolumeCB->ObjectInfoTree.TreeLock); - AFSAcquireExcl( &pCurrentChildObject->NonPagedInfo->ObjectInfoLock, - TRUE); + // + // Cannot hold a TreeLock across an AFSCleanupFcb call + // as it can deadlock with an invalidation ioctl initiated + // from the service. + // + // Dropping the TreeLock permits the + // pCurrentObject->ObjectReferenceCount to change + // - lCount = AFSObjectInfoDecrement( pCurrentChildObject, - AFS_OBJECT_REFERENCE_WORKER); + ntStatus = AFSCleanupFcb( pCurrentChildObject->Fcb, + TRUE); - AFSDbgLogMsg( AFS_SUBSYSTEM_OBJECT_REF_COUNTING, - AFS_TRACE_LEVEL_VERBOSE, - "AFSExamineDirectory Decrement1 count on object %p Cnt %d\n", - pCurrentChildObject, - lCount); + if ( ntStatus == STATUS_RETRY) + { - if( lCount == 0 && - pCurrentChildObject->Fcb != NULL && - pCurrentChildObject->Fcb->OpenReferenceCount == 0) - { + bFcbBusy = TRUE; + } + + AFSAcquireExcl( pVolumeCB->ObjectInfoTree.TreeLock, + TRUE); + } - AFSRemoveFcb( &pCurrentChildObject->Fcb); + AFSAcquireExcl( &pCurrentChildObject->NonPagedInfo->ObjectInfoLock, + TRUE); + + lCount = AFSObjectInfoDecrement( pCurrentChildObject, + AFS_OBJECT_REFERENCE_WORKER); + + AFSDbgLogMsg( AFS_SUBSYSTEM_OBJECT_REF_COUNTING, + AFS_TRACE_LEVEL_VERBOSE, + "AFSExamineDirectory Decrement1 count on object %p Cnt %d\n", + pCurrentChildObject, + lCount); - if( pCurrentChildObject->FileType == AFS_FILE_TYPE_DIRECTORY && - pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB != NULL) + if( lCount == 0 && + pCurrentChildObject->Fcb != NULL && + pCurrentChildObject->Fcb->OpenReferenceCount == 0) { - AFSAcquireExcl( &pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->NonPagedInfo->ObjectInfoLock, - TRUE); + AFSRemoveFcb( &pCurrentChildObject->Fcb); + + if( pCurrentChildObject->FileType == AFS_FILE_TYPE_DIRECTORY && + pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB != NULL) + { - AFSRemoveFcb( &pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->Fcb); + AFSAcquireExcl( &pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->NonPagedInfo->ObjectInfoLock, + TRUE); - AFSReleaseResource( &pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->NonPagedInfo->ObjectInfoLock); + AFSRemoveFcb( &pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->Fcb); - AFSDeleteObjectInfo( &pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation); + AFSReleaseResource( &pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation->NonPagedInfo->ObjectInfoLock); - ExDeleteResourceLite( &pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->NonPaged->Lock); + AFSDeleteObjectInfo( &pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->ObjectInformation); - AFSExFreePoolWithTag( pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->NonPaged, AFS_DIR_ENTRY_NP_TAG); + ExDeleteResourceLite( &pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->NonPaged->Lock); - AFSDbgLogMsg( AFS_SUBSYSTEM_DIRENTRY_ALLOCATION, - AFS_TRACE_LEVEL_VERBOSE, - "AFSExamineDirectory (pioctl) AFS_DIR_ENTRY_TAG deallocating %p\n", - pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB); + AFSExFreePoolWithTag( pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB->NonPaged, AFS_DIR_ENTRY_NP_TAG); - AFSExFreePoolWithTag( pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB, AFS_DIR_ENTRY_TAG); - } + AFSDbgLogMsg( AFS_SUBSYSTEM_DIRENTRY_ALLOCATION, + AFS_TRACE_LEVEL_VERBOSE, + "AFSExamineDirectory (pioctl) AFS_DIR_ENTRY_TAG deallocating %p\n", + pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB); - AFSReleaseResource( &pCurrentChildObject->NonPagedInfo->ObjectInfoLock); + AFSExFreePoolWithTag( pCurrentChildObject->Specific.Directory.PIOCtlDirectoryCB, AFS_DIR_ENTRY_TAG); + } - AFSDbgLogMsg( AFS_SUBSYSTEM_CLEANUP_PROCESSING, - AFS_TRACE_LEVEL_VERBOSE, - "AFSExamineDirectory Deleting object %p\n", - pCurrentChildObject); + AFSReleaseResource( &pCurrentChildObject->NonPagedInfo->ObjectInfoLock); - AFSDeleteObjectInfo( &pCurrentChildObject); - } - else - { + AFSDbgLogMsg( AFS_SUBSYSTEM_CLEANUP_PROCESSING, + AFS_TRACE_LEVEL_VERBOSE, + "AFSExamineDirectory Deleting object %p\n", + pCurrentChildObject); + + AFSDeleteObjectInfo( &pCurrentChildObject); + } + else + { - AFSReleaseResource( &pCurrentChildObject->NonPagedInfo->ObjectInfoLock); + AFSReleaseResource( &pCurrentChildObject->NonPagedInfo->ObjectInfoLock); + } } return bFcbBusy; @@ -1252,45 +1256,49 @@ AFSExamineObjectInfo( IN AFSObjectInfoCB * pCurrentObject, break; } - if ( pCurrentDirEntry->ObjectInformation->Fcb != NULL && - pCurrentDirEntry->ObjectInformation->Fcb->OpenReferenceCount > 0) - { - - break; - } - - if ( liCurrentTime.QuadPart <= pCurrentDirEntry->ObjectInformation->LastAccessCount.QuadPart || - liCurrentTime.QuadPart - pCurrentDirEntry->ObjectInformation->LastAccessCount.QuadPart < - pControlDeviceExt->Specific.Control.ObjectLifeTimeCount.QuadPart) - { - - break; - } - - if ( pCurrentDirEntry->ObjectInformation->FileType == AFS_FILE_TYPE_DIRECTORY) + if ( pCurrentDirEntry->ObjectInformation != NULL) { - if ( pCurrentDirEntry->ObjectInformation->Specific.Directory.DirectoryNodeListHead != NULL) + if ( pCurrentDirEntry->ObjectInformation->Fcb != NULL && + pCurrentDirEntry->ObjectInformation->Fcb->OpenReferenceCount > 0) { break; } - if ( pCurrentDirEntry->ObjectInformation->Specific.Directory.ChildOpenReferenceCount > 0) + if ( liCurrentTime.QuadPart <= pCurrentDirEntry->ObjectInformation->LastAccessCount.QuadPart || + liCurrentTime.QuadPart - pCurrentDirEntry->ObjectInformation->LastAccessCount.QuadPart < + pControlDeviceExt->Specific.Control.ObjectLifeTimeCount.QuadPart) { break; } - } - if ( pCurrentDirEntry->ObjectInformation->FileType == AFS_FILE_TYPE_FILE) - { + if ( pCurrentDirEntry->ObjectInformation->FileType == AFS_FILE_TYPE_DIRECTORY) + { - if ( pCurrentDirEntry->ObjectInformation->Fcb != NULL && - pCurrentDirEntry->ObjectInformation->Fcb->Specific.File.ExtentsDirtyCount > 0) + if ( pCurrentDirEntry->ObjectInformation->Specific.Directory.DirectoryNodeListHead != NULL) + { + + break; + } + + if ( pCurrentDirEntry->ObjectInformation->Specific.Directory.ChildOpenReferenceCount > 0) + { + + break; + } + } + + if ( pCurrentDirEntry->ObjectInformation->FileType == AFS_FILE_TYPE_FILE) { - break; + if ( pCurrentDirEntry->ObjectInformation->Fcb != NULL && + pCurrentDirEntry->ObjectInformation->Fcb->Specific.File.ExtentsDirtyCount > 0) + { + + break; + } } } @@ -1357,45 +1365,49 @@ AFSExamineObjectInfo( IN AFSObjectInfoCB * pCurrentObject, break; } - if ( pCurrentDirEntry->ObjectInformation->Fcb != NULL && - pCurrentDirEntry->ObjectInformation->Fcb->OpenReferenceCount > 0) - { - - break; - } - - if ( liCurrentTime.QuadPart <= pCurrentDirEntry->ObjectInformation->LastAccessCount.QuadPart || - liCurrentTime.QuadPart - pCurrentDirEntry->ObjectInformation->LastAccessCount.QuadPart < - pControlDeviceExt->Specific.Control.ObjectLifeTimeCount.QuadPart) - { - - break; - } - - if ( pCurrentDirEntry->ObjectInformation->FileType == AFS_FILE_TYPE_DIRECTORY) + if ( pCurrentDirEntry->ObjectInformation != NULL) { - if ( pCurrentDirEntry->ObjectInformation->Specific.Directory.DirectoryNodeListHead != NULL) + if ( pCurrentDirEntry->ObjectInformation->Fcb != NULL && + pCurrentDirEntry->ObjectInformation->Fcb->OpenReferenceCount > 0) { break; } - if ( pCurrentDirEntry->ObjectInformation->Specific.Directory.ChildOpenReferenceCount > 0) + if ( liCurrentTime.QuadPart <= pCurrentDirEntry->ObjectInformation->LastAccessCount.QuadPart || + liCurrentTime.QuadPart - pCurrentDirEntry->ObjectInformation->LastAccessCount.QuadPart < + pControlDeviceExt->Specific.Control.ObjectLifeTimeCount.QuadPart) { break; } - } - if ( pCurrentDirEntry->ObjectInformation->FileType == AFS_FILE_TYPE_FILE) - { + if ( pCurrentDirEntry->ObjectInformation->FileType == AFS_FILE_TYPE_DIRECTORY) + { - if ( pCurrentDirEntry->ObjectInformation->Fcb != NULL && - pCurrentDirEntry->ObjectInformation->Fcb->Specific.File.ExtentsDirtyCount > 0) + if ( pCurrentDirEntry->ObjectInformation->Specific.Directory.DirectoryNodeListHead != NULL) + { + + break; + } + + if ( pCurrentDirEntry->ObjectInformation->Specific.Directory.ChildOpenReferenceCount > 0) + { + + break; + } + } + + if ( pCurrentDirEntry->ObjectInformation->FileType == AFS_FILE_TYPE_FILE) { - break; + if ( pCurrentDirEntry->ObjectInformation->Fcb != NULL && + pCurrentDirEntry->ObjectInformation->Fcb->Specific.File.ExtentsDirtyCount > 0) + { + + break; + } } } -- 2.39.5