Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 80783b6

Browse files
janvorlicarlossanlop
authored andcommitted
Merged PR 26445: Restrict named mutex files permissions
This change restricts the permission for files created for named mutexes that are not machine wide. Until now, all named mutexes had underlying files with access to all users. With this change, mutexes that are session local have access for the current user only. In addition to that, sticky bit is set for the `/tmp/.dotnet` and `/tmp/.dotnet/shm` directories to ensure that only creator of a subdirectory in these directories can delete the respective file / subdirectory. Here is an overview of the permissions before and after my change. The differences are shown in **bold**. The `somemutexfile` is a file with a name equal to the named mutex name. This file contains data of the mutex. Before: /tmp RWX for all users, sticky bit set /tmp/.dotnet - RWX for all users /tmp/.dotnet/shm - RWX for all users /tmp/.dotnet/shm/global - RWX for all users /tmp/.dotnet/shm/sessionXXXX - RWX for all users /tmp/.dotnet/shm/global/`somemutexfile`- RW for all users /tmp/.dotnet/shm/sessionXXXX/`somemutexfile` - RW for all users After: /tmp - RWX for all users, sticky bit set /tmp/.dotnet - RWX for all users /tmp/.dotnet/shm - RWX for all users, **sticky bit set** /tmp/.dotnet/shm/global - RWX for all users /tmp/.dotnet/shm/sessionXXXX - RWX for **current user only** /tmp/.dotnet/shm/global/`somemutexfile`- RW for all users /tmp/.dotnet/shm/sessionXXXX/`somemutexfile` - RW for **current user only**
1 parent 6163a96 commit 80783b6

File tree

3 files changed

+41
-18
lines changed

3 files changed

+41
-18
lines changed

src/pal/src/include/pal/sharedmemory.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ class SharedMemoryException
9292
class SharedMemoryHelpers
9393
{
9494
private:
95+
static const mode_t PermissionsMask_CurrentUser_ReadWrite;
9596
static const mode_t PermissionsMask_CurrentUser_ReadWriteExecute;
9697
static const mode_t PermissionsMask_AllUsers_ReadWrite;
9798
static const mode_t PermissionsMask_AllUsers_ReadWriteExecute;
@@ -110,12 +111,12 @@ class SharedMemoryHelpers
110111
static void BuildSharedFilesPath(PathCharString& destination, const char *suffix, int suffixByteCount);
111112
static bool AppendUInt32String(PathCharString& destination, UINT32 value);
112113

113-
static bool EnsureDirectoryExists(const char *path, bool isGlobalLockAcquired, bool createIfNotExist = true, bool isSystemDirectory = false);
114+
static bool EnsureDirectoryExists(const char *path, bool isGlobalLockAcquired, bool hasCurrentUserAccessOnly, bool setStickyFlag, bool createIfNotExist = true, bool isSystemDirectory = false);
114115
private:
115116
static int Open(LPCSTR path, int flags, mode_t mode = static_cast<mode_t>(0));
116117
public:
117118
static int OpenDirectory(LPCSTR path);
118-
static int CreateOrOpenFile(LPCSTR path, bool createIfNotExist = true, bool *createdRef = nullptr);
119+
static int CreateOrOpenFile(LPCSTR path, bool createIfNotExist = true, bool isSessionScope = true, bool *createdRef = nullptr);
119120
static void CloseFile(int fileDescriptor);
120121

121122
static SIZE_T GetFileSize(int fileDescriptor);

src/pal/src/sharedmemory/sharedmemory.cpp

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ DWORD SharedMemoryException::GetErrorCode() const
6363
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
6464
// SharedMemoryHelpers
6565

66+
const mode_t SharedMemoryHelpers::PermissionsMask_CurrentUser_ReadWrite = S_IRUSR | S_IWUSR;
6667
const mode_t SharedMemoryHelpers::PermissionsMask_CurrentUser_ReadWriteExecute = S_IRUSR | S_IWUSR | S_IXUSR;
6768
const mode_t SharedMemoryHelpers::PermissionsMask_AllUsers_ReadWrite =
6869
S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
@@ -97,13 +98,22 @@ SIZE_T SharedMemoryHelpers::AlignUp(SIZE_T value, SIZE_T alignment)
9798
bool SharedMemoryHelpers::EnsureDirectoryExists(
9899
const char *path,
99100
bool isGlobalLockAcquired,
101+
bool hasCurrentUserAccessOnly,
102+
bool setStickyFlag,
100103
bool createIfNotExist,
101104
bool isSystemDirectory)
102105
{
103106
_ASSERTE(path != nullptr);
104107
_ASSERTE(!(isSystemDirectory && createIfNotExist)); // should not create or change permissions on system directories
105108
_ASSERTE(SharedMemoryManager::IsCreationDeletionProcessLockAcquired());
106109
_ASSERTE(!isGlobalLockAcquired || SharedMemoryManager::IsCreationDeletionFileLockAcquired());
110+
_ASSERTE(!(setStickyFlag && hasCurrentUserAccessOnly)); // Sticky bit doesn't make sense with current user access only
111+
112+
mode_t mode = hasCurrentUserAccessOnly ? PermissionsMask_CurrentUser_ReadWriteExecute : PermissionsMask_AllUsers_ReadWriteExecute;
113+
if (setStickyFlag)
114+
{
115+
mode |= S_ISVTX;
116+
}
107117

108118
// Check if the path already exists
109119
struct stat statInfo;
@@ -124,11 +134,11 @@ bool SharedMemoryHelpers::EnsureDirectoryExists(
124134

125135
if (isGlobalLockAcquired)
126136
{
127-
if (mkdir(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
137+
if (mkdir(path, mode) != 0)
128138
{
129139
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
130140
}
131-
if (chmod(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
141+
if (chmod(path, mode) != 0)
132142
{
133143
rmdir(path);
134144
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
@@ -143,7 +153,7 @@ bool SharedMemoryHelpers::EnsureDirectoryExists(
143153
{
144154
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
145155
}
146-
if (chmod(tempPath, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
156+
if (chmod(tempPath, mode) != 0)
147157
{
148158
rmdir(tempPath);
149159
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
@@ -183,13 +193,18 @@ bool SharedMemoryHelpers::EnsureDirectoryExists(
183193
// For non-system directories (such as gSharedFilesPath/SHARED_MEMORY_RUNTIME_TEMP_DIRECTORY_NAME),
184194
// require sufficient permissions for all users and try to update them if requested to create the directory, so that
185195
// shared memory files may be shared by all processes on the system.
186-
if ((statInfo.st_mode & PermissionsMask_AllUsers_ReadWriteExecute) == PermissionsMask_AllUsers_ReadWriteExecute)
196+
if ((statInfo.st_mode & mode) == mode)
187197
{
188198
return true;
189199
}
190-
if (!createIfNotExist || chmod(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
200+
if (!createIfNotExist || chmod(path, mode) != 0)
191201
{
192-
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
202+
// We were not asked to create the path or we weren't able to set the new permissions.
203+
// As a last resort, check that at least the current user has full access.
204+
if ((statInfo.st_mode & PermissionsMask_CurrentUser_ReadWriteExecute) != PermissionsMask_CurrentUser_ReadWriteExecute)
205+
{
206+
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
207+
}
193208
}
194209
return true;
195210
}
@@ -239,7 +254,7 @@ int SharedMemoryHelpers::OpenDirectory(LPCSTR path)
239254
return fileDescriptor;
240255
}
241256

242-
int SharedMemoryHelpers::CreateOrOpenFile(LPCSTR path, bool createIfNotExist, bool *createdRef)
257+
int SharedMemoryHelpers::CreateOrOpenFile(LPCSTR path, bool createIfNotExist, bool isSessionScope, bool *createdRef)
243258
{
244259
_ASSERTE(path != nullptr);
245260
_ASSERTE(path[0] != '\0');
@@ -269,12 +284,13 @@ int SharedMemoryHelpers::CreateOrOpenFile(LPCSTR path, bool createIfNotExist, bo
269284

270285
// File does not exist, create the file
271286
openFlags |= O_CREAT | O_EXCL;
272-
fileDescriptor = Open(path, openFlags, PermissionsMask_AllUsers_ReadWrite);
287+
mode_t mode = isSessionScope ? PermissionsMask_CurrentUser_ReadWrite : PermissionsMask_AllUsers_ReadWrite;
288+
fileDescriptor = Open(path, openFlags, mode);
273289
_ASSERTE(fileDescriptor != -1);
274290

275291
// The permissions mask passed to open() is filtered by the process' permissions umask, so open() may not set all of
276292
// the requested permissions. Use chmod() to set the proper permissions.
277-
if (chmod(path, PermissionsMask_AllUsers_ReadWrite) != 0)
293+
if (chmod(path, mode) != 0)
278294
{
279295
CloseFile(fileDescriptor);
280296
unlink(path);
@@ -655,7 +671,7 @@ SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen(
655671
SharedMemoryHelpers::VerifyStringOperation(SharedMemoryManager::CopySharedMemoryBasePath(filePath));
656672
SharedMemoryHelpers::VerifyStringOperation(filePath.Append('/'));
657673
SharedMemoryHelpers::VerifyStringOperation(id.AppendSessionDirectoryName(filePath));
658-
if (!SharedMemoryHelpers::EnsureDirectoryExists(filePath, true /* isGlobalLockAcquired */, createIfNotExist))
674+
if (!SharedMemoryHelpers::EnsureDirectoryExists(filePath, true /* isGlobalLockAcquired */, id.IsSessionScope(), false /* setStickyFlag */, createIfNotExist))
659675
{
660676
_ASSERTE(!createIfNotExist);
661677
return nullptr;
@@ -668,7 +684,7 @@ SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen(
668684
SharedMemoryHelpers::VerifyStringOperation(filePath.Append(id.GetName(), id.GetNameCharCount()));
669685

670686
bool createdFile;
671-
int fileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(filePath, createIfNotExist, &createdFile);
687+
int fileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(filePath, createIfNotExist, id.IsSessionScope(), &createdFile);
672688
if (fileDescriptor == -1)
673689
{
674690
_ASSERTE(!createIfNotExist);
@@ -1117,17 +1133,23 @@ void SharedMemoryManager::AcquireCreationDeletionFileLock()
11171133
if (!SharedMemoryHelpers::EnsureDirectoryExists(
11181134
*gSharedFilesPath,
11191135
false /* isGlobalLockAcquired */,
1136+
false /* hasCurrentUserAccessOnly */,
1137+
true /* setStickyFlag */,
11201138
false /* createIfNotExist */,
11211139
true /* isSystemDirectory */))
11221140
{
11231141
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
11241142
}
11251143
SharedMemoryHelpers::EnsureDirectoryExists(
11261144
*s_runtimeTempDirectoryPath,
1127-
false /* isGlobalLockAcquired */);
1145+
false /* isGlobalLockAcquired */,
1146+
false /* hasCurrentUserAccessOnly */,
1147+
true /* setStickyFlag */);
11281148
SharedMemoryHelpers::EnsureDirectoryExists(
11291149
*s_sharedMemoryDirectoryPath,
1130-
false /* isGlobalLockAcquired */);
1150+
false /* isGlobalLockAcquired */,
1151+
false /* hasCurrentUserAccessOnly */,
1152+
true /* setStickyFlag */);
11311153
s_creationDeletionLockFileDescriptor = SharedMemoryHelpers::OpenDirectory(*s_sharedMemoryDirectoryPath);
11321154
if (s_creationDeletionLockFileDescriptor == -1)
11331155
{

src/pal/src/synchobj/mutex.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1186,7 +1186,7 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen(
11861186
SharedMemoryHelpers::BuildSharedFilesPath(lockFilePath, SHARED_MEMORY_LOCK_FILES_DIRECTORY_NAME);
11871187
if (created)
11881188
{
1189-
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */);
1189+
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */, false /* hasCurrentUserAccessOnly */, true /* setStickyFlag */);
11901190
}
11911191

11921192
// Create the session directory
@@ -1195,15 +1195,15 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen(
11951195
SharedMemoryHelpers::VerifyStringOperation(id->AppendSessionDirectoryName(lockFilePath));
11961196
if (created)
11971197
{
1198-
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */);
1198+
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */, id->IsSessionScope(), false /* setStickyFlag */);
11991199
autoCleanup.m_lockFilePath = &lockFilePath;
12001200
autoCleanup.m_sessionDirectoryPathCharCount = lockFilePath.GetCount();
12011201
}
12021202

12031203
// Create or open the lock file
12041204
SharedMemoryHelpers::VerifyStringOperation(lockFilePath.Append('/'));
12051205
SharedMemoryHelpers::VerifyStringOperation(lockFilePath.Append(id->GetName(), id->GetNameCharCount()));
1206-
int lockFileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(lockFilePath, created);
1206+
int lockFileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(lockFilePath, created, id->IsSessionScope());
12071207
if (lockFileDescriptor == -1)
12081208
{
12091209
_ASSERTE(!created);

0 commit comments

Comments
 (0)