From a84383cc047b50e07ad62a94ea11f9cf027c7bd8 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 25 Jan 2022 08:15:55 +0100 Subject: [PATCH] Make sure that shared memory object name meets the length requirements (#64099) Co-authored-by: Stephen Toub # Conflicts: # src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateNew.Tests.cs # src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewAccessor.Tests.cs # src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewStream.Tests.cs # src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewStreamConformanceTests.cs --- .../Native/Unix/System.Native/pal_io.c | 6 +++ .../MemoryMappedFile.Unix.cs | 52 +++++++++++++------ .../tests/MemoryMappedFile.CreateNew.Tests.cs | 1 - .../tests/MemoryMappedViewAccessor.Tests.cs | 1 - .../tests/MemoryMappedViewStream.Tests.cs | 1 - .../MemoryMappedViewStreamConformanceTests.cs | 1 - 6 files changed, 42 insertions(+), 20 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_io.c b/src/libraries/Native/Unix/System.Native/pal_io.c index 049e44e12fcef..3541a90f1858b 100644 --- a/src/libraries/Native/Unix/System.Native/pal_io.c +++ b/src/libraries/Native/Unix/System.Native/pal_io.c @@ -324,6 +324,12 @@ int32_t SystemNative_Unlink(const char* path) intptr_t SystemNative_ShmOpen(const char* name, int32_t flags, int32_t mode) { +#if defined(SHM_NAME_MAX) // macOS + assert(strlen(name) <= SHM_NAME_MAX); +#elif defined(PATH_MAX) // other Unixes + assert(strlen(name) <= PATH_MAX); +#endif + #if HAVE_SHM_OPEN_THAT_WORKS_WELL_ENOUGH_WITH_MMAP flags = ConvertOpenFlags(flags); if (flags == -1) diff --git a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs index 389ab072d9a6c..6882527cecc8d 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics; using Microsoft.Win32.SafeHandles; namespace System.IO.MemoryMappedFiles @@ -173,9 +174,6 @@ private static FileStream CreateSharedBackingObject(Interop.Sys.MemoryMappedProt private static FileStream? CreateSharedBackingObjectUsingMemory( Interop.Sys.MemoryMappedProtections protections, long capacity, HandleInheritability inheritability) { - // The POSIX shared memory object name must begin with '/'. After that we just want something short and unique. - string mapName = string.Create(null, stackalloc char[128], $"/corefx_map_{Guid.NewGuid():N}"); - // Determine the flags to use when creating the shared memory object Interop.Sys.OpenFlags flags = (protections & Interop.Sys.MemoryMappedProtections.PROT_WRITE) != 0 ? Interop.Sys.OpenFlags.O_RDWR : @@ -191,22 +189,32 @@ private static FileStream CreateSharedBackingObject(Interop.Sys.MemoryMappedProt if ((protections & Interop.Sys.MemoryMappedProtections.PROT_EXEC) != 0) perms |= Interop.Sys.Permissions.S_IXUSR; - // Create the shared memory object. - SafeFileHandle fd = Interop.Sys.ShmOpen(mapName, flags, (int)perms); - if (fd.IsInvalid) + string mapName; + SafeFileHandle fd; + + do { - Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - if (errorInfo.Error == Interop.Error.ENOTSUP) + mapName = GenerateMapName(); + fd = Interop.Sys.ShmOpen(mapName, flags, (int)perms); // Create the shared memory object. + + if (fd.IsInvalid) { - // If ShmOpen is not supported, fall back to file backing object. - // Note that the System.Native shim will force this failure on platforms where - // the result of native shm_open does not work well with our subsequent call - // to mmap. - return null; - } + Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); + fd.Dispose(); - throw Interop.GetExceptionForIoErrno(errorInfo); - } + if (errorInfo.Error == Interop.Error.ENOTSUP) + { + // If ShmOpen is not supported, fall back to file backing object. + // Note that the System.Native shim will force this failure on platforms where + // the result of native shm_open does not work well with our subsequent call to mmap. + return null; + } + else if (errorInfo.Error != Interop.Error.EEXIST) // map with same name already existed + { + throw Interop.GetExceptionForIoErrno(errorInfo); + } + } + } while (fd.IsInvalid); try { @@ -236,6 +244,18 @@ private static FileStream CreateSharedBackingObject(Interop.Sys.MemoryMappedProt fd.Dispose(); throw; } + + static string GenerateMapName() + { + const int MaxSharedMemoryObjectNameLength = 32; // SHM_NAME_MAX on OSX ARM64, on other systems it's equal PATH_MAX (250) + // The POSIX shared memory object name must begin with '/'. After that we just want something short (32) and unique. + return string.Create(MaxSharedMemoryObjectNameLength, 0, (span, state) => + { + Guid.NewGuid().TryFormat(span, out int charsWritten, "N"); + Debug.Assert(charsWritten == MaxSharedMemoryObjectNameLength); + "/dotnet_".CopyTo(span); + }); + } } private static FileStream CreateSharedBackingObjectUsingFile(Interop.Sys.MemoryMappedProtections protections, long capacity, HandleInheritability inheritability) diff --git a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateNew.Tests.cs b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateNew.Tests.cs index e7c5ee2f80c3a..70b82398301d5 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateNew.Tests.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateNew.Tests.cs @@ -11,7 +11,6 @@ namespace System.IO.MemoryMappedFiles.Tests /// /// Tests for MemoryMappedFile.CreateNew. /// - [ActiveIssue("https://github.com/dotnet/runtime/issues/49104", typeof(PlatformDetection), nameof(PlatformDetection.IsMacOsAppleSilicon))] public class MemoryMappedFileTests_CreateNew : MemoryMappedFilesTestBase { /// diff --git a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewAccessor.Tests.cs b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewAccessor.Tests.cs index 0c8517ca4affe..8fe35396ae804 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewAccessor.Tests.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewAccessor.Tests.cs @@ -11,7 +11,6 @@ namespace System.IO.MemoryMappedFiles.Tests /// /// Tests for MemoryMappedViewAccessor. /// - [ActiveIssue("https://github.com/dotnet/runtime/issues/49104", typeof(PlatformDetection), nameof(PlatformDetection.IsMacOsAppleSilicon))] public class MemoryMappedViewAccessorTests : MemoryMappedFilesTestBase { /// diff --git a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewStream.Tests.cs b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewStream.Tests.cs index a5340527df901..f279b8c33f98f 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewStream.Tests.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewStream.Tests.cs @@ -11,7 +11,6 @@ namespace System.IO.MemoryMappedFiles.Tests /// /// Tests for MemoryMappedViewStream. /// - [ActiveIssue("https://github.com/dotnet/runtime/issues/49104", typeof(PlatformDetection), nameof(PlatformDetection.IsMacOsAppleSilicon))] public class MemoryMappedViewStreamTests : MemoryMappedFilesTestBase { /// diff --git a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewStreamConformanceTests.cs b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewStreamConformanceTests.cs index e84a86a91f20c..c190d329a00fe 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewStreamConformanceTests.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedViewStreamConformanceTests.cs @@ -45,7 +45,6 @@ private Task CreateStream(byte[] initialData, FileAccess access) } } - [ActiveIssue("https://github.com/dotnet/runtime/issues/49104", typeof(PlatformDetection), nameof(PlatformDetection.IsMacOsAppleSilicon))] public class AnonymousMemoryMappedViewStreamConformanceTests : MemoryMappedViewStreamConformanceTests { protected override MemoryMappedFile CreateFile(int length) =>