diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/SharedMemoryManager.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/SharedMemoryManager.Unix.cs index 6ce05fdd8c467b..86da8f90c7374f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/SharedMemoryManager.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/SharedMemoryManager.Unix.cs @@ -384,7 +384,7 @@ internal static class SharedMemoryHelpers private const UnixFileMode PermissionsMask_AllUsers_ReadWriteExecute = PermissionsMask_AllUsers_ReadWrite | UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; private const UnixFileMode PermissionsMask_Sticky = UnixFileMode.StickyBit; - private const string SharedMemoryUniqueTempNameTemplate = ".dotnet.XXXXXX"; + private const int TransientRetryCount = 10; // See https://developer.apple.com/documentation/Foundation/FileManager/containerURL(forSecurityApplicationGroupIdentifier:)#App-Groups-in-macOS for details on this path. private const string ApplicationContainerBasePathSuffix = "/Library/Group Containers/"; @@ -419,7 +419,6 @@ private static string InitalizeSharedFilesPath() internal static SafeFileHandle CreateOrOpenFile(string sharedMemoryFilePath, SharedMemoryId id, bool createIfNotExist, out bool createdFile) { - const int TransientRetryCount = 10; int transientRetryCount = 0; while (true) @@ -527,159 +526,181 @@ internal static SafeFileHandle CreateOrOpenFile(string sharedMemoryFilePath, Sha internal static bool EnsureDirectoryExists(string directoryPath, SharedMemoryId id, bool isGlobalLockAcquired, bool createIfNotExist = true, bool isSystemDirectory = false) { - UnixFileMode permissionsMask = id.IsUserScope - ? PermissionsMask_OwnerUser_ReadWriteExecute - : PermissionsMask_AllUsers_ReadWriteExecute; + int transientRetryCount = 0; + while (true) + { + UnixFileMode permissionsMask = id.IsUserScope + ? PermissionsMask_OwnerUser_ReadWriteExecute + : PermissionsMask_AllUsers_ReadWriteExecute; - int statResult = Interop.Sys.Stat(directoryPath, out Interop.Sys.FileStatus fileStatus); + int statResult = Interop.Sys.Stat(directoryPath, out Interop.Sys.FileStatus fileStatus); - if (statResult != 0 && Interop.Sys.GetLastError() == Interop.Error.ENOENT) - { - if (!createIfNotExist) + if (statResult != 0 && Interop.Sys.GetLastError() == Interop.Error.ENOENT) { - // The directory does not exist and we are not allowed to create it. - return false; - } + if (!createIfNotExist) + { + // The directory does not exist and we are not allowed to create it. + return false; + } - // The path does not exist, create the directory. The permissions mask passed to mkdir() is filtered by the process' - // permissions umask, so mkdir() may not set all of the requested permissions. We need to use chmod() to set the proper - // permissions. That creates a race when there is no global lock acquired when creating the directory. Another user's - // process may create the directory and this user's process may try to use it before the other process sets the full - // permissions. In that case, create a temporary directory first, set the permissions, and rename it to the actual - // directory name. + // The path does not exist, create the directory. The permissions mask passed to mkdir() is filtered by the process' + // permissions umask, so mkdir() may not set all of the requested permissions. We need to use chmod() to set the proper + // permissions. That creates a race when there is no global lock acquired when creating the directory. Another user's + // process may create the directory and this user's process may try to use it before the other process sets the full + // permissions. The validation below tolerates that transient state while using mkdir() on the final path avoids + // rename() replacing an existing empty directory that another process may already be locking. - if (isGlobalLockAcquired) - { + if (isGlobalLockAcquired) + { #pragma warning disable CA1416 // Validate platform compatibility. This file is only included on Unix platforms. - Directory.CreateDirectory(directoryPath, permissionsMask); + Directory.CreateDirectory(directoryPath, permissionsMask); #pragma warning restore CA1416 // Validate platform compatibility - try + try + { + FileSystem.SetUnixFileMode(directoryPath, permissionsMask); + } + catch (Exception) + { + Directory.Delete(directoryPath); + throw; + } + + return true; + } + + if (Interop.Sys.MkDir(directoryPath, (int)permissionsMask) == 0) { - FileSystem.SetUnixFileMode(directoryPath, permissionsMask); + try + { + FileSystem.SetUnixFileMode(directoryPath, permissionsMask); + } + catch (Exception) + { + Directory.Delete(directoryPath); + throw; + } + + return true; } - catch (Exception) + + Interop.ErrorInfo mkdirError = Interop.Sys.GetLastErrorInfo(); + if (mkdirError.Error != Interop.Error.EEXIST) { - Directory.Delete(directoryPath); - throw; + throw Interop.GetExceptionForIoErrno(mkdirError, directoryPath); } - return true; + statResult = Interop.Sys.Stat(directoryPath, out fileStatus); } - string tempPath = Path.Combine(SharedFilesPath, SharedMemoryUniqueTempNameTemplate); - - unsafe + if (statResult != 0) { - byte* tempPathPtr = Utf8StringMarshaller.ConvertToUnmanaged(tempPath); - if (Interop.Sys.MkdTemp(tempPathPtr) == null) + Interop.ErrorInfo error = Interop.Sys.GetLastErrorInfo(); + if (error.Error == Interop.Error.ENOENT && RetryOnTransientPermissionFailure()) { - Utf8StringMarshaller.Free(tempPathPtr); - Interop.ErrorInfo error = Interop.Sys.GetLastErrorInfo(); - throw Interop.GetExceptionForIoErrno(error, tempPath); + continue; } - // Convert the path back to get the substituted path. - tempPath = Utf8StringMarshaller.ConvertToManaged(tempPathPtr)!; - Utf8StringMarshaller.Free(tempPathPtr); - } - try - { - FileSystem.SetUnixFileMode(tempPath, permissionsMask); + throw Interop.GetExceptionForIoErrno(error, directoryPath); } - catch (Exception) + + // If the path exists, check that it's a directory + if ((fileStatus.Mode & Interop.Sys.FileTypes.S_IFDIR) == 0) { - Directory.Delete(tempPath); - throw; + throw new IOException(SR.Format(SR.IO_SharedMemory_PathExistsButNotDirectory, directoryPath)); } - if (Interop.Sys.Rename(tempPath, directoryPath) == 0) + if (isSystemDirectory) { - return true; + // For system directories (such as TEMP_DIRECTORY_PATH), require sufficient permissions only for the + // owner 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 the directory is being used for user-scoped shared memory data, also ensure that either it has the sticky bit or + // it's owned by the current user and without write access for other users. + + permissionsMask = PermissionsMask_OwnerUser_ReadWriteExecute; + if ((fileStatus.Mode & (int)permissionsMask) == (int)permissionsMask + && ( + !id.IsUserScope || + (fileStatus.Mode & (int)PermissionsMask_Sticky) == (int)PermissionsMask_Sticky || + (fileStatus.Uid == id.Uid && (fileStatus.Mode & (int)PermissionsMask_NonOwnerUsers_Write) == 0) + )) + { + return true; + } + + if (RetryOnTransientPermissionFailure()) + { + continue; + } + + throw new IOException(SR.Format(SR.IO_SharedMemory_DirectoryPermissionsIncorrect, directoryPath, fileStatus.Uid, Convert.ToString(fileStatus.Mode, 8))); } - // Another process may have beaten us to it. Delete the temp directory and continue to check the requested directory to - // see if it meets our needs. - Directory.Delete(tempPath); - statResult = Interop.Sys.Stat(directoryPath, out fileStatus); - } + // For non-system directories (such as SharedFilesPath/UserUnscopedRuntimeTempDirectoryName), + // require the sufficient permissions and try to update them if requested to create the directory, so that + // shared memory files may be shared according to its scope. - // If the path exists, check that it's a directory - if (statResult != 0 || (fileStatus.Mode & Interop.Sys.FileTypes.S_IFDIR) == 0) - { - if (statResult != 0) + // For user-scoped directories, verify the owner UID + if (id.IsUserScope && fileStatus.Uid != id.Uid) { - Interop.ErrorInfo error = Interop.Sys.GetLastErrorInfo(); - if (error.Error != Interop.Error.ENOENT) + if (RetryOnTransientPermissionFailure()) { - throw Interop.GetExceptionForIoErrno(error, directoryPath); + continue; } + + throw new IOException(SR.Format(SR.IO_SharedMemory_DirectoryNotOwnedByUid, directoryPath, id.Uid)); } - else + + // Verify the permissions, or try to change them if possible + if ((fileStatus.Mode & (int)PermissionsMask_AllUsers_ReadWriteExecute) == (int)permissionsMask + || (createIfNotExist && Interop.Sys.ChMod(directoryPath, (int)permissionsMask) == 0)) { - throw new IOException(SR.Format(SR.IO_SharedMemory_PathExistsButNotDirectory, directoryPath)); + return true; } - } - if (isSystemDirectory) - { - // For system directories (such as TEMP_DIRECTORY_PATH), require sufficient permissions only for the - // owner 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 the directory is being used for user-scoped shared memory data, also ensure that either it has the sticky bit or - // it's owned by the current user and without write access for other users. + // We were not able to verify or set the necessary permissions. For user-scoped directories, this is treated as a failure + // since other users aren't sufficiently restricted in permissions. + if (id.IsUserScope) + { + if (RetryOnTransientPermissionFailure()) + { + continue; + } + + throw new IOException(SR.Format(SR.IO_SharedMemory_DirectoryPermissionsIncorrectUserScope, directoryPath, Convert.ToString(fileStatus.Mode, 8))); + } + // For user-unscoped directories, as a last resort, check that at least the owner user has full access. permissionsMask = PermissionsMask_OwnerUser_ReadWriteExecute; - if ((fileStatus.Mode & (int)permissionsMask) == (int)permissionsMask - && ( - !id.IsUserScope || - (fileStatus.Mode & (int)PermissionsMask_Sticky) == (int)PermissionsMask_Sticky || - (fileStatus.Uid == id.Uid && (fileStatus.Mode & (int)PermissionsMask_NonOwnerUsers_Write) == 0) - )) + if ((fileStatus.Mode & (int)permissionsMask) == (int)permissionsMask) { return true; } - throw new IOException(SR.Format(SR.IO_SharedMemory_DirectoryPermissionsIncorrect, directoryPath, fileStatus.Uid, Convert.ToString(fileStatus.Mode, 8))); - } - - // For non-system directories (such as SharedFilesPath/UserUnscopedRuntimeTempDirectoryName), - // require the sufficient permissions and try to update them if requested to create the directory, so that - // shared memory files may be shared according to its scope. - - // For user-scoped directories, verify the owner UID - if (id.IsUserScope && fileStatus.Uid != id.Uid) - { - throw new IOException(SR.Format(SR.IO_SharedMemory_DirectoryNotOwnedByUid, directoryPath, id.Uid)); - } + if (RetryOnTransientPermissionFailure()) + { + continue; + } - // Verify the permissions, or try to change them if possible - if ((fileStatus.Mode & (int)PermissionsMask_AllUsers_ReadWriteExecute) == (int)permissionsMask - || (createIfNotExist && Interop.Sys.ChMod(directoryPath, (int)permissionsMask) == 0)) - { - return true; + throw new IOException(SR.Format(SR.IO_SharedMemory_DirectoryOwnerPermissionsIncorrect, directoryPath, Convert.ToString(fileStatus.Mode, 8))); } - // We were not able to verify or set the necessary permissions. For user-scoped directories, this is treated as a failure - // since other users aren't sufficiently restricted in permissions. - if (id.IsUserScope) + bool RetryOnTransientPermissionFailure() { - throw new IOException(SR.Format(SR.IO_SharedMemory_DirectoryPermissionsIncorrectUserScope, directoryPath, Convert.ToString(fileStatus.Mode, 8))); - } - + if (transientRetryCount >= TransientRetryCount) + { + return false; + } - // For user-unscoped directories, as a last resort, check that at least the owner user has full access. - permissionsMask = PermissionsMask_OwnerUser_ReadWriteExecute; - if ((fileStatus.Mode & (int)permissionsMask) != (int)permissionsMask) - { - throw new IOException(SR.Format(SR.IO_SharedMemory_DirectoryOwnerPermissionsIncorrect, directoryPath, Convert.ToString(fileStatus.Mode, 8))); + transientRetryCount++; + Thread.Sleep(1); + return true; } - - return true; } internal static MemoryMappedFileHolder MemoryMapFile(SafeFileHandle fileHandle, nuint sharedDataTotalByteCount)