Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[7.0] Fix session-local named mutex compat issue #90343

Merged
merged 2 commits into from
Sep 1, 2023
Merged

[7.0] Fix session-local named mutex compat issue #90343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c99e52
Revert "Merged PR 26484: Restrict named mutex files permissions"
kouvel Aug 10, 2023
00dc153
Restore a change to improve backward compatibility
kouvel Aug 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/coreclr/pal/src/include/pal/sharedmemory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,9 @@ class SharedMemoryException
class SharedMemoryHelpers
{
private:
static const mode_t PermissionsMask_CurrentUser_ReadWrite;
static const mode_t PermissionsMask_CurrentUser_ReadWriteExecute;
static const mode_t PermissionsMask_AllUsers_ReadWrite;
static const mode_t PermissionsMask_AllUsers_ReadWriteExecute;

public:
static const UINT32 InvalidProcessId;
static const SIZE_T InvalidThreadId;
Expand All @@ -108,12 +106,12 @@ class SharedMemoryHelpers
static void BuildSharedFilesPath(PathCharString& destination, const char *suffix, int suffixByteCount);
static bool AppendUInt32String(PathCharString& destination, UINT32 value);

static bool EnsureDirectoryExists(const char *path, bool isGlobalLockAcquired, bool hasCurrentUserAccessOnly, bool setStickyFlag, bool createIfNotExist = true, bool isSystemDirectory = false);
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:
static int OpenDirectory(LPCSTR path);
static int CreateOrOpenFile(LPCSTR path, bool createIfNotExist = true, bool isSessionScope = true, bool *createdRef = nullptr);
static int CreateOrOpenFile(LPCSTR path, bool createIfNotExist = true, bool *createdRef = nullptr);
static void CloseFile(int fileDescriptor);

static SIZE_T GetFileSize(int fileDescriptor);
Expand Down
41 changes: 12 additions & 29 deletions src/coreclr/pal/src/sharedmemory/sharedmemory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ DWORD SharedMemoryException::GetErrorCode() const
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
// SharedMemoryHelpers

const mode_t SharedMemoryHelpers::PermissionsMask_CurrentUser_ReadWrite = S_IRUSR | S_IWUSR;
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;
Expand Down Expand Up @@ -97,22 +96,13 @@ SIZE_T SharedMemoryHelpers::AlignUp(SIZE_T value, SIZE_T alignment)
bool SharedMemoryHelpers::EnsureDirectoryExists(
const char *path,
bool isGlobalLockAcquired,
bool hasCurrentUserAccessOnly,
bool setStickyFlag,
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());
_ASSERTE(!(setStickyFlag && hasCurrentUserAccessOnly)); // Sticky bit doesn't make sense with current user access only

mode_t mode = hasCurrentUserAccessOnly ? PermissionsMask_CurrentUser_ReadWriteExecute : PermissionsMask_AllUsers_ReadWriteExecute;
if (setStickyFlag)
{
mode |= S_ISVTX;
}

// Check if the path already exists
struct stat statInfo;
Expand All @@ -133,11 +123,11 @@ bool SharedMemoryHelpers::EnsureDirectoryExists(

if (isGlobalLockAcquired)
{
if (mkdir(path, mode) != 0)
if (mkdir(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
{
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
}
if (chmod(path, mode) != 0)
if (chmod(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
{
rmdir(path);
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
Expand All @@ -152,7 +142,7 @@ bool SharedMemoryHelpers::EnsureDirectoryExists(
{
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
}
if (chmod(tempPath, mode) != 0)
if (chmod(tempPath, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
{
rmdir(tempPath);
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
Expand Down Expand Up @@ -192,11 +182,11 @@ bool SharedMemoryHelpers::EnsureDirectoryExists(
// For non-system directories (such as gSharedFilesPath/SHARED_MEMORY_RUNTIME_TEMP_DIRECTORY_NAME),
// 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 & mode) == mode)
if ((statInfo.st_mode & PermissionsMask_AllUsers_ReadWriteExecute) == PermissionsMask_AllUsers_ReadWriteExecute)
{
return true;
}
if (!createIfNotExist || chmod(path, mode) != 0)
if (!createIfNotExist || chmod(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
{
// We were not asked to create the path or we weren't able to set the new permissions.
// As a last resort, check that at least the current user has full access.
Expand Down Expand Up @@ -253,7 +243,7 @@ int SharedMemoryHelpers::OpenDirectory(LPCSTR path)
return fileDescriptor;
}

int SharedMemoryHelpers::CreateOrOpenFile(LPCSTR path, bool createIfNotExist, bool isSessionScope, bool *createdRef)
int SharedMemoryHelpers::CreateOrOpenFile(LPCSTR path, bool createIfNotExist, bool *createdRef)
{
_ASSERTE(path != nullptr);
_ASSERTE(path[0] != '\0');
Expand Down Expand Up @@ -283,13 +273,12 @@ int SharedMemoryHelpers::CreateOrOpenFile(LPCSTR path, bool createIfNotExist, bo

// File does not exist, create the file
openFlags |= O_CREAT | O_EXCL;
mode_t mode = isSessionScope ? PermissionsMask_CurrentUser_ReadWrite : PermissionsMask_AllUsers_ReadWrite;
fileDescriptor = Open(path, openFlags, mode);
fileDescriptor = Open(path, openFlags, PermissionsMask_AllUsers_ReadWrite);
_ASSERTE(fileDescriptor != -1);

// The permissions mask passed to open() is filtered by the process' permissions umask, so open() may not set all of
// the requested permissions. Use chmod() to set the proper permissions.
if (chmod(path, mode) != 0)
if (chmod(path, PermissionsMask_AllUsers_ReadWrite) != 0)
{
CloseFile(fileDescriptor);
unlink(path);
Expand Down Expand Up @@ -675,7 +664,7 @@ SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen(
SharedMemoryHelpers::VerifyStringOperation(SharedMemoryManager::CopySharedMemoryBasePath(filePath));
SharedMemoryHelpers::VerifyStringOperation(filePath.Append('/'));
SharedMemoryHelpers::VerifyStringOperation(id.AppendSessionDirectoryName(filePath));
if (!SharedMemoryHelpers::EnsureDirectoryExists(filePath, true /* isGlobalLockAcquired */, id.IsSessionScope(), false /* setStickyFlag */, createIfNotExist))
if (!SharedMemoryHelpers::EnsureDirectoryExists(filePath, true /* isGlobalLockAcquired */, createIfNotExist))
{
_ASSERTE(!createIfNotExist);
return nullptr;
Expand All @@ -688,7 +677,7 @@ SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen(
SharedMemoryHelpers::VerifyStringOperation(filePath.Append(id.GetName(), id.GetNameCharCount()));

bool createdFile;
int fileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(filePath, createIfNotExist, id.IsSessionScope(), &createdFile);
int fileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(filePath, createIfNotExist, &createdFile);
if (fileDescriptor == -1)
{
_ASSERTE(!createIfNotExist);
Expand Down Expand Up @@ -1163,23 +1152,17 @@ void SharedMemoryManager::AcquireCreationDeletionFileLock()
if (!SharedMemoryHelpers::EnsureDirectoryExists(
*gSharedFilesPath,
false /* isGlobalLockAcquired */,
false /* hasCurrentUserAccessOnly */,
true /* setStickyFlag */,
false /* createIfNotExist */,
true /* isSystemDirectory */))
{
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
}
SharedMemoryHelpers::EnsureDirectoryExists(
*s_runtimeTempDirectoryPath,
false /* isGlobalLockAcquired */,
false /* hasCurrentUserAccessOnly */,
false /* setStickyFlag */);
false /* isGlobalLockAcquired */);
SharedMemoryHelpers::EnsureDirectoryExists(
*s_sharedMemoryDirectoryPath,
false /* isGlobalLockAcquired */,
false /* hasCurrentUserAccessOnly */,
true /* setStickyFlag */);
false /* isGlobalLockAcquired */);
s_creationDeletionLockFileDescriptor = SharedMemoryHelpers::OpenDirectory(*s_sharedMemoryDirectoryPath);
if (s_creationDeletionLockFileDescriptor == -1)
{
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/pal/src/synchobj/mutex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1117,7 +1117,7 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen(
SharedMemoryHelpers::BuildSharedFilesPath(lockFilePath, SHARED_MEMORY_LOCK_FILES_DIRECTORY_NAME);
if (created)
{
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */, false /* hasCurrentUserAccessOnly */, true /* setStickyFlag */);
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */);
}

// Create the session directory
Expand All @@ -1126,15 +1126,15 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen(
SharedMemoryHelpers::VerifyStringOperation(id->AppendSessionDirectoryName(lockFilePath));
if (created)
{
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */, id->IsSessionScope(), false /* setStickyFlag */);
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */);
autoCleanup.m_lockFilePath = &lockFilePath;
autoCleanup.m_sessionDirectoryPathCharCount = lockFilePath.GetCount();
}

// Create or open the lock file
SharedMemoryHelpers::VerifyStringOperation(lockFilePath.Append('/'));
SharedMemoryHelpers::VerifyStringOperation(lockFilePath.Append(id->GetName(), id->GetNameCharCount()));
int lockFileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(lockFilePath, created, id->IsSessionScope());
int lockFileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(lockFilePath, created);
if (lockFileDescriptor == -1)
{
_ASSERTE(!created);
Expand Down
Loading