From eda24a6013a6d83ebad0d9310829838790affbc2 Mon Sep 17 00:00:00 2001 From: William Baker Date: Wed, 15 Aug 2018 14:38:26 -0700 Subject: [PATCH 1/2] Add FileOp specific ShouldHandleFileOpEvent helper to kext --- .../PrjFSKext/PrjFSKext/KauthHandler.cpp | 109 +++++++++++++----- 1 file changed, 81 insertions(+), 28 deletions(-) diff --git a/ProjFS.Mac/PrjFSKext/PrjFSKext/KauthHandler.cpp b/ProjFS.Mac/PrjFSKext/PrjFSKext/KauthHandler.cpp index 5f23cb9b1..ea3ed4d87 100644 --- a/ProjFS.Mac/PrjFSKext/PrjFSKext/KauthHandler.cpp +++ b/ProjFS.Mac/PrjFSKext/PrjFSKext/KauthHandler.cpp @@ -55,7 +55,7 @@ static bool TrySendRequestAndWaitForResponse( static void AbortAllOutstandingEvents(); static bool ShouldIgnoreVnodeType(vtype vnodeType, vnode_t vnode); -static bool ShouldHandleEvent( +static bool ShouldHandleVnodeOpEvent( // In params: vfs_context_t context, const vnode_t vnode, @@ -69,6 +69,15 @@ static bool ShouldHandleEvent( char procname[MAXCOMLEN + 1], int* kauthResult); +static bool ShouldHandleFileOpEvent( + // In params: + vfs_context_t context, + const vnode_t vnode, + kauth_action_t action, + + // Out params: + VirtualizationRoot** root, + int* pid); // Structs typedef struct OutstandingMessage @@ -236,7 +245,7 @@ static int HandleVnodeOperation( int kauthResult = KAUTH_RESULT_DEFER; - if (!ShouldHandleEvent( + if (!ShouldHandleVnodeOpEvent( context, currentVnode, action, @@ -310,6 +319,8 @@ static int HandleVnodeOperation( return kauthResult; } +// Note: a fileop listener MUST NOT return an error, or it will result in a kernel panic. +// Fileop events are informational only. static int HandleFileOpOperation( kauth_cred_t credential, void* idata, @@ -321,11 +332,6 @@ static int HandleFileOpOperation( { atomic_fetch_add(&s_numActiveKauthEvents, 1); - // Note: a fileop listener MUST NOT return an error, or it will result in a kernel panic. - // Fileop events are informational only. - int kauthError; - int kauthResult; - vfs_context_t context = vfs_context_create(NULL); if (KAUTH_FILEOP_CLOSE == action) @@ -334,28 +340,25 @@ static int HandleFileOpOperation( // arg1 is the (const char *) path int closeFlags = static_cast(arg2); - VirtualizationRoot* root = nullptr; - vtype vnodeType; - uint32_t currentVnodeFileFlags; - int pid; - char procname[MAXCOMLEN + 1]; - - if (!ShouldHandleEvent( - context, - currentVnode, - action, - &root, - &vnodeType, - ¤tVnodeFileFlags, - &pid, - procname, - &kauthResult)) - { - goto CleanupAndReturn; - } - if (KAUTH_FILEOP_CLOSE_MODIFIED == closeFlags) { + VirtualizationRoot* root = nullptr; + int pid; + if (!ShouldHandleFileOpEvent( + context, + currentVnode, + action, + &root, + &pid)) + { + goto CleanupAndReturn; + } + + char procname[MAXCOMLEN + 1]; + proc_name(pid, procname, MAXCOMLEN + 1); + + int kauthResult; + int kauthError; if (!TrySendRequestAndWaitForResponse( root, MessageType_KtoU_NotifyFileModified, @@ -379,7 +382,7 @@ static int HandleFileOpOperation( return KAUTH_RESULT_DEFER; } -static bool ShouldHandleEvent( +static bool ShouldHandleVnodeOpEvent( // In params: vfs_context_t context, const vnode_t vnode, @@ -539,6 +542,56 @@ static bool ShouldHandleEvent( return true; } +static bool ShouldHandleFileOpEvent( + // In params: + vfs_context_t context, + const vnode_t vnode, + kauth_action_t action, + + // Out params: + VirtualizationRoot** root, + int* pid) +{ + vtype vnodeType = vnode_vtype(vnode); + if (ShouldIgnoreVnodeType(vnodeType, vnode)) + { + return false; + } + + // TODO(Mac): We will still want to handle renames into a root, and those vnodes would + // not yet have the FileFlags_IsInVirtualizationRoot set + uint32_t vnodeFileFlags = ReadVNodeFileFlags(vnode, context); + if (!FileFlagsBitIsSet(vnodeFileFlags, FileFlags_IsInVirtualizationRoot)) + { + // This vnode is not part of ANY virtualization root, so exit now before doing any more work. + // This gives us a cheap way to avoid adding overhead to IO outside of a virtualization root. + return false; + } + + *root = VirtualizationRoots_FindForVnode(vnode); + int16_t rootIndex = nullptr == *root ? -1 : (*root)->index; + + if (nullptr == *root) + { + KextLog_FileNote(vnode, "ShouldHandleFileOpEvent(%d): No virtualization root found for file with set flag.", action); + return false; + } + else if (nullptr == (*root)->providerUserClient) + { + // There is no registered provider for this root + return false; + } + + // If the calling process is the provider, we must exit right away to avoid deadlocks + *pid = GetPid(context); + if (*pid == (*root)->providerPid) + { + return false; + } + + return true; +} + static bool TrySendRequestAndWaitForResponse( const VirtualizationRoot* root, MessageType messageType, From 4a4f5e7c90eeedff4a95716c99800dd009d71688 Mon Sep 17 00:00:00 2001 From: William Baker Date: Wed, 15 Aug 2018 14:49:27 -0700 Subject: [PATCH 2/2] Remove unused local variable --- ProjFS.Mac/PrjFSKext/PrjFSKext/KauthHandler.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/ProjFS.Mac/PrjFSKext/PrjFSKext/KauthHandler.cpp b/ProjFS.Mac/PrjFSKext/PrjFSKext/KauthHandler.cpp index ea3ed4d87..7d5f1b3d6 100644 --- a/ProjFS.Mac/PrjFSKext/PrjFSKext/KauthHandler.cpp +++ b/ProjFS.Mac/PrjFSKext/PrjFSKext/KauthHandler.cpp @@ -569,8 +569,6 @@ static bool ShouldHandleFileOpEvent( } *root = VirtualizationRoots_FindForVnode(vnode); - int16_t rootIndex = nullptr == *root ? -1 : (*root)->index; - if (nullptr == *root) { KextLog_FileNote(vnode, "ShouldHandleFileOpEvent(%d): No virtualization root found for file with set flag.", action);