From 9d10cc528b08a05bbec7ba5e67c1677058550515 Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Fri, 23 Mar 2018 11:58:57 -0700 Subject: [PATCH] Don't require all-user permissions on the temp directory for named mutexes on Unix under docker Avoids the need for a workaround for one of the issues seen in https://github.com/dotnet/coreclr/issues/17098 --- src/pal/src/include/pal/sharedmemory.h | 3 ++- src/pal/src/sharedmemory/sharedmemory.cpp | 33 ++++++++++++++++++++--- src/pal/src/synchobj/mutex.cpp | 1 + 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/pal/src/include/pal/sharedmemory.h b/src/pal/src/include/pal/sharedmemory.h index 2e0d9d2a79c6..fdc395e3c6df 100644 --- a/src/pal/src/include/pal/sharedmemory.h +++ b/src/pal/src/include/pal/sharedmemory.h @@ -93,6 +93,7 @@ class SharedMemoryException class SharedMemoryHelpers { private: + static const mode_t PermissionsMask_CurrentUser_ReadWriteExecute; static const mode_t PermissionsMask_AllUsers_ReadWrite; static const mode_t PermissionsMask_AllUsers_ReadWriteExecute; public: @@ -110,7 +111,7 @@ class SharedMemoryHelpers template static SIZE_T CopyString(char (&destination)[DestinationByteCount], SIZE_T destinationStartOffset, LPCSTR source, SIZE_T sourceCharCount); template static SIZE_T AppendUInt32String(char (&destination)[DestinationByteCount], SIZE_T destinationStartOffset, UINT32 value); - static bool EnsureDirectoryExists(const char *path, bool isGlobalLockAcquired, bool createIfNotExist = true); + static bool EnsureDirectoryExists(const char *path, bool isGlobalLockAcquired, bool createIfNotExist = true, bool isSystemDirectory = false); private: static int Open(LPCSTR path, int flags, mode_t mode = static_cast(0)); public: diff --git a/src/pal/src/sharedmemory/sharedmemory.cpp b/src/pal/src/sharedmemory/sharedmemory.cpp index 9db1998c0eb8..46c07143a19a 100644 --- a/src/pal/src/sharedmemory/sharedmemory.cpp +++ b/src/pal/src/sharedmemory/sharedmemory.cpp @@ -62,6 +62,7 @@ DWORD SharedMemoryException::GetErrorCode() const //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // SharedMemoryHelpers +const mode_t SharedMemoryHelpers::PermissionsMask_CurrentUser_ReadWriteExecute = S_IRUSR | S_IWUSR | S_IXUSR; const mode_t SharedMemoryHelpers::PermissionsMask_AllUsers_ReadWrite = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH; const mode_t SharedMemoryHelpers::PermissionsMask_AllUsers_ReadWriteExecute = @@ -92,10 +93,16 @@ SIZE_T SharedMemoryHelpers::AlignUp(SIZE_T value, SIZE_T alignment) return AlignDown(value + (alignment - 1), alignment); } -bool SharedMemoryHelpers::EnsureDirectoryExists(const char *path, bool isGlobalLockAcquired, bool createIfNotExist) +bool SharedMemoryHelpers::EnsureDirectoryExists( + const char *path, + bool isGlobalLockAcquired, + bool createIfNotExist, + bool isSystemDirectory) { _ASSERTE(path != nullptr); + _ASSERTE(!(isSystemDirectory && createIfNotExist)); // should not create or change permissions on system directories _ASSERTE(SharedMemoryManager::IsCreationDeletionProcessLockAcquired()); + _ASSERTE(!isGlobalLockAcquired || SharedMemoryManager::IsCreationDeletionFileLockAcquired()); // Check if the path already exists struct stat statInfo; @@ -155,7 +162,24 @@ bool SharedMemoryHelpers::EnsureDirectoryExists(const char *path, bool isGlobalL throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } - // Check the directory's permissions and try to update them + if (isSystemDirectory) + { + // For system directories (such as SHARED_MEMORY_TEMP_DIRECTORY_PATH), require sufficient permissions only for the + // current user. For instance, "docker run --mount ..." to mount /tmp to some directory on the host mounts the + // destination directory with the same permissions as the source directory, which may not include some permissions for + // other users. In the docker container, other user permissions are typically not relevant and relaxing the permissions + // requirement allows for that scenario to work without having to work around it by first giving sufficient permissions + // for all users. + if ((statInfo.st_mode & PermissionsMask_CurrentUser_ReadWriteExecute) == PermissionsMask_CurrentUser_ReadWriteExecute) + { + return true; + } + throw SharedMemoryException(static_cast(SharedMemoryError::IO)); + } + + // For non-system directories (such as SHARED_MEMORY_RUNTIME_TEMP_DIRECTORY_PATH), require sufficient permissions for all + // users and try to update them if requested to create the directory, so that shared memory files may be shared by all + // processes on the system. if ((statInfo.st_mode & PermissionsMask_AllUsers_ReadWriteExecute) == PermissionsMask_AllUsers_ReadWriteExecute) { return true; @@ -214,6 +238,8 @@ int SharedMemoryHelpers::CreateOrOpenFile(LPCSTR path, bool createIfNotExist, bo { _ASSERTE(path != nullptr); _ASSERTE(path[0] != '\0'); + _ASSERTE(SharedMemoryManager::IsCreationDeletionProcessLockAcquired()); + _ASSERTE(!createIfNotExist || SharedMemoryManager::IsCreationDeletionFileLockAcquired()); // Try to open the file int openFlags = O_RDWR; @@ -1032,7 +1058,8 @@ void SharedMemoryManager::AcquireCreationDeletionFileLock() if (!SharedMemoryHelpers::EnsureDirectoryExists( SHARED_MEMORY_TEMP_DIRECTORY_PATH, false /* isGlobalLockAcquired */, - false /* createIfNotExist */)) + false /* createIfNotExist */, + true /* isSystemDirectory */)) { throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } diff --git a/src/pal/src/synchobj/mutex.cpp b/src/pal/src/synchobj/mutex.cpp index ccebd3b26152..d5f4edd110c7 100644 --- a/src/pal/src/synchobj/mutex.cpp +++ b/src/pal/src/synchobj/mutex.cpp @@ -1159,6 +1159,7 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen( { // If the shared memory file was created, the creation/deletion file lock would have been acquired so that we can // initialize the shared data + _ASSERTE(SharedMemoryManager::IsCreationDeletionFileLockAcquired()); autoCleanup.m_acquiredCreationDeletionFileLock = true; } if (processDataHeader == nullptr)