diff --git a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/GitFilesTests.cs b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/GitFilesTests.cs index 7020b61c42..bb72cab8e1 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/GitFilesTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/GitFilesTests.cs @@ -376,19 +376,23 @@ public void FileMovedFromOutsideRepoToInside() [TestCase, Order(17)] public void FileMovedFromInsideRepoToOutside() { - string fileName = "InsideRepoToOutside.txt"; - string fileInsideRepo = this.Enlistment.GetVirtualPathTo(fileName); - this.fileSystem.WriteAllText(fileInsideRepo, "Contents for the new file"); - fileInsideRepo.ShouldBeAFile(this.fileSystem); - this.Enlistment.WaitForBackgroundOperations(); - GVFSHelpers.ModifiedPathsShouldContain(this.Enlistment, this.fileSystem, fileName); + string fileInsideRepoEntry = "GitCommandsTests/RenameFileTests/1/#test"; + string fileNameFullPath = Path.Combine("GitCommandsTests", "RenameFileTests", "1", "#test"); + string fileInsideRepo = this.Enlistment.GetVirtualPathTo(fileNameFullPath); + GVFSHelpers.ModifiedPathsShouldNotContain(this.Enlistment, this.fileSystem, fileInsideRepoEntry); + + string fileNameOutsideRepo = "FileNameOutSideRepo"; + string fileMovedOutsideRepo = Path.Combine(this.Enlistment.EnlistmentRoot, fileNameOutsideRepo); + GVFSHelpers.ModifiedPathsShouldNotContain(this.Enlistment, this.fileSystem, fileNameOutsideRepo); - string fileMovedOutsideRepo = Path.Combine(this.Enlistment.EnlistmentRoot, fileName); this.fileSystem.MoveFile(fileInsideRepo, fileMovedOutsideRepo); + fileInsideRepo.ShouldNotExistOnDisk(this.fileSystem); fileMovedOutsideRepo.ShouldBeAFile(this.fileSystem); + this.fileSystem.ReadAllText(fileMovedOutsideRepo).ShouldContain("test"); this.Enlistment.WaitForBackgroundOperations(); - GVFSHelpers.ModifiedPathsShouldNotContain(this.Enlistment, this.fileSystem, fileName); + GVFSHelpers.ModifiedPathsShouldContain(this.Enlistment, this.fileSystem, fileInsideRepoEntry); + GVFSHelpers.ModifiedPathsShouldNotContain(this.Enlistment, this.fileSystem, fileNameOutsideRepo); } [TestCase, Order(18)] diff --git a/ProjFS.Mac/PrjFS.xcodeproj/project.pbxproj b/ProjFS.Mac/PrjFS.xcodeproj/project.pbxproj index 1f67fc5a87..78492f96f8 100644 --- a/ProjFS.Mac/PrjFS.xcodeproj/project.pbxproj +++ b/ProjFS.Mac/PrjFS.xcodeproj/project.pbxproj @@ -283,6 +283,8 @@ 4A08257021E77BDD00E21AFD /* org.vfsforgit.prjfs.PrjFSKextLogDaemon.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = org.vfsforgit.prjfs.PrjFSKextLogDaemon.plist; sourceTree = ""; }; 4A08257121E77BDD00E21AFD /* PrjFSKextLogDaemon.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PrjFSKextLogDaemon.cpp; sourceTree = ""; }; 4A08257221E77BDD00E21AFD /* Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; + 4A2A699B2295AA7800ACAAAF /* ArrayUtilities.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = ArrayUtilities.hpp; sourceTree = ""; }; + 4A2A699D2296BACE00ACAAAF /* KauthHandlerPrivate.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = KauthHandlerPrivate.hpp; sourceTree = ""; }; 4A2A699F2297339A00ACAAAF /* KextAssertIntegration.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = KextAssertIntegration.m; sourceTree = ""; }; 4A2A69A12297375A00ACAAAF /* KextAssertIntegration.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = KextAssertIntegration.h; sourceTree = ""; }; 4A558DB822357AB000AFDE07 /* ProviderMessaging.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = ProviderMessaging.cpp; sourceTree = ""; }; @@ -439,9 +441,11 @@ 4391F88D21E42AA70008103C /* PrjFSKext */ = { isa = PBXGroup; children = ( + 4A2A699B2295AA7800ACAAAF /* ArrayUtilities.hpp */, 4391F88F21E42AC40008103C /* Info.plist */, 4391F89321E42AC40008103C /* KauthHandler.cpp */, 4391F8A421E42AC50008103C /* KauthHandler.hpp */, + 4A2A699D2296BACE00ACAAAF /* KauthHandlerPrivate.hpp */, F5E39C8121F11556006D65C2 /* KauthHandlerTestable.hpp */, 4391F89021E42AC40008103C /* kernel-header-wrappers */, 4391F89121E42AC40008103C /* KextLog.cpp */, diff --git a/ProjFS.Mac/PrjFSKext/ArrayUtilities.hpp b/ProjFS.Mac/PrjFSKext/ArrayUtilities.hpp new file mode 100644 index 0000000000..af873d4677 --- /dev/null +++ b/ProjFS.Mac/PrjFSKext/ArrayUtilities.hpp @@ -0,0 +1,24 @@ +#pragma once + +template + constexpr size_t Array_Size(T (&array)[size]) +{ + return size; +} + +template void Array_CopyElements(T* destination, const T* source, size_t count) +{ + for (size_t i = 0; i < count; ++i) + { + destination[i] = source[i]; + } +} + +template void Array_DefaultInit(T* array, size_t count) +{ + for (size_t i = 0; i < count; ++i) + { + array[i] = T(); + } +} + diff --git a/ProjFS.Mac/PrjFSKext/KauthHandler.cpp b/ProjFS.Mac/PrjFSKext/KauthHandler.cpp index ac8353d460..43ff4e149e 100644 --- a/ProjFS.Mac/PrjFSKext/KauthHandler.cpp +++ b/ProjFS.Mac/PrjFSKext/KauthHandler.cpp @@ -2,7 +2,10 @@ #include #include #include +#include +#include +#include "KauthHandlerPrivate.hpp" #include "public/PrjFSCommon.h" #include "public/PrjFSPerfCounter.h" #include "public/PrjFSXattrs.h" @@ -19,17 +22,30 @@ #include "ProviderMessaging.hpp" #include "VnodeCache.hpp" #include "Memory.hpp" +#include "ArrayUtilities.hpp" #ifdef KEXT_UNIT_TESTING #include "KauthHandlerTestable.hpp" #endif +template + auto clamp(const T& value, const MIN_T& min, const MAX_T& max) +{ + return value < min ? min : (value > max ? max : value); +} + enum ProviderCallbackPolicy { CallbackPolicy_AllowAny, CallbackPolicy_UserInitiatedOnly, }; +struct PendingRenameOperation +{ + vnode_t vnode; + thread_t thread; +}; + // Function prototypes KEXT_STATIC int HandleVnodeOperation( kauth_cred_t credential, @@ -101,11 +117,19 @@ KEXT_STATIC bool ShouldHandleFileOpEvent( VirtualizationRootHandle* root, int* pid); +KEXT_STATIC bool InitPendingRenames(); +KEXT_STATIC void CleanupPendingRenames(); +KEXT_STATIC void ResizePendingRenames(uint32_t newMaxPendingRenames); + // State static kauth_listener_t s_vnodeListener = nullptr; static kauth_listener_t s_fileopListener = nullptr; static atomic_int s_numActiveKauthEvents; +static SpinLock s_renameLock; +static PendingRenameOperation* s_pendingRenames = nullptr; +KEXT_STATIC uint32_t s_pendingRenameCount = 0; +KEXT_STATIC uint32_t s_maxPendingRenames = 0; // Public functions kern_return_t KauthHandler_Init() @@ -130,6 +154,11 @@ kern_return_t KauthHandler_Init() goto CleanupAndFail; } + if (!InitPendingRenames()) + { + goto CleanupAndFail; + } + s_vnodeListener = kauth_listen_scope(KAUTH_SCOPE_VNODE, HandleVnodeOperation, nullptr); if (nullptr == s_vnodeListener) { @@ -178,6 +207,8 @@ kern_return_t KauthHandler_Cleanup() WaitForListenerCompletion(); + CleanupPendingRenames(); + if (VnodeCache_Cleanup()) { result = KERN_FAILURE; @@ -216,6 +247,162 @@ KEXT_STATIC void UseMainForkIfNamedStream( } } +KEXT_STATIC bool InitPendingRenames() +{ + if (version_major >= PrjFSDarwinMajorVersion::MacOS10_14_Mojave) // Only need to track renames on Mojave and newer + { + s_renameLock = SpinLock_Alloc(); + s_maxPendingRenames = 8; // Arbitrary choice, should be maximum number of expected concurrent threads performing renames, but array will resize on demand + s_pendingRenameCount = 0; + s_pendingRenames = Memory_AllocArray(s_maxPendingRenames); + if (!SpinLock_IsValid(s_renameLock) || s_pendingRenames == nullptr) + { + return false; + } + + Array_DefaultInit(s_pendingRenames, s_maxPendingRenames); + } + + return true; +} + +KEXT_STATIC void CleanupPendingRenames() +{ + if (version_major >= PrjFSDarwinMajorVersion::MacOS10_14_Mojave) + { + if (SpinLock_IsValid(s_renameLock)) + { + SpinLock_FreeMemory(&s_renameLock); + } + + if (s_pendingRenames != nullptr) + { + assert(s_pendingRenameCount == 0); + Memory_FreeArray(s_pendingRenames, s_maxPendingRenames); + s_pendingRenames = nullptr; + s_maxPendingRenames = 0; + } + } +} + +KEXT_STATIC void ResizePendingRenames(uint32_t newMaxPendingRenames) +{ + PendingRenameOperation* newArray = Memory_AllocArray(newMaxPendingRenames); + assert(newArray != nullptr); + PendingRenameOperation* arrayToFree = nullptr; + uint32_t arrayToFreeLength = 0; + + SpinLock_Acquire(s_renameLock); + { + if (newMaxPendingRenames > s_maxPendingRenames) + { + Array_CopyElements(newArray, s_pendingRenames, s_maxPendingRenames); + Array_DefaultInit(newArray + s_maxPendingRenames, newMaxPendingRenames - s_maxPendingRenames); + + arrayToFree = s_pendingRenames; + arrayToFreeLength = s_maxPendingRenames; + + s_pendingRenames = newArray; + s_maxPendingRenames = newMaxPendingRenames; + } + else + { + arrayToFree = newArray; + arrayToFreeLength = newMaxPendingRenames; + } + } + SpinLock_Release(s_renameLock); + + if (arrayToFree != nullptr) + { + assert(arrayToFreeLength > 0); + Memory_FreeArray(arrayToFree, arrayToFreeLength); + } +} + +KEXT_STATIC void RecordPendingRenameOperation(vnode_t vnode) +{ + assertf(version_major >= PrjFSDarwinMajorVersion::MacOS10_14_Mojave, "This function should only be called from the KAUTH_FILEOP_WILL_RENAME handler, which is only supported by Darwin 18 (macOS 10.14 Mojave) and newer (version_major = %u)", version_major); + thread_t myThread = current_thread(); + + bool resizeTable; + do + { + resizeTable = false; + uint32_t resizeTableLength = 0; + + SpinLock_Acquire(s_renameLock); + { + if (s_pendingRenameCount < s_maxPendingRenames) + { + s_pendingRenames[s_pendingRenameCount].thread = myThread; + s_pendingRenames[s_pendingRenameCount].vnode = vnode; + ++s_pendingRenameCount; + } + else + { + for (uint32_t i = 0; i < s_pendingRenameCount; ++i) + { + assert(s_pendingRenames[i].thread != myThread); + } + + resizeTable = true; + + resizeTableLength = static_cast(clamp(s_maxPendingRenames * UINT64_C(2), 1u, UINT32_MAX)); + assert(resizeTableLength > s_maxPendingRenames); + } + } + SpinLock_Release(s_renameLock); + + if (resizeTable) + { + if (resizeTableLength > 16) + { + KextLog_Error("Warning: RecordPendingRenameOperation is causing pending rename array resize to %u items.", resizeTableLength); + } + ResizePendingRenames(resizeTableLength); + } + } while (resizeTable); +} + +KEXT_STATIC bool DeleteOpIsForRename(vnode_t vnode) +{ + if (version_major < PrjFSDarwinMajorVersion::MacOS10_14_Mojave) + { + // High Sierra and earlier do not support WILL_RENAME notification, so we have to assume any delete may be caused by a rename + assert(s_pendingRenameCount == 0); + assert(s_maxPendingRenames == 0); + return true; + } + + bool isRename = false; + + thread_t myThread = current_thread(); + + SpinLock_Acquire(s_renameLock); + { + for (uint32_t i = 0; i < s_pendingRenameCount; ++i) + { + if (s_pendingRenames[i].thread == myThread) + { + isRename = true; + assert(s_pendingRenames[i].vnode == vnode); + --s_pendingRenameCount; + if (i != s_pendingRenameCount) + { + s_pendingRenames[i] = s_pendingRenames[s_pendingRenameCount]; + } + + s_pendingRenames[s_pendingRenameCount] = PendingRenameOperation{}; + break; + } + } + } + SpinLock_Release(s_renameLock); + + return isRename; +} + // Private functions KEXT_STATIC int HandleVnodeOperation( kauth_cred_t credential, @@ -247,6 +434,24 @@ KEXT_STATIC int HandleVnodeOperation( char procname[MAXCOMLEN + 1] = ""; bool isDeleteAction = false; bool isDirectory = false; + bool isRename = false; + + { + PerfSample considerVnodeSample(&perfTracer, PrjFSPerfCounter_VnodeOp_BasicVnodeChecks); + if (!VnodeIsEligibleForEventHandling(currentVnode)) + { + goto CleanupAndReturn; + } + } + + isDeleteAction = ActionBitIsSet(action, KAUTH_VNODE_DELETE); + if (isDeleteAction) + { + // This removes a matching entry from the array, so must run under the same + // conditions as the original RecordPendingRenameOperation call - hence early + // in the callback. + isRename = DeleteOpIsForRename(currentVnode); + } if (!ShouldHandleVnodeOpEvent( &perfTracer, @@ -262,22 +467,21 @@ KEXT_STATIC int HandleVnodeOperation( goto CleanupAndReturn; } - isDeleteAction = ActionBitIsSet(action, KAUTH_VNODE_DELETE); isDirectory = vnode_isdir(currentVnode); if (isDirectory) { - if (ActionBitIsSet( + if (isRename || + ActionBitIsSet( action, KAUTH_VNODE_LIST_DIRECTORY | KAUTH_VNODE_SEARCH | KAUTH_VNODE_READ_SECURITY | KAUTH_VNODE_READ_ATTRIBUTES | - KAUTH_VNODE_READ_EXTATTRIBUTES | - KAUTH_VNODE_DELETE)) + KAUTH_VNODE_READ_EXTATTRIBUTES)) { - // Recursively expand directory on delete to ensure child placeholders are created before rename operations - if (isDeleteAction) + // Recursively expand directory on rename as user will expect the moved directory to have the same contents as in its original location + if (isRename) { // Prevent system services from expanding directories as part of enumeration as this tends to cause deadlocks with the kauth listeners for Antivirus software if (!TryGetVirtualizationRoot(&perfTracer, context, currentVnode, pid, CallbackPolicy_UserInitiatedOnly, &root, &vnodeFsidInode, &kauthResult, kauthError)) @@ -331,7 +535,8 @@ KEXT_STATIC int HandleVnodeOperation( } else { - if (ActionBitIsSet( + if (isRename || // Hydrate before a file is moved as the user will not expect an empty file at the new location + ActionBitIsSet( action, KAUTH_VNODE_READ_ATTRIBUTES | KAUTH_VNODE_WRITE_ATTRIBUTES | @@ -340,7 +545,6 @@ KEXT_STATIC int HandleVnodeOperation( KAUTH_VNODE_READ_DATA | KAUTH_VNODE_WRITE_DATA | KAUTH_VNODE_EXECUTE | - KAUTH_VNODE_DELETE | // Hydrate on delete to ensure files are hydrated before rename operations KAUTH_VNODE_APPEND_DATA)) { if (FileFlagsBitIsSet(currentVnodeFileFlags, FileFlags_IsEmpty)) @@ -419,7 +623,7 @@ KEXT_STATIC int HandleVnodeOperation( root, isDirectory ? MessageType_KtoU_NotifyDirectoryPreDelete : - MessageType_KtoU_NotifyFilePreDelete, + isRename ? MessageType_KtoU_NotifyFilePreDeleteFromRename : MessageType_KtoU_NotifyFilePreDelete, currentVnode, vnodeFsidInode, nullptr, // path not needed, use fsid/inode @@ -757,6 +961,15 @@ KEXT_STATIC int HandleFileOpOperation( goto CleanupAndReturn; } } + else if (KAUTH_FILEOP_WILL_RENAME == action) + { + currentVnode = reinterpret_cast(arg0); + if (VnodeIsEligibleForEventHandling(currentVnode)) + { + // Records thread/vnode as rename() operation in progress, enabling optimisation in subsequent DELETE vnode listener handler + RecordPendingRenameOperation(currentVnode); + } + } CleanupAndReturn: if (NULLVP != currentVnode && putCurrentVnode) @@ -823,15 +1036,6 @@ KEXT_STATIC bool ShouldHandleVnodeOpEvent( } } - { - PerfSample considerVnodeSample(perfTracer, PrjFSPerfCounter_VnodeOp_ShouldHandle_BasicVnodeChecks); - if (!VnodeIsEligibleForEventHandling(vnode)) - { - *kauthResult = KAUTH_RESULT_DEFER; - return false; - } - } - { PerfSample readFlagsSample(perfTracer, PrjFSPerfCounter_VnodeOp_ShouldHandle_ReadFileFlags); if (!TryReadVNodeFileFlags(vnode, context, vnodeFileFlags)) diff --git a/ProjFS.Mac/PrjFSKext/KauthHandlerPrivate.hpp b/ProjFS.Mac/PrjFSKext/KauthHandlerPrivate.hpp new file mode 100644 index 0000000000..72a3ec158a --- /dev/null +++ b/ProjFS.Mac/PrjFSKext/KauthHandlerPrivate.hpp @@ -0,0 +1,8 @@ +#pragma once + +#include "kernel-header-wrappers/kauth.h" + +// Define missing symbol when building with 10.13 SDK/Xcode 9 +#ifndef KAUTH_FILEOP_WILL_RENAME +#define KAUTH_FILEOP_WILL_RENAME 8 +#endif diff --git a/ProjFS.Mac/PrjFSKext/KauthHandlerTestable.hpp b/ProjFS.Mac/PrjFSKext/KauthHandlerTestable.hpp index 5ca41ffa64..1cae4b6d94 100644 --- a/ProjFS.Mac/PrjFSKext/KauthHandlerTestable.hpp +++ b/ProjFS.Mac/PrjFSKext/KauthHandlerTestable.hpp @@ -5,6 +5,7 @@ #include "../PrjFSKext/PerformanceTracing.hpp" #include "../PrjFSKext/VirtualizationRoots.hpp" #include "KauthHandler.hpp" +#include "KauthHandlerPrivate.hpp" #ifndef __cplusplus #error None of the kext code is set up for being called from C or Objective-C; change the including file to C++ or Objective-C++ @@ -16,6 +17,9 @@ struct FsidInode; +extern uint32_t s_maxPendingRenames; +extern uint32_t s_pendingRenameCount; + KEXT_STATIC_INLINE bool FileFlagsBitIsSet(uint32_t fileFlags, uint32_t bit); KEXT_STATIC_INLINE bool ActionBitIsSet(kauth_action_t action, kauth_action_t mask); KEXT_STATIC_INLINE bool TryGetFileIsFlaggedAsInRoot(vnode_t vnode, vfs_context_t context, bool* flaggedInRoot); @@ -64,3 +68,9 @@ KEXT_STATIC bool ShouldHandleFileOpEvent( int* pid); KEXT_STATIC void UseMainForkIfNamedStream(vnode_t& vnode, bool& putVnodeWhenDone); KEXT_STATIC bool CurrentProcessWasSpawnedByRegularUser(); +KEXT_STATIC bool InitPendingRenames(); +KEXT_STATIC void CleanupPendingRenames(); +KEXT_STATIC void ResizePendingRenames(uint32_t newMaxPendingRenames); +KEXT_STATIC void RecordPendingRenameOperation(vnode_t vnode); +KEXT_STATIC bool DeleteOpIsForRename(vnode_t vnode); + diff --git a/ProjFS.Mac/PrjFSKext/Locks.cpp b/ProjFS.Mac/PrjFSKext/Locks.cpp index 4c15aeac09..c4662c810e 100644 --- a/ProjFS.Mac/PrjFSKext/Locks.cpp +++ b/ProjFS.Mac/PrjFSKext/Locks.cpp @@ -180,3 +180,30 @@ void RWLock_DropExclusiveToShared(RWLock& rwLock) #endif } + +SpinLock SpinLock_Alloc() +{ + return SpinLock { lck_spin_alloc_init(s_lockGroup, LCK_ATTR_NULL) }; +} + +void SpinLock_FreeMemory(SpinLock* lock) +{ + assert(lock->p != nullptr); + lck_spin_free(lock->p, s_lockGroup); + lock->p = nullptr; +} + +bool SpinLock_IsValid(SpinLock lock) +{ + return lock.p != nullptr; +} + +void SpinLock_Acquire(SpinLock lock) +{ + lck_spin_lock(lock.p); +} + +void SpinLock_Release(SpinLock lock) +{ + lck_spin_unlock(lock.p); +} diff --git a/ProjFS.Mac/PrjFSKext/Locks.hpp b/ProjFS.Mac/PrjFSKext/Locks.hpp index b4b7ca118d..11d3e63dec 100644 --- a/ProjFS.Mac/PrjFSKext/Locks.hpp +++ b/ProjFS.Mac/PrjFSKext/Locks.hpp @@ -49,6 +49,17 @@ void RWLock_ReleaseExclusive(RWLock& rwLock); void RWLock_DropExclusiveToShared(RWLock& rwLock); bool RWLock_AcquireSharedToExclusive(RWLock& rwLock); +typedef struct __lck_spin_t__ lck_spin_t; +struct SpinLock +{ + lck_spin_t* p; +}; + +SpinLock SpinLock_Alloc(); +void SpinLock_FreeMemory(SpinLock* lock); +bool SpinLock_IsValid(SpinLock lock); +void SpinLock_Acquire(SpinLock lock); +void SpinLock_Release(SpinLock lock); #endif /* Locks_h */ diff --git a/ProjFS.Mac/PrjFSKext/Message_Kernel.cpp b/ProjFS.Mac/PrjFSKext/Message_Kernel.cpp index 79f9e7fd6b..396802a881 100644 --- a/ProjFS.Mac/PrjFSKext/Message_Kernel.cpp +++ b/ProjFS.Mac/PrjFSKext/Message_Kernel.cpp @@ -1,13 +1,8 @@ #include "Message_Kernel.hpp" +#include "ArrayUtilities.hpp" #include #include -template - constexpr size_t Array_Size(T (&array)[size]) -{ - return size; -} - void Message_Init( Message* spec, MessageHeader* header, diff --git a/ProjFS.Mac/PrjFSKext/ProviderMessaging.cpp b/ProjFS.Mac/PrjFSKext/ProviderMessaging.cpp index cd3605e4af..da037761b8 100644 --- a/ProjFS.Mac/PrjFSKext/ProviderMessaging.cpp +++ b/ProjFS.Mac/PrjFSKext/ProviderMessaging.cpp @@ -92,6 +92,7 @@ void ProviderMessaging_HandleKernelMessageResponse(VirtualizationRootHandle prov case MessageType_KtoU_HydrateFile: case MessageType_KtoU_NotifyFileModified: case MessageType_KtoU_NotifyFilePreDelete: + case MessageType_KtoU_NotifyFilePreDeleteFromRename: case MessageType_KtoU_NotifyDirectoryPreDelete: case MessageType_KtoU_NotifyFileCreated: case MessageType_KtoU_NotifyFileRenamed: diff --git a/ProjFS.Mac/PrjFSKext/public/Message.h b/ProjFS.Mac/PrjFSKext/public/Message.h index 1083ad6d6a..2af36931ef 100644 --- a/ProjFS.Mac/PrjFSKext/public/Message.h +++ b/ProjFS.Mac/PrjFSKext/public/Message.h @@ -16,6 +16,7 @@ typedef enum MessageType_KtoU_NotifyFileModified, MessageType_KtoU_NotifyFilePreDelete, + MessageType_KtoU_NotifyFilePreDeleteFromRename, MessageType_KtoU_NotifyDirectoryPreDelete, MessageType_KtoU_NotifyFileCreated, MessageType_KtoU_NotifyFileRenamed, diff --git a/ProjFS.Mac/PrjFSKext/public/PrjFSCommon.h b/ProjFS.Mac/PrjFSKext/public/PrjFSCommon.h index abd70711c0..54965a41d8 100644 --- a/ProjFS.Mac/PrjFSKext/public/PrjFSCommon.h +++ b/ProjFS.Mac/PrjFSKext/public/PrjFSCommon.h @@ -44,4 +44,14 @@ enum PrjFSServiceUserClientType #define KEXT_STATIC_INLINE static inline #endif +namespace PrjFSDarwinMajorVersion +{ + enum + { + MacOS10_13_HighSierra = 17, + MacOS10_14_Mojave = 18, + MacOS10_15_Catalina = 19, + }; +} + #endif /* PrjFSCommon_h */ diff --git a/ProjFS.Mac/PrjFSKext/public/PrjFSPerfCounter.h b/ProjFS.Mac/PrjFSKext/public/PrjFSPerfCounter.h index 0d4bda8e46..ded0712590 100644 --- a/ProjFS.Mac/PrjFSKext/public/PrjFSPerfCounter.h +++ b/ProjFS.Mac/PrjFSKext/public/PrjFSPerfCounter.h @@ -7,10 +7,10 @@ enum PrjFSPerfCounter : int32_t PrjFSPerfCounter_VnodeOp, PrjFSPerfCounter_VnodeOp_GetPath, + PrjFSPerfCounter_VnodeOp_BasicVnodeChecks, PrjFSPerfCounter_VnodeOp_ShouldHandle, PrjFSPerfCounter_VnodeOp_ShouldHandle_IsVnodeAccessCheck, PrjFSPerfCounter_VnodeOp_ShouldHandle_IgnoredVnodeAccessCheck, - PrjFSPerfCounter_VnodeOp_ShouldHandle_BasicVnodeChecks, PrjFSPerfCounter_VnodeOp_ShouldHandle_ReadFileFlags, PrjFSPerfCounter_VnodeOp_ShouldHandle_NotInAnyRoot, PrjFSPerfCounter_VnodeOp_ShouldHandle_CheckFileSystemCrawler, diff --git a/ProjFS.Mac/PrjFSKextTests/HandleFileOpOperationTests.mm b/ProjFS.Mac/PrjFSKextTests/HandleFileOpOperationTests.mm index bd25e27591..a4bd0105cd 100644 --- a/ProjFS.Mac/PrjFSKextTests/HandleFileOpOperationTests.mm +++ b/ProjFS.Mac/PrjFSKextTests/HandleFileOpOperationTests.mm @@ -21,6 +21,7 @@ using std::make_tuple; using std::shared_ptr; using std::vector; +using std::string; using KextMock::_; class PrjFSProviderUserClient @@ -457,4 +458,24 @@ - (void)testFileopHardlinkOtherRepoOtherProviderPID _)); } +- (void)testNoPendingRenameRecordedOnIneligibleFilesystem +{ + shared_ptr testMountNone = mount::Create("msdos", fsid_t{}, 0); + const string testMountPath = "/Volumes/USBSTICK"; + shared_ptr testMountRoot = testMountNone->CreateVnodeTree(testMountPath, VDIR); + const string filePath = testMountPath + "/file"; + shared_ptr testVnode = testMountNone->CreateVnodeTree(filePath); + const string renamedFilePath = filePath + "_renamed"; + + HandleFileOpOperation( + nullptr, // credential + nullptr, /* idata, unused */ + KAUTH_FILEOP_WILL_RENAME, + reinterpret_cast(testVnode.get()), + reinterpret_cast(filePath.c_str()), + reinterpret_cast(renamedFilePath.c_str()), + 0); // unused + XCTAssertEqual(0, s_pendingRenameCount); +} + @end diff --git a/ProjFS.Mac/PrjFSKextTests/HandleOperationTests.mm b/ProjFS.Mac/PrjFSKextTests/HandleOperationTests.mm index ca5366cf44..d94128af9d 100644 --- a/ProjFS.Mac/PrjFSKextTests/HandleOperationTests.mm +++ b/ProjFS.Mac/PrjFSKextTests/HandleOperationTests.mm @@ -21,8 +21,12 @@ using std::make_tuple; using std::shared_ptr; using std::vector; +using std::string; using KextMock::_; +// Darwin version of running kernel +int version_major = PrjFSDarwinMajorVersion::MacOS10_14_Mojave; + class PrjFSProviderUserClient { }; @@ -35,15 +39,31 @@ static void SetPrjFSFileXattrData(const shared_ptr& vnode) vnode->xattrs.insert(make_pair(PrjFSFileXAttrName, rootXattrData)); } +static void TestForDarwinVersionRange(int versionMin, int versionMax, void(^testBlock)(void)) +{ + const int savedVersion = version_major; + for (int version = versionMin; version <= versionMax; ++version) + { + version_major = version; + testBlock(); + } + version_major = savedVersion; +} + +static void TestForAllSupportedDarwinVersions(void(^testBlock)(void)) +{ + TestForDarwinVersionRange(PrjFSDarwinMajorVersion::MacOS10_13_HighSierra, PrjFSDarwinMajorVersion::MacOS10_15_Catalina, testBlock); +} + @interface HandleVnodeOperationTests : PFSKextTestCase @end @implementation HandleVnodeOperationTests { vfs_context_t context; - const char* repoPath; - const char* filePath; - const char* dirPath; + string repoPath; + string filePath; + string dirPath; VirtualizationRootHandle dummyRepoHandle; PrjFSProviderUserClient dummyClient; pid_t dummyClientPid; @@ -75,7 +95,7 @@ - (void) setUp testDirVnode = testMount->CreateVnodeTree(dirPath, VDIR); // Register provider for the repository path (Simulate a mount) - VirtualizationRootResult result = VirtualizationRoot_RegisterProviderForPath(&dummyClient, dummyClientPid, repoPath); + VirtualizationRootResult result = VirtualizationRoot_RegisterProviderForPath(&dummyClient, dummyClientPid, repoPath.c_str()); XCTAssertEqual(result.error, 0); self->dummyRepoHandle = result.root; @@ -102,6 +122,7 @@ - (void) tearDown cacheWrapper.FreeCache(); VirtualizationRoots_Cleanup(); + CleanupPendingRenames(); vfs_context_rele(context); MockVnodes_CheckAndClear(); MockCalls::Clear(); @@ -125,8 +146,7 @@ - (void) testEmptyFileHydrates { testFileVnode->attrValues.va_flags = FileFlags_IsEmpty | FileFlags_IsInVirtualizationRoot; SetPrjFSFileXattrData(testFileVnode); - const int actionCount = 9; - kauth_action_t actions[actionCount] = + kauth_action_t actions[] = { KAUTH_VNODE_READ_ATTRIBUTES, KAUTH_VNODE_WRITE_ATTRIBUTES, @@ -135,10 +155,10 @@ - (void) testEmptyFileHydrates { KAUTH_VNODE_READ_DATA, KAUTH_VNODE_WRITE_DATA, KAUTH_VNODE_EXECUTE, - KAUTH_VNODE_DELETE, KAUTH_VNODE_APPEND_DATA, }; - + const int actionCount = std::extent::value; + for (int i = 0; i < actionCount; i++) { XCTAssertTrue(HandleVnodeOperation( @@ -166,6 +186,53 @@ - (void) testEmptyFileHydrates { } } +- (void) testFileDeleteHydratesOnlyWhenNecessary +{ + testFileVnode->attrValues.va_flags = FileFlags_IsEmpty | FileFlags_IsInVirtualizationRoot; + SetPrjFSFileXattrData(testFileVnode); + + TestForAllSupportedDarwinVersions(^{ + InitPendingRenames(); + XCTAssertEqual( + KAUTH_RESULT_DEFER, + HandleVnodeOperation( + nullptr, + nullptr, + KAUTH_VNODE_DELETE, + reinterpret_cast(self->context), + reinterpret_cast(self->testFileVnode.get()), + 0, + 0)); + bool didHydrate = + MockCalls::DidCallFunction( + ProviderMessaging_TrySendRequestAndWaitForResponse, + _, + MessageType_KtoU_HydrateFile, + self->testFileVnode.get(), + _, + _, + _, + _, + _, + _, + nullptr); + + // On High Sierra, any delete causes hydration as it might be due to a rename: + if (version_major <= PrjFSDarwinMajorVersion::MacOS10_13_HighSierra) + { + XCTAssertTrue(didHydrate); + } + else + { + // On Mojave+, no hydration on non-rename delete: + XCTAssertFalse(didHydrate); + } + + MockCalls::Clear(); + CleanupPendingRenames(); + }); +} + - (void) testVnodeAccessCausesNoEvent { testFileVnode->attrValues.va_flags = FileFlags_IsEmpty | FileFlags_IsInVirtualizationRoot; SetPrjFSFileXattrData(testFileVnode); @@ -325,8 +392,20 @@ - (void) testEventsThatShouldNotHydrate { } } -- (void) testDeleteFile { +- (void) testHydrationOnDeleteWhenRenamingFile { testFileVnode->attrValues.va_flags = FileFlags_IsEmpty | FileFlags_IsInVirtualizationRoot; + + // Hydration on delete only occurs if deletion is caused by rename + string renamedFilePath = filePath + "_renamed"; + HandleFileOpOperation( + nullptr, // credential + nullptr, /* idata, unused */ + KAUTH_FILEOP_WILL_RENAME, + reinterpret_cast(testFileVnode.get()), + reinterpret_cast(filePath.c_str()), + reinterpret_cast(renamedFilePath.c_str()), + 0); // unused + XCTAssertTrue(HandleVnodeOperation( nullptr, nullptr, @@ -351,7 +430,7 @@ - (void) testDeleteFile { ProviderMessaging_TrySendRequestAndWaitForResponse, make_tuple( _, - MessageType_KtoU_NotifyFilePreDelete, + MessageType_KtoU_NotifyFilePreDeleteFromRename, testFileVnode.get(), _, _, @@ -364,8 +443,166 @@ - (void) testDeleteFile { XCTAssertTrue(MockCalls::CallCount(ProviderMessaging_TrySendRequestAndWaitForResponse) == 2); } -- (void) testDeleteDir { +- (void) testDeleteFileNonRenamed +{ + testFileVnode->attrValues.va_flags = FileFlags_IsEmpty | FileFlags_IsInVirtualizationRoot; + + XCTAssertTrue(HandleVnodeOperation( + nullptr, + nullptr, + KAUTH_VNODE_DELETE, + reinterpret_cast(context), + reinterpret_cast(testFileVnode.get()), + 0, + 0) == KAUTH_RESULT_DEFER); + XCTAssertTrue(MockCalls::DidCallFunction( + ProviderMessaging_TrySendRequestAndWaitForResponse, + _, + MessageType_KtoU_NotifyFilePreDelete, + testFileVnode.get(), + _, + _, + _, + _, + _, + _, + nullptr)); + + // Should not hydrate if delete is not caused by rename + XCTAssertFalse(MockCalls::DidCallFunction( + ProviderMessaging_TrySendRequestAndWaitForResponse, + _, + MessageType_KtoU_HydrateFile, + _, + _, + _, + _, + _, + _, + _, + nullptr)); +} + +- (void) testConcurrentRenameOperationRecording +{ + self->testFileVnode->attrValues.va_flags = FileFlags_IsEmpty | FileFlags_IsInVirtualizationRoot; + + string otherFilePath = repoPath + "/otherFile"; + shared_ptr otherTestFileVnode = testMount->CreateVnodeTree(otherFilePath); + otherTestFileVnode->attrValues.va_flags = FileFlags_IsEmpty | FileFlags_IsInVirtualizationRoot; + + string renamedFilePath = filePath + "_renamed"; + string renamedOtherFilePath = otherFilePath + "_renamed"; + + MockProcess_SetCurrentThreadIndex(0); + HandleFileOpOperation( + nullptr, // credential + nullptr, /* idata, unused */ + KAUTH_FILEOP_WILL_RENAME, + reinterpret_cast(testFileVnode.get()), + reinterpret_cast(filePath.c_str()), + reinterpret_cast(renamedFilePath.c_str()), + 0); // unused + + MockProcess_SetCurrentThreadIndex(1); + HandleFileOpOperation( + nullptr, // credential + nullptr, /* idata, unused */ + KAUTH_FILEOP_WILL_RENAME, + reinterpret_cast(otherTestFileVnode.get()), + reinterpret_cast(otherFilePath.c_str()), + reinterpret_cast(renamedOtherFilePath.c_str()), + 0); // unused + + MockProcess_SetCurrentThreadIndex(0); + XCTAssertTrue(HandleVnodeOperation( + nullptr, + nullptr, + KAUTH_VNODE_DELETE, + reinterpret_cast(context), + reinterpret_cast(testFileVnode.get()), + 0, + 0) == KAUTH_RESULT_DEFER); + + MockProcess_SetCurrentThreadIndex(1); + XCTAssertTrue(HandleVnodeOperation( + nullptr, + nullptr, + KAUTH_VNODE_DELETE, + reinterpret_cast(context), + reinterpret_cast(otherTestFileVnode.get()), + 0, + 0) == KAUTH_RESULT_DEFER); + + XCTAssertTrue(MockCalls::DidCallFunctionsInOrder( + ProviderMessaging_TrySendRequestAndWaitForResponse, + make_tuple( + _, + MessageType_KtoU_HydrateFile, + testFileVnode.get(), + _, + _, + _, + _, + _, + _, + nullptr), + ProviderMessaging_TrySendRequestAndWaitForResponse, + make_tuple( + _, + MessageType_KtoU_NotifyFilePreDeleteFromRename, + testFileVnode.get(), + _, + _, + _, + _, + _, + _, + nullptr), + ProviderMessaging_TrySendRequestAndWaitForResponse, + make_tuple( + _, + MessageType_KtoU_HydrateFile, + otherTestFileVnode.get(), + _, + _, + _, + _, + _, + _, + nullptr), + ProviderMessaging_TrySendRequestAndWaitForResponse, + make_tuple( + _, + MessageType_KtoU_NotifyFilePreDeleteFromRename, + otherTestFileVnode.get(), + _, + _, + _, + _, + _, + _, + nullptr)) + ); + XCTAssertTrue(MockCalls::CallCount(ProviderMessaging_TrySendRequestAndWaitForResponse) == 4); +} + + +- (void) testHydrationOnDeleteWhenRenamingDirectory +{ testDirVnode->attrValues.va_flags = FileFlags_IsInVirtualizationRoot; + + // Hydration on delete only occurs if deletion is caused by rename + string renamedDirPath = self->dirPath + "_renamed"; + HandleFileOpOperation( + nullptr, // credential + nullptr, /* idata, unused */ + KAUTH_FILEOP_WILL_RENAME, + reinterpret_cast(self->testDirVnode.get()), + reinterpret_cast(self->dirPath.c_str()), + reinterpret_cast(renamedDirPath.c_str()), + 0); // unused + XCTAssertTrue(HandleVnodeOperation( nullptr, nullptr, @@ -403,6 +640,58 @@ - (void) testDeleteDir { XCTAssertTrue(MockCalls::CallCount(ProviderMessaging_TrySendRequestAndWaitForResponse) == 2); } +- (void) testDeleteDirNonRenamed +{ + testDirVnode->attrValues.va_flags = FileFlags_IsInVirtualizationRoot; + + TestForAllSupportedDarwinVersions(^{ + XCTAssertTrue(HandleVnodeOperation( + nullptr, + nullptr, + KAUTH_VNODE_DELETE, + reinterpret_cast(self->context), + reinterpret_cast(self->testDirVnode.get()), + 0, + 0) == KAUTH_RESULT_DEFER); + XCTAssertTrue(MockCalls::DidCallFunction( + ProviderMessaging_TrySendRequestAndWaitForResponse, + _, + MessageType_KtoU_NotifyDirectoryPreDelete, + self->testDirVnode.get(), + _, + _, + _, + _, + _, + _, + nullptr)); + + // Should not enumerate if delete is not caused by rename, except on High Sierra + bool didEnumerate = MockCalls::DidCallFunction( + ProviderMessaging_TrySendRequestAndWaitForResponse, + _, + MessageType_KtoU_RecursivelyEnumerateDirectory, + self->testDirVnode.get(), + _, + _, + _, + _, + _, + _, + nullptr); + if (version_major <= PrjFSDarwinMajorVersion::MacOS10_13_HighSierra) + { + XCTAssertTrue(didEnumerate); + } + else + { + XCTAssertFalse(didEnumerate); + } + + MockCalls::Clear(); + }); +} + - (void) testEmptyDirectoryEnumerates { testDirVnode->attrValues.va_flags = FileFlags_IsEmpty | FileFlags_IsInVirtualizationRoot; const int actionCount = 5; @@ -694,9 +983,20 @@ - (void) testDeleteDirectoryWithDisappearingVirtualizationRoot { } // When the first call to getVirtualizationRoot fails, ensure no more calls are made -- (void) testDeleteDirectoryWhenFirstRequestFails { +- (void) testRenameDirectoryWhenFirstRequestFails { ProviderMessageMock_SetDefaultRequestResult(false); testDirVnode->attrValues.va_flags = FileFlags_IsInVirtualizationRoot; + + string renamedDirPath = self->dirPath + "_renamed"; + HandleFileOpOperation( + nullptr, // credential + nullptr, /* idata, unused */ + KAUTH_FILEOP_WILL_RENAME, + reinterpret_cast(self->testDirVnode.get()), + reinterpret_cast(self->dirPath.c_str()), + reinterpret_cast(renamedDirPath.c_str()), + 0); // unused + XCTAssertTrue(HandleVnodeOperation( nullptr, nullptr, @@ -709,8 +1009,20 @@ - (void) testDeleteDirectoryWhenFirstRequestFails { XCTAssertTrue(MockCalls::CallCount(ProviderMessaging_TrySendRequestAndWaitForResponse) == 1); } -- (void) testDeleteDirectoryWhenSecondRequestFails { +- (void) testDeleteDirectoryForRenameWhenSecondRequestFails { ProviderMessageMock_SetSecondRequestResult(false); + + // Hydration on delete only occurs if deletion is caused by rename + string renamedDirPath = self->dirPath + "_renamed"; + HandleFileOpOperation( + nullptr, // credential + nullptr, /* idata, unused */ + KAUTH_FILEOP_WILL_RENAME, + reinterpret_cast(self->testDirVnode.get()), + reinterpret_cast(self->dirPath.c_str()), + reinterpret_cast(renamedDirPath.c_str()), + 0); // unused + testDirVnode->attrValues.va_flags = FileFlags_IsInVirtualizationRoot; XCTAssertTrue(HandleVnodeOperation( nullptr, @@ -724,17 +1036,30 @@ - (void) testDeleteDirectoryWhenSecondRequestFails { XCTAssertTrue(MockCalls::CallCount(ProviderMessaging_TrySendRequestAndWaitForResponse) == 2); } -- (void) testDeleteDirectoryWithNoVirtualizationRoot { +- (void) testRenameDirectoryWithNoVirtualizationRoot { [self removeAllVirtualizationRoots]; testDirVnode->attrValues.va_flags = FileFlags_IsInVirtualizationRoot; - XCTAssertTrue(HandleVnodeOperation( - nullptr, - nullptr, - KAUTH_VNODE_DELETE, - reinterpret_cast(context), - reinterpret_cast(testDirVnode.get()), - 0, - 0) == KAUTH_RESULT_DEFER); + + string renamedDirPath = self->dirPath + "_renamed"; + HandleFileOpOperation( + nullptr, // credential + nullptr, // idata, unused + KAUTH_FILEOP_WILL_RENAME, + reinterpret_cast(self->testDirVnode.get()), + reinterpret_cast(self->dirPath.c_str()), + reinterpret_cast(renamedDirPath.c_str()), + 0); // unused + + XCTAssertEqual( + KAUTH_RESULT_DEFER, + HandleVnodeOperation( + nullptr, + nullptr, + KAUTH_VNODE_DELETE, + reinterpret_cast(context), + reinterpret_cast(testDirVnode.get()), + 0, + 0)); XCTAssertFalse(MockCalls::DidCallFunction(ProviderMessaging_TrySendRequestAndWaitForResponse)); } @@ -853,5 +1178,22 @@ - (void) testWriteDataToForkedPath { XCTAssertTrue(MockCalls::CallCount(ProviderMessaging_TrySendRequestAndWaitForResponse) == 0); } +- (void) testIneligibleFilesystemType +{ + shared_ptr testMountNone = mount::Create("msdos", fsid_t{}, 0); + shared_ptr testVnodeNone = testMountNone->CreateVnodeTree("/Volumes/USBSTICK", VDIR); + + XCTAssertEqual( + KAUTH_RESULT_DEFER, + HandleVnodeOperation( + nullptr, + nullptr, + KAUTH_VNODE_ADD_FILE, + reinterpret_cast(context), + reinterpret_cast(testVnodeNone.get()), + 0, + 0)); + XCTAssertFalse(MockCalls::DidCallFunction(ProviderMessaging_TrySendRequestAndWaitForResponse)); +} @end diff --git a/ProjFS.Mac/PrjFSKextTests/KauthHandlerTests.mm b/ProjFS.Mac/PrjFSKextTests/KauthHandlerTests.mm index 2f744825be..edcd996dc5 100644 --- a/ProjFS.Mac/PrjFSKextTests/KauthHandlerTests.mm +++ b/ProjFS.Mac/PrjFSKextTests/KauthHandlerTests.mm @@ -11,6 +11,7 @@ #include "VnodeCacheEntriesWrapper.hpp" using std::shared_ptr; +using std::string; @interface KauthHandlerTests : PFSKextTestCase @end @@ -37,6 +38,7 @@ - (void) tearDown { self->cacheWrapper.FreeCache(); MockVnodes_CheckAndClear(); VirtualizationRoots_Cleanup(); + MockCalls::Clear(); [super tearDown]; } @@ -344,4 +346,55 @@ - (void)testUseMainForkIfNamedStream { vnode_put(testVnode); } +- (void)testResizePendingRenamesRace +{ + InitPendingRenames(); + XCTAssertLessThan(s_maxPendingRenames, 16); + ResizePendingRenames(16); + ResizePendingRenames(16); + XCTAssertEqual(s_maxPendingRenames, 16); + CleanupPendingRenames(); +} + +- (void)testResizePendingRenamesLogsLargeArrayError +{ + shared_ptr testMount = mount::Create(); + string filePath = "/Users/test/code/Repo/file"; + shared_ptr testFile = testMount->CreateVnodeTree(filePath); + + InitPendingRenames(); + ResizePendingRenames(16); + s_pendingRenameCount = 16; // pretend we're full + XCTAssertFalse(MockCalls::DidCallFunction(KextMessageLogged, KEXTLOG_ERROR)); + RecordPendingRenameOperation(testFile.get()); + XCTAssertTrue(MockCalls::DidCallFunction(KextMessageLogged, KEXTLOG_ERROR)); + XCTAssertEqual(s_pendingRenameCount, 17); + XCTAssertGreaterThan(s_maxPendingRenames, 16); + XCTAssertTrue(DeleteOpIsForRename(testFile.get())); + XCTAssertEqual(s_pendingRenameCount, 16); + s_pendingRenameCount = 0; + CleanupPendingRenames(); +} + +- (void)testPendingRenamesOutOfOrderInsertAndRemoval +{ + shared_ptr testMount = mount::Create(); + string file1Path = "/Users/test/code/Repo/file1"; + string file2Path = "/Users/test/code/Repo/file2"; + shared_ptr testFile1 = testMount->CreateVnodeTree(file1Path); + shared_ptr testFile2 = testMount->CreateVnodeTree(file2Path); + + InitPendingRenames(); + RecordPendingRenameOperation(testFile1.get()); + MockProcess_SetCurrentThreadIndex(1); + RecordPendingRenameOperation(testFile2.get()); + MockProcess_SetCurrentThreadIndex(0); + XCTAssertTrue(DeleteOpIsForRename(testFile1.get())); + MockProcess_SetCurrentThreadIndex(1); + XCTAssertTrue(DeleteOpIsForRename(testFile2.get())); + MockProcess_SetCurrentThreadIndex(0); + XCTAssertEqual(s_pendingRenameCount, 0); + CleanupPendingRenames(); +} + @end diff --git a/ProjFS.Mac/PrjFSKextTests/MockProc.cpp b/ProjFS.Mac/PrjFSKextTests/MockProc.cpp index 2252cc3d35..7d96fbea0c 100644 --- a/ProjFS.Mac/PrjFSKextTests/MockProc.cpp +++ b/ProjFS.Mac/PrjFSKextTests/MockProc.cpp @@ -2,6 +2,10 @@ #include #include +struct thread +{ +}; + using std::make_pair; using std::map; using std::string; @@ -10,12 +14,15 @@ static map s_credentialMap; static map s_contextMap; static map s_processMap; static int s_selfPid; +static uint16_t s_currentThreadIndex = 0; +static thread s_threadPool[MockProcess_ThreadPoolSize] = {}; void MockProcess_Reset() { s_processMap.clear(); s_credentialMap.clear(); s_contextMap.clear(); + MockProcess_SetCurrentThreadIndex(0); } void MockProcess_SetSelfPid(int selfPid) @@ -157,3 +164,14 @@ void MockProcess_AddProcess(int pid, uintptr_t credentialId, int ppid, string na proc process = proc {pid, credentialId, ppid, name}; s_processMap.insert(make_pair(pid, process)); } + +kernel_thread_t current_thread() +{ + return &s_threadPool[s_currentThreadIndex]; +} + +void MockProcess_SetCurrentThreadIndex(uint16_t threadIndex) +{ + assert(threadIndex < MockProcess_ThreadPoolSize); + s_currentThreadIndex = threadIndex; +} diff --git a/ProjFS.Mac/PrjFSKextTests/MockProc.hpp b/ProjFS.Mac/PrjFSKextTests/MockProc.hpp index 8bb645a06a..a015e116bb 100644 --- a/ProjFS.Mac/PrjFSKextTests/MockProc.hpp +++ b/ProjFS.Mac/PrjFSKextTests/MockProc.hpp @@ -1,6 +1,10 @@ #include "../PrjFSKext/kernel-header-wrappers/vnode.h" #include +// In the kernel/kext this type is "thread_t" but there already exists a +// conflicting Mach type in user space hence the name change. +typedef struct thread* kernel_thread_t; + // Kernel functions that are being mocked extern "C" { @@ -16,6 +20,7 @@ extern "C" proc_t proc_find(int pid); int proc_rele(proc_t p); int proc_selfpid(void); + kernel_thread_t current_thread(void); } struct proc { @@ -30,3 +35,7 @@ void MockProcess_AddCredential(uintptr_t credentialId, uid_t UID); void MockProcess_AddContext(vfs_context_t context, int pid); void MockProcess_AddProcess(int pid, uintptr_t credentialId, int ppid, std::string procName); void MockProcess_Reset(); + +// Tests will only need to simulate a small number of threads, so we provide an existing, indexed pool. +constexpr uint16_t MockProcess_ThreadPoolSize = 2; +void MockProcess_SetCurrentThreadIndex(uint16_t threadIndex); diff --git a/ProjFS.Mac/PrjFSKextTests/TestLocks.cpp b/ProjFS.Mac/PrjFSKextTests/TestLocks.cpp index f2b6a44c65..d560cd4fa5 100644 --- a/ProjFS.Mac/PrjFSKextTests/TestLocks.cpp +++ b/ProjFS.Mac/PrjFSKextTests/TestLocks.cpp @@ -39,3 +39,25 @@ void RWLock_FreeMemory(RWLock* rwLock) delete rwLock->p; rwLock->p = nullptr; } + +void SpinLock_Acquire(SpinLock lock) +{ +} + +void SpinLock_Release(SpinLock lock) +{ +} + +SpinLock SpinLock_Alloc() +{ + return SpinLock{}; +} + +void SpinLock_FreeMemory(SpinLock* lock) +{ +} + +bool SpinLock_IsValid(SpinLock lock) +{ + return true; +} diff --git a/ProjFS.Mac/PrjFSKextTests/TestMemory.cpp b/ProjFS.Mac/PrjFSKextTests/TestMemory.cpp index ec57877acc..23bc5dbf90 100644 --- a/ProjFS.Mac/PrjFSKextTests/TestMemory.cpp +++ b/ProjFS.Mac/PrjFSKextTests/TestMemory.cpp @@ -1,11 +1,22 @@ #include "../PrjFSKext/Memory.hpp" +#include + +struct TrackedKextMemoryAllocation +{ + uint32_t size; + uint8_t data[]; +}; void Memory_Free(void* memory, uint32_t sizeBytes) { - free(memory); + TrackedKextMemoryAllocation* memoryObject = reinterpret_cast(static_cast(memory) - offsetof(TrackedKextMemoryAllocation, data)); + assert(memoryObject->size == sizeBytes); + free(memoryObject); } void* Memory_Alloc(uint32_t sizeBytes) { - return malloc(sizeBytes); + TrackedKextMemoryAllocation* memoryObject = static_cast(malloc(sizeBytes + sizeof(TrackedKextMemoryAllocation))); + memoryObject->size = sizeBytes; + return memoryObject->data; } diff --git a/ProjFS.Mac/PrjFSLib.Mac.Managed/NotificationType.cs b/ProjFS.Mac/PrjFSLib.Mac.Managed/NotificationType.cs index 08b780d728..cd3a1a5e4a 100644 --- a/ProjFS.Mac/PrjFSLib.Mac.Managed/NotificationType.cs +++ b/ProjFS.Mac/PrjFSLib.Mac.Managed/NotificationType.cs @@ -10,6 +10,7 @@ public enum NotificationType None = 0x00000001, NewFileCreated = 0x00000004, PreDelete = 0x00000010, + PreDeleteFromRename = 0x00000011, FileRenamed = 0x00000080, HardLinkCreated = 0x00000100, PreConvertToFull = 0x00001000, diff --git a/ProjFS.Mac/PrjFSLib.Mac.Managed/VirtualizationInstance.cs b/ProjFS.Mac/PrjFSLib.Mac.Managed/VirtualizationInstance.cs index 8dc5b1aaa6..eb78b7ce75 100644 --- a/ProjFS.Mac/PrjFSLib.Mac.Managed/VirtualizationInstance.cs +++ b/ProjFS.Mac/PrjFSLib.Mac.Managed/VirtualizationInstance.cs @@ -191,6 +191,7 @@ public virtual Result StopVirtualizationInstance() switch (notificationType) { case NotificationType.PreDelete: + case NotificationType.PreDeleteFromRename: return this.OnPreDelete(relativePath, isDirectory); case NotificationType.FileModified: diff --git a/ProjFS.Mac/PrjFSLib/PrjFSLib.cpp b/ProjFS.Mac/PrjFSLib/PrjFSLib.cpp index c82fce7c56..786d706653 100644 --- a/ProjFS.Mac/PrjFSLib/PrjFSLib.cpp +++ b/ProjFS.Mac/PrjFSLib/PrjFSLib.cpp @@ -753,6 +753,7 @@ static void HandleKernelRequest(void* messageMemory, uint32_t messageSize) case MessageType_KtoU_NotifyFileModified: case MessageType_KtoU_NotifyFilePreDelete: + case MessageType_KtoU_NotifyFilePreDeleteFromRename: case MessageType_KtoU_NotifyDirectoryPreDelete: case MessageType_KtoU_NotifyFilePreConvertToFull: { @@ -1122,10 +1123,13 @@ static PrjFS_Result HandleFileNotification( notificationType, nullptr /* destinationRelativePath */); - // We have to convert to full for PreDelete until we have WILL_RENAME support. The PreDelete could be for a rename. + // Remove placeholder xattrs for renames (PreDeleteFromRename) because: + // - Renames are treated as "Delete old path" + "create new path", and new files should not be marked as placeholders + // - The target of the rename might be outside of the root, and we should not allow placeholder files to + // live outside of the virtualization root (as the kext won't know what provider they belong to) if (result == PrjFS_Result_Success && placeholderFile && - (PrjFS_NotificationType_PreConvertToFull == notificationType || PrjFS_NotificationType_PreDelete == notificationType)) + (PrjFS_NotificationType_PreConvertToFull == notificationType || PrjFS_NotificationType_PreDeleteFromRename == notificationType)) { errno_t result = RemoveXAttrWithoutFollowingLinks(absolutePath, PrjFSFileXAttrName); if (0 != result) @@ -1317,6 +1321,9 @@ static inline PrjFS_NotificationType KUMessageTypeToNotificationType(MessageType case MessageType_KtoU_NotifyDirectoryPreDelete: return PrjFS_NotificationType_PreDelete; + case MessageType_KtoU_NotifyFilePreDeleteFromRename: + return PrjFS_NotificationType_PreDeleteFromRename; + case MessageType_KtoU_NotifyFilePreConvertToFull: return PrjFS_NotificationType_PreConvertToFull; @@ -1445,6 +1452,8 @@ static const char* NotificationTypeToString(PrjFS_NotificationType notificationT return STRINGIFY(PrjFS_NotificationType_PreDelete); case PrjFS_NotificationType_FileRenamed: return STRINGIFY(PrjFS_NotificationType_FileRenamed); + case PrjFS_NotificationType_PreDeleteFromRename: + return STRINGIFY(PrjFS_NotificationType_PreDeleteFromRename); case PrjFS_NotificationType_HardLinkCreated: return STRINGIFY(PrjFS_NotificationType_HardLinkCreated); case PrjFS_NotificationType_PreConvertToFull: diff --git a/ProjFS.Mac/PrjFSLib/PrjFSLib.h b/ProjFS.Mac/PrjFSLib/PrjFSLib.h index f6173b9782..21a312c2c7 100644 --- a/ProjFS.Mac/PrjFSLib/PrjFSLib.h +++ b/ProjFS.Mac/PrjFSLib/PrjFSLib.h @@ -47,6 +47,7 @@ typedef enum PrjFS_NotificationType_None = 0x00000001, PrjFS_NotificationType_NewFileCreated = 0x00000004, PrjFS_NotificationType_PreDelete = 0x00000010, + PrjFS_NotificationType_PreDeleteFromRename = 0x00000011, PrjFS_NotificationType_FileRenamed = 0x00000080, PrjFS_NotificationType_HardLinkCreated = 0x00000100, PrjFS_NotificationType_PreConvertToFull = 0x00001000, diff --git a/ProjFS.Mac/PrjFSLib/PrjFSUser.cpp b/ProjFS.Mac/PrjFSLib/PrjFSUser.cpp index a2ad096770..8e06de0bb3 100644 --- a/ProjFS.Mac/PrjFSLib/PrjFSUser.cpp +++ b/ProjFS.Mac/PrjFSLib/PrjFSUser.cpp @@ -335,8 +335,8 @@ static void InitDataQueueFunctions() return; } - if ((osVersion.major == 17 && osVersion.minor >= 7) // macOS 10.13.6+ - || (osVersion.major == 18 && osVersion.minor == 0)) // macOS 10.14(.0) exactly + if ((osVersion.major == PrjFSDarwinMajorVersion::MacOS10_13_HighSierra && osVersion.minor >= 7) // macOS 10.13.6+ + || (osVersion.major == PrjFSDarwinMajorVersion::MacOS10_14_Mojave && osVersion.minor == 0)) // macOS 10.14(.0) exactly { void* dataQueueLibrary = dlopen("libSharedDataQueue.dylib", RTLD_LAZY); if (nullptr == dataQueueLibrary) diff --git a/ProjFS.Mac/PrjFSLib/prjfs-log/kext-perf-tracing.cpp b/ProjFS.Mac/PrjFSLib/prjfs-log/kext-perf-tracing.cpp index 1e3bdfa9f8..f600e9f927 100644 --- a/ProjFS.Mac/PrjFSLib/prjfs-log/kext-perf-tracing.cpp +++ b/ProjFS.Mac/PrjFSLib/prjfs-log/kext-perf-tracing.cpp @@ -31,10 +31,10 @@ static constexpr const char* const PerfCounterNames[PrjFSPerfCounter_Count] = { [PrjFSPerfCounter_VnodeOp] = "HandleVnodeOperation", [PrjFSPerfCounter_VnodeOp_GetPath] = " |--GetPath", + [PrjFSPerfCounter_VnodeOp_BasicVnodeChecks] = " |--BasicVnodeChecks", [PrjFSPerfCounter_VnodeOp_ShouldHandle] = " |--ShouldHandleVnodeOpEvent", [PrjFSPerfCounter_VnodeOp_ShouldHandle_IsVnodeAccessCheck] = " | |--IsVnodeAccessCheck", [PrjFSPerfCounter_VnodeOp_ShouldHandle_IgnoredVnodeAccessCheck] = " | | |--IgnoredVnodeAccessCheck", - [PrjFSPerfCounter_VnodeOp_ShouldHandle_BasicVnodeChecks] = " | |--BasicVnodeChecks", [PrjFSPerfCounter_VnodeOp_ShouldHandle_ReadFileFlags] = " | |--TryReadVNodeFileFlags", [PrjFSPerfCounter_VnodeOp_ShouldHandle_NotInAnyRoot] = " | | |--NotInAnyRoot", [PrjFSPerfCounter_VnodeOp_ShouldHandle_CheckFileSystemCrawler] = " | |--IsFileSystemCrawler", diff --git a/ProjFS.Mac/Scripts/Build.sh b/ProjFS.Mac/Scripts/Build.sh index 46a3a1a300..a85697e82a 100755 --- a/ProjFS.Mac/Scripts/Build.sh +++ b/ProjFS.Mac/Scripts/Build.sh @@ -59,7 +59,8 @@ while read line; do # PrjFSKext exclusions [[ $line != *"AllArrayElementsInitialized"* ]] && #Function is used for compile time checks only [[ $line != *"KauthHandler_Init"* ]] && - [[ $line != *"KauthHandler_Cleanup"* ]] && + [[ $line != *"KauthHandler_Cleanup"* ]] && + [[ $line != *"InitPendingRenames"* ]] && [[ $line != *"HandleFileOpOperation"* ]] && #SHOULD ADD COVERAGE [[ $line != *"TryGetVirtualizationRoot"* ]] && #SHOULD ADD COVERAGE [[ $line != *"WaitForListenerCompletion"* ]] &&