Skip to content

Commit

Permalink
Revert "[6.0] Fix session-local named mutex compat issue (#90344)"
Browse files Browse the repository at this point in the history
This reverts commit 17a9224.
  • Loading branch information
mmitche committed Oct 18, 2023
1 parent 0e36ec9 commit 8915010
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 17 deletions.
6 changes: 4 additions & 2 deletions src/coreclr/pal/src/include/pal/sharedmemory.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,11 @@ 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 @@ -109,12 +111,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 createIfNotExist = true, bool isSystemDirectory = false);
static bool EnsureDirectoryExists(const char *path, bool isGlobalLockAcquired, bool hasCurrentUserAccessOnly, bool setStickyFlag, 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 *createdRef = nullptr);
static int CreateOrOpenFile(LPCSTR path, bool createIfNotExist = true, bool isSessionScope = true, bool *createdRef = nullptr);
static void CloseFile(int fileDescriptor);

static SIZE_T GetFileSize(int fileDescriptor);
Expand Down
41 changes: 29 additions & 12 deletions src/coreclr/pal/src/sharedmemory/sharedmemory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ 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 @@ -96,13 +97,22 @@ 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 @@ -123,11 +133,11 @@ bool SharedMemoryHelpers::EnsureDirectoryExists(

if (isGlobalLockAcquired)
{
if (mkdir(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
if (mkdir(path, mode) != 0)
{
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
}
if (chmod(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
if (chmod(path, mode) != 0)
{
rmdir(path);
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
Expand All @@ -142,7 +152,7 @@ bool SharedMemoryHelpers::EnsureDirectoryExists(
{
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
}
if (chmod(tempPath, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
if (chmod(tempPath, mode) != 0)
{
rmdir(tempPath);
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
Expand Down Expand Up @@ -182,11 +192,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 & PermissionsMask_AllUsers_ReadWriteExecute) == PermissionsMask_AllUsers_ReadWriteExecute)
if ((statInfo.st_mode & mode) == mode)
{
return true;
}
if (!createIfNotExist || chmod(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
if (!createIfNotExist || chmod(path, mode) != 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 @@ -243,7 +253,7 @@ int SharedMemoryHelpers::OpenDirectory(LPCSTR path)
return fileDescriptor;
}

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

// File does not exist, create the file
openFlags |= O_CREAT | O_EXCL;
fileDescriptor = Open(path, openFlags, PermissionsMask_AllUsers_ReadWrite);
mode_t mode = isSessionScope ? PermissionsMask_CurrentUser_ReadWrite : PermissionsMask_AllUsers_ReadWrite;
fileDescriptor = Open(path, openFlags, mode);
_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, PermissionsMask_AllUsers_ReadWrite) != 0)
if (chmod(path, mode) != 0)
{
CloseFile(fileDescriptor);
unlink(path);
Expand Down Expand Up @@ -664,7 +675,7 @@ SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen(
SharedMemoryHelpers::VerifyStringOperation(SharedMemoryManager::CopySharedMemoryBasePath(filePath));
SharedMemoryHelpers::VerifyStringOperation(filePath.Append('/'));
SharedMemoryHelpers::VerifyStringOperation(id.AppendSessionDirectoryName(filePath));
if (!SharedMemoryHelpers::EnsureDirectoryExists(filePath, true /* isGlobalLockAcquired */, createIfNotExist))
if (!SharedMemoryHelpers::EnsureDirectoryExists(filePath, true /* isGlobalLockAcquired */, id.IsSessionScope(), false /* setStickyFlag */, createIfNotExist))
{
_ASSERTE(!createIfNotExist);
return nullptr;
Expand All @@ -677,7 +688,7 @@ SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen(
SharedMemoryHelpers::VerifyStringOperation(filePath.Append(id.GetName(), id.GetNameCharCount()));

bool createdFile;
int fileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(filePath, createIfNotExist, &createdFile);
int fileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(filePath, createIfNotExist, id.IsSessionScope(), &createdFile);
if (fileDescriptor == -1)
{
_ASSERTE(!createIfNotExist);
Expand Down Expand Up @@ -1152,17 +1163,23 @@ 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 /* isGlobalLockAcquired */,
false /* hasCurrentUserAccessOnly */,
true /* setStickyFlag */);
SharedMemoryHelpers::EnsureDirectoryExists(
*s_sharedMemoryDirectoryPath,
false /* isGlobalLockAcquired */);
false /* isGlobalLockAcquired */,
false /* hasCurrentUserAccessOnly */,
true /* setStickyFlag */);
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
Original file line number Diff line number Diff line change
Expand Up @@ -1121,7 +1121,7 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen(
SharedMemoryHelpers::BuildSharedFilesPath(lockFilePath, SHARED_MEMORY_LOCK_FILES_DIRECTORY_NAME);
if (created)
{
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */);
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */, false /* hasCurrentUserAccessOnly */, true /* setStickyFlag */);
}

// Create the session directory
Expand All @@ -1130,15 +1130,15 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen(
SharedMemoryHelpers::VerifyStringOperation(id->AppendSessionDirectoryName(lockFilePath));
if (created)
{
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */);
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */, id->IsSessionScope(), false /* setStickyFlag */);
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);
int lockFileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(lockFilePath, created, id->IsSessionScope());
if (lockFileDescriptor == -1)
{
_ASSERTE(!created);
Expand Down

0 comments on commit 8915010

Please sign in to comment.