From c3ed4794c1ed7db14b67eb0c3bb65c1aed18ff35 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Thu, 22 Nov 2012 11:52:11 -0500 Subject: [PATCH] Windows: Trend Micro QueryDirectory deadlock Trend Micro will deadlock both itself and the AFS redirector by calling a worker thread to generate a temporary file name during an active FindFirst Directory Query. The Trend Micro worker will also attempt to enumerate the directory. If the directory contains an entry for which the data version as reported by the service is different than the data version in the ObjectInformationCB a deadlock will occur on the matching FileCB Resource. To avoid this deadlock, prevent AFSValidateEntry from purging or updating the ObjectInformationCB and FileObject information when called from AFSQueryDirectory. Change-Id: I8f2f7136796759eb91dadfea34a89513c1a1fff4 Reviewed-on: http://gerrit.openafs.org/8492 Reviewed-by: Peter Scott Reviewed-by: Jeffrey Altman Tested-by: Jeffrey Altman --- src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp | 9 +- src/WINNT/afsrdr/kernel/lib/AFSDirControl.cpp | 1 + src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp | 11 + src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp | 262 ++++++++++-------- .../afsrdr/kernel/lib/Include/AFSCommon.h | 3 +- 5 files changed, 171 insertions(+), 115 deletions(-) diff --git a/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp b/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp index ef932c00a..2a42f1862 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp @@ -1393,7 +1393,8 @@ AFSOpenRoot( IN PIRP Irp, ntStatus = AFSValidateEntry( VolumeCB->DirectoryCB, AuthGroup, - FALSE); + FALSE, + TRUE); if( !NT_SUCCESS( ntStatus)) { @@ -2526,7 +2527,8 @@ AFSProcessOpen( IN PIRP Irp, ntStatus = AFSValidateEntry( DirectoryCB, AuthGroup, - FALSE); + FALSE, + TRUE); if( !NT_SUCCESS( ntStatus)) { @@ -3025,7 +3027,8 @@ AFSProcessOverwriteSupersede( IN PDEVICE_OBJECT DeviceObject, ntStatus = AFSValidateEntry( DirectoryCB, AuthGroup, - FALSE); + FALSE, + TRUE); if( !NT_SUCCESS( ntStatus)) { diff --git a/src/WINNT/afsrdr/kernel/lib/AFSDirControl.cpp b/src/WINNT/afsrdr/kernel/lib/AFSDirControl.cpp index 9a1946883..88b25f442 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSDirControl.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSDirControl.cpp @@ -744,6 +744,7 @@ AFSQueryDirectory( IN PIRP Irp) AFSValidateEntry( pDirEntry, &pCcb->AuthGroup, + FALSE, FALSE); pObjectInfo = pDirEntry->ObjectInformation; diff --git a/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp b/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp index 5071b07cd..526144e00 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp @@ -65,6 +65,7 @@ AFSQueryFileInfo( IN PDEVICE_OBJECT LibDeviceObject, BOOLEAN bReleaseMain = FALSE; LONG lLength; FILE_INFORMATION_CLASS stFileInformationClass; + GUID stAuthGroup; PVOID pBuffer; __try @@ -93,6 +94,16 @@ AFSQueryFileInfo( IN PDEVICE_OBJECT LibDeviceObject, stFileInformationClass = pIrpSp->Parameters.QueryFile.FileInformationClass; pBuffer = Irp->AssociatedIrp.SystemBuffer; + RtlZeroMemory( &stAuthGroup, + sizeof( GUID)); + + AFSRetrieveAuthGroupFnc( (ULONGLONG)PsGetCurrentProcessId(), + (ULONGLONG)PsGetCurrentThreadId(), + &stAuthGroup); + + AFSVerifyEntry( &stAuthGroup, + pCcb->DirectoryCB); + // // Grab the main shared right off the bat // diff --git a/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp b/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp index 9272697ff..8eb6604a0 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp @@ -1753,21 +1753,26 @@ AFSInvalidateObject( IN OUT AFSObjectInfoCB **ppObjectInfo, ntStatus = stIoStatus.Status; } - if ( !CcPurgeCacheSection( &(*ppObjectInfo)->Fcb->NPFcb->SectionObjectPointers, - NULL, - 0, - FALSE)) + + if ( (*ppObjectInfo)->Fcb->NPFcb->SectionObjectPointers.DataSectionObject != NULL) { - AFSDbgLogMsg( AFS_SUBSYSTEM_IO_PROCESSING, - AFS_TRACE_LEVEL_WARNING, - "AFSInvalidateObject CcPurgeCacheSection failure FID %08lX-%08lX-%08lX-%08lX\n", - (*ppObjectInfo)->FileId.Cell, - (*ppObjectInfo)->FileId.Volume, - (*ppObjectInfo)->FileId.Vnode, - (*ppObjectInfo)->FileId.Unique); + if ( !CcPurgeCacheSection( &(*ppObjectInfo)->Fcb->NPFcb->SectionObjectPointers, + NULL, + 0, + FALSE)) + { - SetFlag( (*ppObjectInfo)->Fcb->Flags, AFS_FCB_FLAG_PURGE_ON_CLOSE); + AFSDbgLogMsg( AFS_SUBSYSTEM_IO_PROCESSING, + AFS_TRACE_LEVEL_WARNING, + "AFSInvalidateObject CcPurgeCacheSection failure FID %08lX-%08lX-%08lX-%08lX\n", + (*ppObjectInfo)->FileId.Cell, + (*ppObjectInfo)->FileId.Volume, + (*ppObjectInfo)->FileId.Vnode, + (*ppObjectInfo)->FileId.Unique); + + SetFlag( (*ppObjectInfo)->Fcb->Flags, AFS_FCB_FLAG_PURGE_ON_CLOSE); + } } } __except( EXCEPTION_EXECUTE_HANDLER) @@ -2881,7 +2886,8 @@ AFSVerifyEntry( IN GUID *AuthGroup, ntStatus = stIoStatus.Status; } - if ( bPurgeExtents) + if ( bPurgeExtents && + pObjectInfo->Fcb->NPFcb->SectionObjectPointers.DataSectionObject != NULL) { if ( !CcPurgeCacheSection( &pObjectInfo->Fcb->NPFcb->SectionObjectPointers, @@ -3788,7 +3794,8 @@ try_exit: NTSTATUS AFSValidateEntry( IN AFSDirectoryCB *DirEntry, IN GUID *AuthGroup, - IN BOOLEAN FastCall) + IN BOOLEAN FastCall, + IN BOOLEAN bSafeToPurge) { NTSTATUS ntStatus = STATUS_SUCCESS; @@ -3946,6 +3953,8 @@ AFSValidateEntry( IN AFSDirectoryCB *DirEntry, case AFS_FILE_TYPE_FILE: { + BOOLEAN bPurgeExtents = FALSE; + // // For a file where the data version has become invalid we need to // fail any current extent requests and purge the cache for the file @@ -3978,7 +3987,6 @@ AFSValidateEntry( IN AFSDirectoryCB *DirEntry, { IO_STATUS_BLOCK stIoStatus; - BOOLEAN bPurgeExtents = FALSE; AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING, AFS_TRACE_LEVEL_VERBOSE_2, @@ -4007,91 +4015,107 @@ AFSValidateEntry( IN AFSDirectoryCB *DirEntry, bPurgeExtents = TRUE; } - if ( BooleanFlagOn( pObjectInfo->Flags, AFS_OBJECT_FLAGS_VERIFY_DATA)) - { - bPurgeExtents = TRUE; - - AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING, - AFS_TRACE_LEVEL_VERBOSE, - "AFSVerifyEntry Clearing VERIFY_DATA flag %wZ FID %08lX-%08lX-%08lX-%08lX\n", - &DirEntry->NameInformation.FileName, - pObjectInfo->FileId.Cell, - pObjectInfo->FileId.Volume, - pObjectInfo->FileId.Vnode, - pObjectInfo->FileId.Unique); - - ClearFlag( pObjectInfo->Flags, AFS_OBJECT_FLAGS_VERIFY_DATA); - } - - __try + if ( bSafeToPurge) { - CcFlushCache( &pCurrentFcb->NPFcb->SectionObjectPointers, - NULL, - 0, - &stIoStatus); - - if( !NT_SUCCESS( stIoStatus.Status)) + if ( BooleanFlagOn( pObjectInfo->Flags, AFS_OBJECT_FLAGS_VERIFY_DATA)) { + bPurgeExtents = TRUE; - AFSDbgLogMsg( AFS_SUBSYSTEM_IO_PROCESSING, - AFS_TRACE_LEVEL_ERROR, - "AFSValidateEntry CcFlushCache failure %wZ FID %08lX-%08lX-%08lX-%08lX Status 0x%08lX Bytes 0x%08lX\n", + AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING, + AFS_TRACE_LEVEL_VERBOSE, + "AFSVerifyEntry Clearing VERIFY_DATA flag %wZ FID %08lX-%08lX-%08lX-%08lX\n", &DirEntry->NameInformation.FileName, pObjectInfo->FileId.Cell, pObjectInfo->FileId.Volume, pObjectInfo->FileId.Vnode, - pObjectInfo->FileId.Unique, - stIoStatus.Status, - stIoStatus.Information); + pObjectInfo->FileId.Unique); - ntStatus = stIoStatus.Status; + ClearFlag( pObjectInfo->Flags, AFS_OBJECT_FLAGS_VERIFY_DATA); } - if ( bPurgeExtents) + __try { - if ( !CcPurgeCacheSection( &pObjectInfo->Fcb->NPFcb->SectionObjectPointers, - NULL, - 0, - FALSE)) + CcFlushCache( &pCurrentFcb->NPFcb->SectionObjectPointers, + NULL, + 0, + &stIoStatus); + + if( !NT_SUCCESS( stIoStatus.Status)) { AFSDbgLogMsg( AFS_SUBSYSTEM_IO_PROCESSING, - AFS_TRACE_LEVEL_WARNING, - "AFSValidateEntry CcPurgeCacheSection failure %wZ FID %08lX-%08lX-%08lX-%08lX\n", + AFS_TRACE_LEVEL_ERROR, + "AFSValidateEntry CcFlushCache failure %wZ FID %08lX-%08lX-%08lX-%08lX Status 0x%08lX Bytes 0x%08lX\n", &DirEntry->NameInformation.FileName, pObjectInfo->FileId.Cell, pObjectInfo->FileId.Volume, pObjectInfo->FileId.Vnode, - pObjectInfo->FileId.Unique); + pObjectInfo->FileId.Unique, + stIoStatus.Status, + stIoStatus.Information); - SetFlag( pObjectInfo->Fcb->Flags, AFS_FCB_FLAG_PURGE_ON_CLOSE); + ntStatus = stIoStatus.Status; } + + if ( bPurgeExtents && + pObjectInfo->Fcb->NPFcb->SectionObjectPointers.DataSectionObject != NULL) + { + + if ( !CcPurgeCacheSection( &pObjectInfo->Fcb->NPFcb->SectionObjectPointers, + NULL, + 0, + FALSE)) + { + + AFSDbgLogMsg( AFS_SUBSYSTEM_IO_PROCESSING, + AFS_TRACE_LEVEL_WARNING, + "AFSValidateEntry CcPurgeCacheSection failure %wZ FID %08lX-%08lX-%08lX-%08lX\n", + &DirEntry->NameInformation.FileName, + pObjectInfo->FileId.Cell, + pObjectInfo->FileId.Volume, + pObjectInfo->FileId.Vnode, + pObjectInfo->FileId.Unique); + + SetFlag( pObjectInfo->Fcb->Flags, AFS_FCB_FLAG_PURGE_ON_CLOSE); + } + } + } + __except( EXCEPTION_EXECUTE_HANDLER) + { + ntStatus = GetExceptionCode(); + + AFSDbgLogMsg( 0, + 0, + "EXCEPTION - AFSValidateEntry CcFlushCache or CcPurgeCacheSection %wZ FID %08lX-%08lX-%08lX-%08lX Status 0x%08lX\n", + &DirEntry->NameInformation.FileName, + pObjectInfo->FileId.Cell, + pObjectInfo->FileId.Volume, + pObjectInfo->FileId.Vnode, + pObjectInfo->FileId.Unique, + ntStatus); + + SetFlag( pObjectInfo->Fcb->Flags, AFS_FCB_FLAG_PURGE_ON_CLOSE); } } - __except( EXCEPTION_EXECUTE_HANDLER) + else { - ntStatus = GetExceptionCode(); - AFSDbgLogMsg( 0, - 0, - "EXCEPTION - AFSValidateEntry CcFlushCache or CcPurgeCacheSection %wZ FID %08lX-%08lX-%08lX-%08lX Status 0x%08lX\n", - &DirEntry->NameInformation.FileName, - pObjectInfo->FileId.Cell, - pObjectInfo->FileId.Volume, - pObjectInfo->FileId.Vnode, - pObjectInfo->FileId.Unique, - ntStatus); + if ( bPurgeExtents) + { - SetFlag( pObjectInfo->Fcb->Flags, AFS_FCB_FLAG_PURGE_ON_CLOSE); + SetFlag( pObjectInfo->Flags, AFS_OBJECT_FLAGS_VERIFY_DATA); + } } + AFSReleaseResource( &pCurrentFcb->NPFcb->Resource); bReleaseFcb = FALSE; - if ( bPurgeExtents) + if ( bPurgeExtents && + bSafeToPurge) { AFSFlushExtents( pCurrentFcb, AuthGroup); @@ -4100,46 +4124,55 @@ AFSValidateEntry( IN AFSDirectoryCB *DirEntry, } // - // Update the metadata for the entry + // Update the metadata for the entry but only if it is safe to do so. + // If it was determined that a data version change has occurred or + // that a pending data verification was required, do not update the + // ObjectInfo meta data or the FileObject size information. That + // way it is consistent for the next time that the data is verified + // or validated. // - ntStatus = AFSUpdateMetaData( DirEntry, - pDirEnumEntry); - - if( !NT_SUCCESS( ntStatus)) + if ( !(bPurgeExtents && bSafeToPurge)) { - AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING, - AFS_TRACE_LEVEL_ERROR, - "AFSValidateEntry Meta Data Update failed %wZ FID %08lX-%08lX-%08lX-%08lX Status 0x%08lX\n", - &DirEntry->NameInformation.FileName, - pObjectInfo->FileId.Cell, - pObjectInfo->FileId.Volume, - pObjectInfo->FileId.Vnode, - pObjectInfo->FileId.Unique, - ntStatus); + ntStatus = AFSUpdateMetaData( DirEntry, + pDirEnumEntry); - break; - } + if( !NT_SUCCESS( ntStatus)) + { - ClearFlag( pObjectInfo->Flags, AFS_OBJECT_FLAGS_VERIFY | AFS_OBJECT_FLAGS_NOT_EVALUATED); + AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING, + AFS_TRACE_LEVEL_ERROR, + "AFSValidateEntry Meta Data Update failed %wZ FID %08lX-%08lX-%08lX-%08lX Status 0x%08lX\n", + &DirEntry->NameInformation.FileName, + pObjectInfo->FileId.Cell, + pObjectInfo->FileId.Volume, + pObjectInfo->FileId.Vnode, + pObjectInfo->FileId.Unique, + ntStatus); - // - // Update file sizes - // + break; + } - if( pObjectInfo->Fcb != NULL) - { - FILE_OBJECT *pCCFileObject = CcGetFileObjectFromSectionPtrs( &pObjectInfo->Fcb->NPFcb->SectionObjectPointers); + ClearFlag( pObjectInfo->Flags, AFS_OBJECT_FLAGS_VERIFY | AFS_OBJECT_FLAGS_NOT_EVALUATED); - pObjectInfo->Fcb->Header.AllocationSize.QuadPart = pObjectInfo->AllocationSize.QuadPart; - pObjectInfo->Fcb->Header.FileSize.QuadPart = pObjectInfo->EndOfFile.QuadPart; - pObjectInfo->Fcb->Header.ValidDataLength.QuadPart = pObjectInfo->EndOfFile.QuadPart; + // + // Update file sizes + // - if ( pCCFileObject != NULL) + if( pObjectInfo->Fcb != NULL) { - CcSetFileSizes( pCCFileObject, - (PCC_FILE_SIZES)&pObjectInfo->Fcb->Header.AllocationSize); + FILE_OBJECT *pCCFileObject = CcGetFileObjectFromSectionPtrs( &pObjectInfo->Fcb->NPFcb->SectionObjectPointers); + + pObjectInfo->Fcb->Header.AllocationSize.QuadPart = pObjectInfo->AllocationSize.QuadPart; + pObjectInfo->Fcb->Header.FileSize.QuadPart = pObjectInfo->EndOfFile.QuadPart; + pObjectInfo->Fcb->Header.ValidDataLength.QuadPart = pObjectInfo->EndOfFile.QuadPart; + + if ( pCCFileObject != NULL) + { + CcSetFileSizes( pCCFileObject, + (PCC_FILE_SIZES)&pObjectInfo->Fcb->Header.AllocationSize); + } } } break; @@ -7010,21 +7043,25 @@ AFSCleanupFcb( IN AFSFcb *Fcb, ntStatus = stIoStatus.Status; } - if ( !CcPurgeCacheSection( &Fcb->NPFcb->SectionObjectPointers, - NULL, - 0, - FALSE)) + if ( Fcb->NPFcb->SectionObjectPointers.DataSectionObject != NULL) { - AFSDbgLogMsg( AFS_SUBSYSTEM_IO_PROCESSING, - AFS_TRACE_LEVEL_WARNING, - "AFSCleanupFcb CcPurgeCacheSection [1] failure FID %08lX-%08lX-%08lX-%08lX\n", - Fcb->ObjectInformation->FileId.Cell, - Fcb->ObjectInformation->FileId.Volume, - Fcb->ObjectInformation->FileId.Vnode, - Fcb->ObjectInformation->FileId.Unique); + if ( !CcPurgeCacheSection( &Fcb->NPFcb->SectionObjectPointers, + NULL, + 0, + FALSE)) + { - SetFlag( Fcb->Flags, AFS_FCB_FLAG_PURGE_ON_CLOSE); + AFSDbgLogMsg( AFS_SUBSYSTEM_IO_PROCESSING, + AFS_TRACE_LEVEL_WARNING, + "AFSCleanupFcb CcPurgeCacheSection [1] failure FID %08lX-%08lX-%08lX-%08lX\n", + Fcb->ObjectInformation->FileId.Cell, + Fcb->ObjectInformation->FileId.Volume, + Fcb->ObjectInformation->FileId.Vnode, + Fcb->ObjectInformation->FileId.Unique); + + SetFlag( Fcb->Flags, AFS_FCB_FLAG_PURGE_ON_CLOSE); + } } } __except( EXCEPTION_EXECUTE_HANDLER) @@ -7152,7 +7189,8 @@ AFSCleanupFcb( IN AFSFcb *Fcb, ntStatus = stIoStatus.Status; } - if( ForceFlush) + if( ForceFlush && + Fcb->NPFcb->SectionObjectPointers.DataSectionObject != NULL) { if ( !CcPurgeCacheSection( &Fcb->NPFcb->SectionObjectPointers, @@ -9063,7 +9101,8 @@ AFSPerformObjectInvalidate( IN AFSObjectInfoCB *ObjectInfo, bExtentsLocked = FALSE; - if( !CcPurgeCacheSection( &ObjectInfo->Fcb->NPFcb->SectionObjectPointers, + if( ObjectInfo->Fcb->NPFcb->SectionObjectPointers.DataSectionObject != NULL && + !CcPurgeCacheSection( &ObjectInfo->Fcb->NPFcb->SectionObjectPointers, NULL, 0, FALSE)) @@ -9143,7 +9182,8 @@ AFSPerformObjectInvalidate( IN AFSObjectInfoCB *ObjectInfo, ulSize = ByteRangeList[ulIndex].Length.QuadPart > DWORD_MAX ? DWORD_MAX : ByteRangeList[ulIndex].Length.LowPart; - if( !CcPurgeCacheSection( &ObjectInfo->Fcb->NPFcb->SectionObjectPointers, + if( ObjectInfo->Fcb->NPFcb->SectionObjectPointers.DataSectionObject != NULL && + !CcPurgeCacheSection( &ObjectInfo->Fcb->NPFcb->SectionObjectPointers, &ByteRangeList[ulIndex].FileOffset, ulSize, FALSE)) diff --git a/src/WINNT/afsrdr/kernel/lib/Include/AFSCommon.h b/src/WINNT/afsrdr/kernel/lib/Include/AFSCommon.h index 416c39d97..b3d885aa0 100644 --- a/src/WINNT/afsrdr/kernel/lib/Include/AFSCommon.h +++ b/src/WINNT/afsrdr/kernel/lib/Include/AFSCommon.h @@ -1199,7 +1199,8 @@ AFSUpdateMetaData( IN AFSDirectoryCB *DirEntry, NTSTATUS AFSValidateEntry( IN AFSDirectoryCB *DirEntry, IN GUID *AuthGroup, - IN BOOLEAN FastCall); + IN BOOLEAN FastCall, + IN BOOLEAN SafeToPurge); AFSDirectoryCB * AFSGetSpecialShareNameEntry( IN UNICODE_STRING *ShareName, -- 2.39.5