Don't require all-user permissions on the temp directory for named mutexes on Unix...
authorKoundinya Veluri <kouvel@users.noreply.github.com>
Mon, 26 Mar 2018 11:06:55 +0000 (04:06 -0700)
committerJan Vorlicek <janvorli@microsoft.com>
Mon, 26 Mar 2018 11:06:55 +0000 (13:06 +0200)
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
src/pal/src/sharedmemory/sharedmemory.cpp
src/pal/src/synchobj/mutex.cpp

index 2e0d9d2a79c6c59dc3ffe57221263a463005e324..fdc395e3c6df2a30d2a59128643f86efd3935666 100644 (file)
@@ -93,6 +93,7 @@ public:
 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 @@ public:
     template<SIZE_T DestinationByteCount> static SIZE_T CopyString(char (&destination)[DestinationByteCount], SIZE_T destinationStartOffset, LPCSTR source, SIZE_T sourceCharCount);
     template<SIZE_T DestinationByteCount> 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<mode_t>(0));
 public:
index 9db1998c0eb841ff9916cdfee2e7f8755294e727..46c07143a19acf8d9d692a5b54fedd21eb826ef3 100644 (file)
@@ -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<DWORD>(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<DWORD>(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<DWORD>(SharedMemoryError::IO));
         }
index ccebd3b261521c13a43e9cb386bdb2781d7ed6f2..d5f4edd110c7694763b93003a57c4b8260ea9a11 100644 (file)
@@ -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)