Skip to content

Commit

Permalink
Merge pull request #322 from pmj/virt-root-refactor
Browse files Browse the repository at this point in the history 
Mac kext: Virtualisation root refactoring & array resizing
  • Loading branch information
@pmj
pmj authored Oct 6, 2018
2 parents 4057d91 + 0a0ac88 commit 5747bf8
Show file tree
Hide file tree
Showing 7 changed files with 285 additions and 124 deletions.
7 changes: 6 additions & 1 deletion ProjFS.Mac/PrjFSKext/PrjFSKext.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@
"$(inherited)",
"MACH_ASSERT=1",
);
GCC_WARN_SHADOW = YES;
INFOPLIST_FILE = PrjFSKext/Info.plist;
MODULE_NAME = io.gvfs.PrjFSKext;
MODULE_START = PrjFSKext_Start;
Expand All @@ -430,7 +431,11 @@
buildSettings = {
CODE_SIGN_STYLE = Automatic;
DEVELOPMENT_TEAM = UBF8T346G9;
GCC_PREPROCESSOR_DEFINITIONS = "MACH_ASSERT=1";
GCC_PREPROCESSOR_DEFINITIONS = (
"MACH_ASSERT=1",
"$(inherited)",
);
GCC_WARN_SHADOW = YES;
INFOPLIST_FILE = PrjFSKext/Info.plist;
MODULE_NAME = io.gvfs.PrjFSKext;
MODULE_START = PrjFSKext_Start;
Expand Down
91 changes: 47 additions & 44 deletions ProjFS.Mac/PrjFSKext/PrjFSKext/KauthHandler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,9 @@ static inline bool ActionBitIsSet(kauth_action_t action, kauth_action_t mask);

static bool IsFileSystemCrawler(char* procname);

static const char* GetRelativePath(const char* path, const char* root);

static void Sleep(int seconds, void* channel);
static bool TrySendRequestAndWaitForResponse(
const VirtualizationRoot* root,
VirtualizationRootHandle root,
MessageType messageType,
const vnode_t vnode,
const FsidInode& vnodeFsidInode,
Expand All @@ -65,7 +63,7 @@ static bool ShouldHandleVnodeOpEvent(
kauth_action_t action,

// Out params:
VirtualizationRoot** root,
VirtualizationRootHandle* root,
vtype* vnodeType,
uint32_t* vnodeFileFlags,
FsidInode* vnodeFsidInode,
Expand All @@ -80,7 +78,7 @@ static bool ShouldHandleFileOpEvent(
kauth_action_t action,

// Out params:
VirtualizationRoot** root,
VirtualizationRootHandle* root,
FsidInode* vnodeFsidInode,
int* pid);

Expand Down Expand Up @@ -260,12 +258,24 @@ static int HandleVnodeOperation(
// arg2 is the (vnode_t) parent vnode
int* kauthError = reinterpret_cast<int*>(arg3);
int kauthResult = KAUTH_RESULT_DEFER;
bool putVnodeWhenDone = false;

// A lot of our file checks such as attribute tests behave oddly if the vnode
// refers to a named fork/stream; apply the logic as if the vnode operation was
// occurring on the file itself. (/path/to/file/..namedfork/rsrc)
if (vnode_isnamedstream(currentVnode))
{
vnode_t mainFileFork = vnode_getparent(currentVnode);
assert(NULLVP != mainFileFork);
currentVnode = mainFileFork;
putVnodeWhenDone = true;
}

const char* vnodePath = nullptr;
char vnodePathBuffer[PrjFSMaxPath];
int vnodePathLength = PrjFSMaxPath;

VirtualizationRoot* root = nullptr;
VirtualizationRootHandle root = RootHandle_None;
vtype vnodeType;
uint32_t currentVnodeFileFlags;
FsidInode vnodeFsidInode;
Expand Down Expand Up @@ -384,6 +394,11 @@ static int HandleVnodeOperation(
}

CleanupAndReturn:
if (putVnodeWhenDone)
{
vnode_put(currentVnode);
}

atomic_fetch_sub(&s_numActiveKauthEvents, 1);
return kauthResult;
}
Expand All @@ -408,7 +423,7 @@ static int HandleFileOpOperation(
KAUTH_FILEOP_LINK == action)
{
// arg0 is the (const char *) fromPath (or the file being linked to)
const char* newPath = (const char*)arg1;
const char* newPath = reinterpret_cast<const char*>(arg1);

// TODO(Mac): We need to handle failures to lookup the vnode. If we fail to lookup the vnode
// it's possible that we'll miss notifications
Expand All @@ -418,7 +433,7 @@ static int HandleFileOpOperation(
goto CleanupAndReturn;
}

VirtualizationRoot* root = nullptr;
VirtualizationRootHandle root = RootHandle_None;
FsidInode vnodeFsidInode;
int pid;
if (!ShouldHandleFileOpEvent(
Expand Down Expand Up @@ -464,7 +479,7 @@ static int HandleFileOpOperation(
else if (KAUTH_FILEOP_CLOSE == action)
{
vnode_t currentVnode = reinterpret_cast<vnode_t>(arg0);
const char* path = (const char*)arg1;
const char* path = reinterpret_cast<const char*>(arg1);
int closeFlags = static_cast<int>(arg2);

if (vnode_isdir(currentVnode))
Expand All @@ -478,7 +493,7 @@ static int HandleFileOpOperation(
goto CleanupAndReturn;
}

VirtualizationRoot* root = nullptr;
VirtualizationRootHandle root = RootHandle_None;
FsidInode vnodeFsidInode;
int pid;
if (!ShouldHandleFileOpEvent(
Expand Down Expand Up @@ -554,16 +569,16 @@ static bool ShouldHandleVnodeOpEvent(
kauth_action_t action,

// Out params:
VirtualizationRoot** root,
VirtualizationRootHandle* root,
vtype* vnodeType,
uint32_t* vnodeFileFlags,
FsidInode* vnodeFsidInode,
int* pid,
char procname[MAXCOMLEN + 1],
int* kauthResult)
{
*root = nullptr;
*kauthResult = KAUTH_RESULT_DEFER;
*root = RootHandle_None;

if (!VirtualizationRoot_VnodeIsOnAllowedFilesystem(vnode))
{
Expand Down Expand Up @@ -609,19 +624,22 @@ static bool ShouldHandleVnodeOpEvent(
}

*vnodeFsidInode = Vnode_GetFsidAndInode(vnode, context);
*root = VirtualizationRoots_FindForVnode(vnode, *vnodeFsidInode);
*root = VirtualizationRoot_FindForVnode(vnode, *vnodeFsidInode);

if (nullptr == *root)
if (RootHandle_ProviderTemporaryDirectory == *root)
{
*kauthResult = KAUTH_RESULT_DEFER;
return false;
}
else if (RootHandle_None == *root)
{
KextLog_FileNote(vnode, "No virtualization root found for file with set flag.");

*kauthResult = KAUTH_RESULT_DEFER;
return false;
}
else if (nullptr == (*root)->providerUserClient)
else if (!VirtualizationRoot_IsOnline(*root))
{
// There is no registered provider for this root

// TODO(Mac): Protect files in the worktree from modification (and prevent
// the creation of new files) when the provider is offline

Expand All @@ -630,7 +648,7 @@ static bool ShouldHandleVnodeOpEvent(
}

// If the calling process is the provider, we must exit right away to avoid deadlocks
if (*pid == (*root)->providerPid)
if (VirtualizationRoot_PIDMatchesProvider(*root, *pid))
{
*kauthResult = KAUTH_RESULT_DEFER;
return false;
Expand All @@ -646,40 +664,38 @@ static bool ShouldHandleFileOpEvent(
kauth_action_t action,

// Out params:
VirtualizationRoot** root,
VirtualizationRootHandle* root,
FsidInode* vnodeFsidInode,
int* pid)
{
*root = RootHandle_None;

vtype vnodeType = vnode_vtype(vnode);
if (ShouldIgnoreVnodeType(vnodeType, vnode))
{
return false;
}

*vnodeFsidInode = Vnode_GetFsidAndInode(vnode, context);
*root = VirtualizationRoots_FindForVnode(vnode, *vnodeFsidInode);
if (nullptr == *root)
{
return false;
}
else if (nullptr == (*root)->providerUserClient)
*root = VirtualizationRoot_FindForVnode(vnode, *vnodeFsidInode);
if (!VirtualizationRoot_IsValidRootHandle(*root))
{
// There is no registered provider for this root
// This VNode is not part of a 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)
if (VirtualizationRoot_PIDMatchesProvider(*root, *pid))
{
// If the calling process is the provider, we must exit right away to avoid deadlocks
return false;
}

return true;
}

static bool TrySendRequestAndWaitForResponse(
const VirtualizationRoot* root,
VirtualizationRootHandle root,
MessageType messageType,
const vnode_t vnode,
const FsidInode& vnodeFsidInode,
Expand All @@ -702,7 +718,7 @@ static bool TrySendRequestAndWaitForResponse(
return false;
}

const char* relativePath = GetRelativePath(vnodePath, root->path);
const char* relativePath = VirtualizationRoot_GetRootRelativePath(root, vnodePath);

int nextMessageId = OSIncrementAtomic(&s_nextMessageId);

Expand Down Expand Up @@ -731,7 +747,7 @@ static bool TrySendRequestAndWaitForResponse(

// TODO(Mac): Should we pass in the root directly, rather than root->index?
// The index seems more like a private implementation detail.
if (!isShuttingDown && 0 != ActiveProvider_SendMessage(root->index, messageSpec))
if (!isShuttingDown && 0 != ActiveProvider_SendMessage(root, messageSpec))
{
// TODO: appropriately handle unresponsive providers

Expand Down Expand Up @@ -868,19 +884,6 @@ static bool IsFileSystemCrawler(char* procname)
return false;
}

static const char* GetRelativePath(const char* path, const char* root)
{
assert(strlen(path) >= strlen(root));

const char* relativePath = path + strlen(root);
if (relativePath[0] == '/')
{
relativePath++;
}

return relativePath;
}

static bool ShouldIgnoreVnodeType(vtype vnodeType, vnode_t vnode)
{
switch (vnodeType)
Expand Down
13 changes: 13 additions & 0 deletions ProjFS.Mac/PrjFSKext/PrjFSKext/Memory.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,17 @@ kern_return_t Memory_Cleanup();
void* Memory_Alloc(uint32_t size);
void Memory_Free(void* buffer, uint32_t size);

template <typename T>
T* Memory_AllocArray(uint32_t arrayLength)
{
size_t allocBytes = arrayLength * sizeof(T);
if (allocBytes > UINT32_MAX)
{
return nullptr;
}

return static_cast<T*>(Memory_Alloc(static_cast<uint32_t>(allocBytes)));
}


#endif /* Memory_h */
20 changes: 12 additions & 8 deletions ProjFS.Mac/PrjFSKext/PrjFSKext/PrjFSProviderUserClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ static const IOExternalMethodDispatch ProviderUserClientDispatch[] =
{
[ProviderSelector_RegisterVirtualizationRootPath] =
{
&PrjFSProviderUserClient::registerVirtualizationRoot, 0, kIOUCVariableStructureSize, 1, 0
.function = &PrjFSProviderUserClient::registerVirtualizationRoot,
.checkScalarInputCount = 0,
.checkStructureInputSize = kIOUCVariableStructureSize, // null-terminated string: virtualisation root path
.checkScalarOutputCount = 1, // returned errno
.checkStructureOutputSize = 0
},
[ProviderSelector_KernelMessageResponse] =
{
Expand All @@ -37,7 +41,7 @@ bool PrjFSProviderUserClient::initWithTask(
UInt32 type,
OSDictionary* properties)
{
this->virtualizationRootIndex = -1;
this->virtualizationRootHandle = RootHandle_None;
this->pid = proc_selfpid();

if (!this->super::initWithTask(owningTask, securityToken, type, properties))
Expand Down Expand Up @@ -92,9 +96,9 @@ void PrjFSProviderUserClient::free()
// the connection.
IOReturn PrjFSProviderUserClient::clientClose()
{
int32_t root = this->virtualizationRootIndex;
this->virtualizationRootIndex = -1;
if (-1 != root)
VirtualizationRootHandle root = this->virtualizationRootHandle;
this->virtualizationRootHandle = RootHandle_None;
if (RootHandle_None != root)
{
ActiveProvider_Disconnect(root);
}
Expand Down Expand Up @@ -210,7 +214,7 @@ IOReturn PrjFSProviderUserClient::registerVirtualizationRoot(const char* rootPat
*outError = EINVAL;
return kIOReturnSuccess;
}
else if (this->virtualizationRootIndex != -1)
else if (this->virtualizationRootHandle != RootHandle_None)
{
// Already set
*outError = EBUSY;
Expand All @@ -220,11 +224,11 @@ IOReturn PrjFSProviderUserClient::registerVirtualizationRoot(const char* rootPat
VirtualizationRootResult result = VirtualizationRoot_RegisterProviderForPath(this, this->pid, rootPath);
if (0 == result.error)
{
this->virtualizationRootIndex = result.rootIndex;
this->virtualizationRootHandle = result.root;

// Sets the root index in the IORegistry for diagnostic purposes
char location[5] = "";
snprintf(location, sizeof(location), "%d", result.rootIndex);
snprintf(location, sizeof(location), "%d", result.root);
this->setLocation(location);
}

Expand Down
5 changes: 3 additions & 2 deletions ProjFS.Mac/PrjFSKext/PrjFSKext/PrjFSProviderUserClient.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "PrjFSClasses.hpp"
#include "Locks.hpp"
#include "Message.h"
#include "VirtualizationRoots.hpp"
#include <IOKit/IOUserClient.h>

struct MessageHeader;
Expand All @@ -18,8 +19,8 @@ class PrjFSProviderUserClient : public IOUserClient
Mutex dataQueueWriterMutex;
public:
pid_t pid;
// The root for which this is the provider; -1 prior to registration
int32_t virtualizationRootIndex;
// The root for which this is the provider; RootHandle_None prior to registration
VirtualizationRootHandle virtualizationRootHandle;

// IOUserClient methods:
virtual bool initWithTask(
Expand Down
Loading

0 comments on commit 5747bf8

Please sign in to comment.