From 15e49578925525d4ba3057b3c4dee58733333add Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Sat, 20 Jul 2024 22:54:11 +0300 Subject: [PATCH 01/13] Use memfd_create when available --- .../Unix/System.Native/Interop.MemfdCreate.cs | 34 +++++++++++++++ .../src/System.IO.MemoryMappedFiles.csproj | 4 ++ .../MemoryMappedFile.Unix.cs | 22 +++++++--- .../src/System/Environment.SunOS.cs | 2 +- src/native/libs/Common/pal_config.h.in | 1 + src/native/libs/System.Native/entrypoints.c | 2 + src/native/libs/System.Native/pal_io.c | 42 +++++++++++++++++++ src/native/libs/System.Native/pal_io.h | 14 +++++++ src/native/libs/configure.cmake | 4 ++ 9 files changed, 118 insertions(+), 7 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Unix/System.Native/Interop.MemfdCreate.cs diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MemfdCreate.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MemfdCreate.cs new file mode 100644 index 00000000000000..f323677e5788ee --- /dev/null +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MemfdCreate.cs @@ -0,0 +1,34 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; +using System.Threading; +using Microsoft.Win32.SafeHandles; + +internal static partial class Interop +{ + internal static partial class Sys + { + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_MemfdCreate", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] + internal static partial SafeFileHandle MemfdCreate(string name); + + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_MemfdSupported", SetLastError = true)] + private static partial int MemfdSupportedImpl(); + + private static volatile sbyte s_memfdSupported; + + internal static bool MemfdSupported + { + get + { + sbyte memfdSupported = s_memfdSupported; + if (memfdSupported == 0) + { + Interlocked.CompareExchange(ref s_memfdSupported, (sbyte)(MemfdSupportedImpl() == 1 ? 1 : -1), 0); + memfdSupported = s_memfdSupported; + } + return memfdSupported > 0; + } + } + } +} diff --git a/src/libraries/System.IO.MemoryMappedFiles/src/System.IO.MemoryMappedFiles.csproj b/src/libraries/System.IO.MemoryMappedFiles/src/System.IO.MemoryMappedFiles.csproj index 8c7cd9cfdda07b..d024cdd7d73110 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/src/System.IO.MemoryMappedFiles.csproj +++ b/src/libraries/System.IO.MemoryMappedFiles/src/System.IO.MemoryMappedFiles.csproj @@ -91,6 +91,8 @@ Link="Common\Interop\Unix\Interop.Libraries.cs" /> + + 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 2c49129705d1f9..c4c13e3703fd25 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 @@ -190,7 +190,14 @@ private static SafeFileHandle CreateSharedBackingObject(Interop.Sys.MemoryMapped do { mapName = GenerateMapName(); - fd = Interop.Sys.ShmOpen(mapName, flags, (int)perms); // Create the shared memory object. + if (Interop.Sys.MemfdSupported) + { + fd = Interop.Sys.MemfdCreate(mapName); + } + else + { + fd = Interop.Sys.ShmOpen(mapName, flags, (int)perms); // Create the shared memory object. + } if (fd.IsInvalid) { @@ -204,7 +211,7 @@ private static SafeFileHandle CreateSharedBackingObject(Interop.Sys.MemoryMapped // the result of native shm_open does not work well with our subsequent call to mmap. return null; } - else if (errorInfo.Error == Interop.Error.ENAMETOOLONG) + else if (!Interop.Sys.MemfdSupported && errorInfo.Error == Interop.Error.ENAMETOOLONG) { Debug.Fail($"shm_open failed with ENAMETOOLONG for {Encoding.UTF8.GetByteCount(mapName)} byte long name."); // in theory it should not happen anymore, but just to be extra safe we use the fallback @@ -219,10 +226,13 @@ private static SafeFileHandle CreateSharedBackingObject(Interop.Sys.MemoryMapped try { - // Unlink the shared memory object immediately so that it'll go away once all handles - // to it are closed (as with opened then unlinked files, it'll remain usable via - // the open handles even though it's unlinked and can't be opened anew via its name). - Interop.CheckIo(Interop.Sys.ShmUnlink(mapName)); + if (!Interop.Sys.MemfdSupported) + { + // Unlink the shared memory object immediately so that it'll go away once all handles + // to it are closed (as with opened then unlinked files, it'll remain usable via + // the open handles even though it's unlinked and can't be opened anew via its name). + Interop.CheckIo(Interop.Sys.ShmUnlink(mapName)); + } // Give it the right capacity. We do this directly with ftruncate rather // than via FileStream.SetLength after the FileStream is created because, on some systems, diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.SunOS.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.SunOS.cs index 84199a3d306d28..fb7ff75cb2e6c6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.SunOS.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.SunOS.cs @@ -8,6 +8,6 @@ namespace System public static partial class Environment { public static long WorkingSet => - (long)(Interop.procfs.TryReadProcessStatusInfo(Interop.procfs.ProcPid.Self, out Interop.procfs.ProcessStatusInfo status) ? status.ResidentSetSize : 0); + (long)(Interop.procfs.TryReadProcessStatusInfo(ProcessId, out Interop.procfs.ProcessStatusInfo status) ? status.ResidentSetSize : 0); } } diff --git a/src/native/libs/Common/pal_config.h.in b/src/native/libs/Common/pal_config.h.in index 0960f6aa971c8e..211bd2949ae42f 100644 --- a/src/native/libs/Common/pal_config.h.in +++ b/src/native/libs/Common/pal_config.h.in @@ -11,6 +11,7 @@ #cmakedefine01 HAVE_F_DUPFD #cmakedefine01 HAVE_F_FULLFSYNC #cmakedefine01 HAVE_O_CLOEXEC +#cmakedefine01 HAVE_MEMFD_CREATE #cmakedefine01 HAVE_GETIFADDRS #cmakedefine01 HAVE_UTSNAME_DOMAINNAME #cmakedefine01 HAVE_STAT64 diff --git a/src/native/libs/System.Native/entrypoints.c b/src/native/libs/System.Native/entrypoints.c index 51c761109159b5..d9f9b4d537b0ad 100644 --- a/src/native/libs/System.Native/entrypoints.c +++ b/src/native/libs/System.Native/entrypoints.c @@ -62,6 +62,8 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_Close) DllImportEntry(SystemNative_Dup) DllImportEntry(SystemNative_Unlink) + DllImportEntry(SystemNative_MemfdSupported) + DllImportEntry(SystemNative_MemfdCreate) DllImportEntry(SystemNative_ShmOpen) DllImportEntry(SystemNative_ShmUnlink) DllImportEntry(SystemNative_GetReadDirRBufferSize) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 4051656d35ac07..8a0b85d79f844a 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -369,6 +369,48 @@ int32_t SystemNative_Unlink(const char* path) return result; } +int32_t SystemNative_MemfdSupported(void) +{ +#if HAVE_MEMFD_CREATE +#ifdef TARGET_LINUX + struct utsname uts; + int32_t major, minor; + + // memfd_create is only known to work properly on kernel version > 3.17 and throws SIGSEGV instead of ENOTSUP + if (uname(&uts) == 0 && sscanf(uts.release, "%d.%d", &major, &minor) == 2 && (major < 3 || (major == 3 && minor < 17))) + { + return 0; + } +#endif + + int32_t fd = memfd_create("test", MFD_CLOEXEC | MFD_ALLOW_SEALING); + if (fd < 0) return 0; + + close(fd); + return 1; +#else + errno = ENOTSUP; + return 0; +#endif +} + +intptr_t SystemNative_MemfdCreate(const char* name) +{ +#if HAVE_MEMFD_CREATE +#if defined(SHM_NAME_MAX) // macOS + assert(strlen(name) <= SHM_NAME_MAX); +#elif defined(PATH_MAX) // other Unixes + assert(strlen(name) <= PATH_MAX); +#endif + + return memfd_create(name, MFD_CLOEXEC | MFD_ALLOW_SEALING); +#else + (void)name; + errno = ENOTSUP; + return -1; +#endif +} + intptr_t SystemNative_ShmOpen(const char* name, int32_t flags, int32_t mode) { #if defined(SHM_NAME_MAX) // macOS diff --git a/src/native/libs/System.Native/pal_io.h b/src/native/libs/System.Native/pal_io.h index 03fd94cea25417..a5a8261e569372 100644 --- a/src/native/libs/System.Native/pal_io.h +++ b/src/native/libs/System.Native/pal_io.h @@ -369,6 +369,20 @@ PALEXPORT intptr_t SystemNative_Dup(intptr_t oldfd); */ PALEXPORT int32_t SystemNative_Unlink(const char* path); +/** + * Check if the system supports memfd_create(2). + * + * Returns 1 if memfd_create is supported, 0 if not supported, or -1 on failure. Sets errno on failure. + */ +PALEXPORT int32_t SystemNative_MemfdSupported(void); + +/** + * Create an anonymous file descriptor. Implemented as shim to memfd_create(2). + * + * Returns file descriptor or -1 on failure. Sets errno on failure. + */ +PALEXPORT intptr_t SystemNative_MemfdCreate(const char* name); + /** * Open or create a shared memory object. Implemented as shim to shm_open(3). * diff --git a/src/native/libs/configure.cmake b/src/native/libs/configure.cmake index 871e5bd2ea8099..07f7918406d920 100644 --- a/src/native/libs/configure.cmake +++ b/src/native/libs/configure.cmake @@ -137,6 +137,10 @@ check_symbol_exists( fcntl.h HAVE_F_FULLFSYNC) +check_function_exists( + memfd_create + HAVE_MEMFD_CREATE) + check_function_exists( getifaddrs HAVE_GETIFADDRS) From b946a1566e34a8d20afee6ca51f4b5a0f638f696 Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Mon, 22 Jul 2024 01:44:38 +0300 Subject: [PATCH 02/13] Remove unnecessary sealing flag --- src/native/libs/System.Native/pal_io.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 8a0b85d79f844a..f1181f66986607 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -383,7 +383,7 @@ int32_t SystemNative_MemfdSupported(void) } #endif - int32_t fd = memfd_create("test", MFD_CLOEXEC | MFD_ALLOW_SEALING); + int32_t fd = memfd_create("test", MFD_CLOEXEC); if (fd < 0) return 0; close(fd); @@ -403,7 +403,7 @@ intptr_t SystemNative_MemfdCreate(const char* name) assert(strlen(name) <= PATH_MAX); #endif - return memfd_create(name, MFD_CLOEXEC | MFD_ALLOW_SEALING); + return memfd_create(name, MFD_CLOEXEC); #else (void)name; errno = ENOTSUP; From 78da311e9e9ac4fbb1b3a590c9b5671de8096f35 Mon Sep 17 00:00:00 2001 From: Adeel Mujahid <3840695+am11@users.noreply.github.com> Date: Mon, 22 Jul 2024 11:56:13 +0300 Subject: [PATCH 03/13] Apply suggestions from code review Co-authored-by: Adam Sitnik --- .../Unix/System.Native/Interop.MemfdCreate.cs | 2 +- .../MemoryMappedFile.Unix.cs | 71 ++++++++++--------- src/native/libs/System.Native/pal_io.c | 2 + 3 files changed, 42 insertions(+), 33 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MemfdCreate.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MemfdCreate.cs index f323677e5788ee..ad05741923197b 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MemfdCreate.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MemfdCreate.cs @@ -17,7 +17,7 @@ internal static partial class Sys private static volatile sbyte s_memfdSupported; - internal static bool MemfdSupported + internal static bool IsMemfdSupported { get { 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 c4c13e3703fd25..ac4ce03634af62 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 @@ -169,33 +169,33 @@ private static SafeFileHandle CreateSharedBackingObject(Interop.Sys.MemoryMapped private static SafeFileHandle? CreateSharedBackingObjectUsingMemory( Interop.Sys.MemoryMappedProtections protections, long capacity, HandleInheritability inheritability) { - // 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 : - Interop.Sys.OpenFlags.O_RDONLY; - flags |= Interop.Sys.OpenFlags.O_CREAT | Interop.Sys.OpenFlags.O_EXCL; // CreateNew - - // Determine the permissions with which to create the file - var perms = UnixFileMode.None; - if ((protections & Interop.Sys.MemoryMappedProtections.PROT_READ) != 0) - perms |= UnixFileMode.UserRead; - if ((protections & Interop.Sys.MemoryMappedProtections.PROT_WRITE) != 0) - perms |= UnixFileMode.UserWrite; - if ((protections & Interop.Sys.MemoryMappedProtections.PROT_EXEC) != 0) - perms |= UnixFileMode.UserExecute; - string mapName; SafeFileHandle fd; do { mapName = GenerateMapName(); - if (Interop.Sys.MemfdSupported) + if (Interop.Sys.IsMemfdSupported) { fd = Interop.Sys.MemfdCreate(mapName); } else { + // 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 : + Interop.Sys.OpenFlags.O_RDONLY; + flags |= Interop.Sys.OpenFlags.O_CREAT | Interop.Sys.OpenFlags.O_EXCL; // CreateNew + + // Determine the permissions with which to create the file + UnixFileMode perms = UnixFileMode.None; + if ((protections & Interop.Sys.MemoryMappedProtections.PROT_READ) != 0) + perms |= UnixFileMode.UserRead; + if ((protections & Interop.Sys.MemoryMappedProtections.PROT_WRITE) != 0) + perms |= UnixFileMode.UserWrite; + if ((protections & Interop.Sys.MemoryMappedProtections.PROT_EXEC) != 0) + perms |= UnixFileMode.UserExecute; + fd = Interop.Sys.ShmOpen(mapName, flags, (int)perms); // Create the shared memory object. } @@ -204,29 +204,36 @@ private static SafeFileHandle CreateSharedBackingObject(Interop.Sys.MemoryMapped Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); fd.Dispose(); - 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 (!Interop.Sys.MemfdSupported && errorInfo.Error == Interop.Error.ENAMETOOLONG) + if (!Interop.Sys.IsMemfdSupported) { - Debug.Fail($"shm_open failed with ENAMETOOLONG for {Encoding.UTF8.GetByteCount(mapName)} byte long name."); - // in theory it should not happen anymore, but just to be extra safe we use the fallback - return null; - } - else if (errorInfo.Error != Interop.Error.EEXIST) // map with same name already existed - { - throw Interop.GetExceptionForIoErrno(errorInfo); + if (errorInfo.Error == Interop.Error.ENOTSUP) + { + Debug.Assert(!Interop.Sys.IsMemfdSupported); + + // 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.ENAMETOOLONG) + { + Debug.Fail($"shm_open failed with ENAMETOOLONG for {Encoding.UTF8.GetByteCount(mapName)} byte long name."); + // in theory it should not happen anymore, but just to be extra safe we use the fallback + return null; + } + else if (errorInfo.Error == Interop.Error.EEXIST) // map with same name already existed + { + continue; + } } + + throw Interop.GetExceptionForIoErrno(errorInfo); } } while (fd.IsInvalid); try { - if (!Interop.Sys.MemfdSupported) + if (!Interop.Sys.IsMemfdSupported) { // Unlink the shared memory object immediately so that it'll go away once all handles // to it are closed (as with opened then unlinked files, it'll remain usable via diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index f1181f66986607..e88c522818ba69 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -383,6 +383,8 @@ int32_t SystemNative_MemfdSupported(void) } #endif + // Note that the name has no affect on file descriptor behavior. From linux manpage: + // Names do not affect the behavior of the file descriptor, and as such multiple files can have the same name without any side effects. int32_t fd = memfd_create("test", MFD_CLOEXEC); if (fd < 0) return 0; From 56a091dbad4416e90085bc7f35d6abde218cf57f Mon Sep 17 00:00:00 2001 From: Adeel Mujahid <3840695+am11@users.noreply.github.com> Date: Mon, 22 Jul 2024 18:27:53 +0300 Subject: [PATCH 04/13] Update src/native/libs/System.Native/pal_io.c Co-authored-by: Stephen Toub --- src/native/libs/System.Native/pal_io.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index e88c522818ba69..13c338587860c7 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -376,7 +376,8 @@ int32_t SystemNative_MemfdSupported(void) struct utsname uts; int32_t major, minor; - // memfd_create is only known to work properly on kernel version > 3.17 and throws SIGSEGV instead of ENOTSUP + // memfd_create is known to only work properly on kernel version > 3.17. + // On earlier versions, it may raise SIGSEGV instead of returning ENOTSUP. if (uname(&uts) == 0 && sscanf(uts.release, "%d.%d", &major, &minor) == 2 && (major < 3 || (major == 3 && minor < 17))) { return 0; From 74adb284819d3caa10adbd035f1e9620eda72e4e Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Mon, 22 Jul 2024 18:40:05 +0300 Subject: [PATCH 05/13] Implement write sealing --- .../Unix/System.Native/Interop.Fcntl.cs | 3 + .../MemoryMappedFile.Unix.cs | 186 +++++++++++------- src/native/libs/System.Native/entrypoints.c | 1 + src/native/libs/System.Native/pal_io.c | 11 +- src/native/libs/System.Native/pal_io.h | 7 + 5 files changed, 134 insertions(+), 74 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Fcntl.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Fcntl.cs index 71c97699fd07e7..8ce3909a7ff3a8 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Fcntl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Fcntl.cs @@ -22,6 +22,9 @@ internal static partial class Fcntl [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_FcntlSetFD", SetLastError = true)] internal static partial int SetFD(SafeHandle fd, int flags); + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_FcntlSetSealWrite", SetLastError = true)] + internal static partial int SetSealWrite(SafeHandle fd); + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_FcntlGetFD", SetLastError = true)] internal static partial int GetFD(SafeHandle fd); 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 ac4ce03634af62..bb72dadaacc910 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 @@ -162,83 +162,146 @@ private static FileAccess TranslateProtectionsToFileAccess(Interop.Sys.MemoryMap private static SafeFileHandle CreateSharedBackingObject(Interop.Sys.MemoryMappedProtections protections, long capacity, HandleInheritability inheritability) { - return CreateSharedBackingObjectUsingMemory(protections, capacity, inheritability) - ?? CreateSharedBackingObjectUsingFile(protections, capacity, inheritability); + SafeFileHandle? handle = Interop.Sys.IsMemfdSupported ? + CreateSharedBackingObjectUsingMemoryMemfdCreate(protections, capacity, inheritability) : + CreateSharedBackingObjectUsingMemoryShmOpen(protections, capacity, inheritability); + + return handle ?? CreateSharedBackingObjectUsingFile(protections, capacity, inheritability); } - private static SafeFileHandle? CreateSharedBackingObjectUsingMemory( + private static SafeFileHandle? CreateSharedBackingObjectUsingMemoryShmOpen( Interop.Sys.MemoryMappedProtections protections, long capacity, HandleInheritability inheritability) { + // 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 : + Interop.Sys.OpenFlags.O_RDONLY; + flags |= Interop.Sys.OpenFlags.O_CREAT | Interop.Sys.OpenFlags.O_EXCL; // CreateNew + + // Determine the permissions with which to create the file + var perms = UnixFileMode.None; + if ((protections & Interop.Sys.MemoryMappedProtections.PROT_READ) != 0) + perms |= UnixFileMode.UserRead; + if ((protections & Interop.Sys.MemoryMappedProtections.PROT_WRITE) != 0) + perms |= UnixFileMode.UserWrite; + if ((protections & Interop.Sys.MemoryMappedProtections.PROT_EXEC) != 0) + perms |= UnixFileMode.UserExecute; + string mapName; SafeFileHandle fd; do { mapName = GenerateMapName(); - if (Interop.Sys.IsMemfdSupported) + fd = Interop.Sys.ShmOpen(mapName, flags, (int)perms); // Create the shared memory object. + + if (fd.IsInvalid) { - fd = Interop.Sys.MemfdCreate(mapName); + Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); + fd.Dispose(); + + 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.ENAMETOOLONG) + { + Debug.Fail($"shm_open failed with ENAMETOOLONG for {Encoding.UTF8.GetByteCount(mapName)} byte long name."); + // in theory it should not happen anymore, but just to be extra safe we use the fallback + return null; + } + else if (errorInfo.Error != Interop.Error.EEXIST) // map with same name already existed + { + throw Interop.GetExceptionForIoErrno(errorInfo); + } } - else + } while (fd.IsInvalid); + + try + { + // Unlink the shared memory object immediately so that it'll go away once all handles + // to it are closed (as with opened then unlinked files, it'll remain usable via + // the open handles even though it's unlinked and can't be opened anew via its name). + Interop.CheckIo(Interop.Sys.ShmUnlink(mapName)); + + // Give it the right capacity. We do this directly with ftruncate rather + // than via FileStream.SetLength after the FileStream is created because, on some systems, + // lseek fails on shared memory objects, causing the FileStream to think it's unseekable, + // causing it to preemptively throw from SetLength. + Interop.CheckIo(Interop.Sys.FTruncate(fd, capacity)); + + // shm_open sets CLOEXEC implicitly. If the inheritability requested is Inheritable, remove CLOEXEC. + if (inheritability == HandleInheritability.Inheritable && + Interop.Sys.Fcntl.SetFD(fd, 0) == -1) { - // 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 : - Interop.Sys.OpenFlags.O_RDONLY; - flags |= Interop.Sys.OpenFlags.O_CREAT | Interop.Sys.OpenFlags.O_EXCL; // CreateNew - - // Determine the permissions with which to create the file - UnixFileMode perms = UnixFileMode.None; - if ((protections & Interop.Sys.MemoryMappedProtections.PROT_READ) != 0) - perms |= UnixFileMode.UserRead; - if ((protections & Interop.Sys.MemoryMappedProtections.PROT_WRITE) != 0) - perms |= UnixFileMode.UserWrite; - if ((protections & Interop.Sys.MemoryMappedProtections.PROT_EXEC) != 0) - perms |= UnixFileMode.UserExecute; - - fd = Interop.Sys.ShmOpen(mapName, flags, (int)perms); // Create the shared memory object. + fd.Dispose(); + throw Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo()); } + return fd; + } + catch + { + fd.Dispose(); + throw; + } + } + + private static string GenerateMapName() + { + // macOS shm_open documentation says that the sys-call can fail with ENAMETOOLONG if the name exceeds SHM_NAME_MAX characters. + // The problem is that SHM_NAME_MAX is not defined anywhere and is not consistent amongst macOS versions (arm64 vs x64 for example). + // It was reported in 2008 (https://lists.apple.com/archives/xcode-users/2008/Apr/msg00523.html), + // but considered to be by design (http://web.archive.org/web/20140109200632/http://lists.apple.com/archives/darwin-development/2003/Mar/msg00244.html). + // According to https://github.com/qt/qtbase/blob/1ed449e168af133184633d174fd7339a13d1d595/src/corelib/kernel/qsharedmemory.cpp#L53-L56 the actual value is 30. + // Some other OSS libs use 32 (we did as well, but it was not enough) or 31, but we prefer 30 just to be extra safe. + const int MaxNameLength = 30; + // The POSIX shared memory object name must begin with '/'. After that we just want something short (30) and unique. + const string NamePrefix = "/dotnet_"; + return string.Create(MaxNameLength, 0, (span, state) => + { + Span guid = stackalloc char[32]; + Guid.NewGuid().TryFormat(guid, out int charsWritten, "N"); + Debug.Assert(charsWritten == 32); + NamePrefix.CopyTo(span); + guid.Slice(0, MaxNameLength - NamePrefix.Length).CopyTo(span.Slice(NamePrefix.Length)); + Debug.Assert(Encoding.UTF8.GetByteCount(span) <= MaxNameLength); // the standard uses Utf8 + }); + } + + private static SafeFileHandle? CreateSharedBackingObjectUsingMemoryMemfdCreate( + Interop.Sys.MemoryMappedProtections protections, long capacity, HandleInheritability inheritability) + { + string mapName; + SafeFileHandle fd; + + do + { + mapName = GenerateMapName(); + fd = Interop.Sys.MemfdCreate(mapName); + if (fd.IsInvalid) { Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); fd.Dispose(); - if (!Interop.Sys.IsMemfdSupported) - { - if (errorInfo.Error == Interop.Error.ENOTSUP) - { - Debug.Assert(!Interop.Sys.IsMemfdSupported); - - // 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.ENAMETOOLONG) - { - Debug.Fail($"shm_open failed with ENAMETOOLONG for {Encoding.UTF8.GetByteCount(mapName)} byte long name."); - // in theory it should not happen anymore, but just to be extra safe we use the fallback - return null; - } - else if (errorInfo.Error == Interop.Error.EEXIST) // map with same name already existed - { - continue; - } - } - throw Interop.GetExceptionForIoErrno(errorInfo); } } while (fd.IsInvalid); try { - if (!Interop.Sys.IsMemfdSupported) + // Add a writeseal for readonly case when eadonly protection requested + if ((protections & Interop.Sys.MemoryMappedProtections.PROT_READ) != 0 && + (protections & Interop.Sys.MemoryMappedProtections.PROT_WRITE) == 0 && + // seal write failed + Interop.Sys.Fcntl.SetSealWrite(fd) == -1) { - // Unlink the shared memory object immediately so that it'll go away once all handles - // to it are closed (as with opened then unlinked files, it'll remain usable via - // the open handles even though it's unlinked and can't be opened anew via its name). - Interop.CheckIo(Interop.Sys.ShmUnlink(mapName)); + fd.Dispose(); + throw Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo()); } // Give it the right capacity. We do this directly with ftruncate rather @@ -251,6 +314,7 @@ private static SafeFileHandle CreateSharedBackingObject(Interop.Sys.MemoryMapped if (inheritability == HandleInheritability.Inheritable && Interop.Sys.Fcntl.SetFD(fd, 0) == -1) { + fd.Dispose(); throw Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo()); } @@ -261,28 +325,6 @@ private static SafeFileHandle CreateSharedBackingObject(Interop.Sys.MemoryMapped fd.Dispose(); throw; } - - static string GenerateMapName() - { - // macOS shm_open documentation says that the sys-call can fail with ENAMETOOLONG if the name exceeds SHM_NAME_MAX characters. - // The problem is that SHM_NAME_MAX is not defined anywhere and is not consistent amongst macOS versions (arm64 vs x64 for example). - // It was reported in 2008 (https://lists.apple.com/archives/xcode-users/2008/Apr/msg00523.html), - // but considered to be by design (http://web.archive.org/web/20140109200632/http://lists.apple.com/archives/darwin-development/2003/Mar/msg00244.html). - // According to https://github.com/qt/qtbase/blob/1ed449e168af133184633d174fd7339a13d1d595/src/corelib/kernel/qsharedmemory.cpp#L53-L56 the actual value is 30. - // Some other OSS libs use 32 (we did as well, but it was not enough) or 31, but we prefer 30 just to be extra safe. - const int MaxNameLength = 30; - // The POSIX shared memory object name must begin with '/'. After that we just want something short (30) and unique. - const string NamePrefix = "/dotnet_"; - return string.Create(MaxNameLength, 0, (span, state) => - { - Span guid = stackalloc char[32]; - Guid.NewGuid().TryFormat(guid, out int charsWritten, "N"); - Debug.Assert(charsWritten == 32); - NamePrefix.CopyTo(span); - guid.Slice(0, MaxNameLength - NamePrefix.Length).CopyTo(span.Slice(NamePrefix.Length)); - Debug.Assert(Encoding.UTF8.GetByteCount(span) <= MaxNameLength); // the standard uses Utf8 - }); - } } private static SafeFileHandle CreateSharedBackingObjectUsingFile(Interop.Sys.MemoryMappedProtections protections, long capacity, HandleInheritability inheritability) diff --git a/src/native/libs/System.Native/entrypoints.c b/src/native/libs/System.Native/entrypoints.c index d9f9b4d537b0ad..bb519f01b46cba 100644 --- a/src/native/libs/System.Native/entrypoints.c +++ b/src/native/libs/System.Native/entrypoints.c @@ -71,6 +71,7 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_OpenDir) DllImportEntry(SystemNative_CloseDir) DllImportEntry(SystemNative_Pipe) + DllImportEntry(SystemNative_FcntlSetSealWrite) DllImportEntry(SystemNative_FcntlSetFD) DllImportEntry(SystemNative_FcntlGetFD) DllImportEntry(SystemNative_FcntlCanGetSetPipeSz) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 13c338587860c7..eb39892546b624 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -386,7 +386,7 @@ int32_t SystemNative_MemfdSupported(void) // Note that the name has no affect on file descriptor behavior. From linux manpage: // Names do not affect the behavior of the file descriptor, and as such multiple files can have the same name without any side effects. - int32_t fd = memfd_create("test", MFD_CLOEXEC); + int32_t fd = memfd_create("test", MFD_CLOEXEC | MFD_ALLOW_SEALING); if (fd < 0) return 0; close(fd); @@ -406,7 +406,7 @@ intptr_t SystemNative_MemfdCreate(const char* name) assert(strlen(name) <= PATH_MAX); #endif - return memfd_create(name, MFD_CLOEXEC); + return memfd_create(name, MFD_CLOEXEC | MFD_ALLOW_SEALING); #else (void)name; errno = ENOTSUP; @@ -666,6 +666,13 @@ int32_t SystemNative_Pipe(int32_t pipeFds[2], int32_t flags) return result; } +int32_t SystemNative_FcntlSetSealWrite(intptr_t fd) +{ + int result; + while ((result = fcntl(ToFileDescriptor(fd), F_ADD_SEALS, F_SEAL_WRITE)) < 0 && errno == EINTR); + return result; +} + int32_t SystemNative_FcntlSetFD(intptr_t fd, int32_t flags) { int result; diff --git a/src/native/libs/System.Native/pal_io.h b/src/native/libs/System.Native/pal_io.h index a5a8261e569372..5df85e20641ed0 100644 --- a/src/native/libs/System.Native/pal_io.h +++ b/src/native/libs/System.Native/pal_io.h @@ -432,6 +432,13 @@ PALEXPORT int32_t SystemNative_Pipe(int32_t pipefd[2], // [out] pipefds[0] gets // for each command. This allows use to have strongly typed arguments and saves // complexity around converting command codes. +/** + * Sets the write seal on memfd_create file descriptor. + * + * Returns 0 for success; -1 for failure. Sets errno for failure. + */ +PALEXPORT int32_t SystemNative_FcntlSetSealWrite(intptr_t fd); + /** * Sets the O_CLOEXEC flag on a file descriptor. * From d2ec4c90f015546400df67664f98c0fd7eb8175a Mon Sep 17 00:00:00 2001 From: Adeel Mujahid <3840695+am11@users.noreply.github.com> Date: Mon, 22 Jul 2024 18:59:31 +0300 Subject: [PATCH 06/13] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) 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 bb72dadaacc910..017cc994735e30 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 @@ -294,14 +294,15 @@ private static string GenerateMapName() try { - // Add a writeseal for readonly case when eadonly protection requested + // Add a write seal for readonly case when readonly protection requested if ((protections & Interop.Sys.MemoryMappedProtections.PROT_READ) != 0 && (protections & Interop.Sys.MemoryMappedProtections.PROT_WRITE) == 0 && - // seal write failed Interop.Sys.Fcntl.SetSealWrite(fd) == -1) { + // seal write failed + Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); fd.Dispose(); - throw Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo()); + throw Interop.GetExceptionForIoErrno(errorInfo); } // Give it the right capacity. We do this directly with ftruncate rather @@ -310,12 +311,13 @@ private static string GenerateMapName() // causing it to preemptively throw from SetLength. Interop.CheckIo(Interop.Sys.FTruncate(fd, capacity)); - // shm_open sets CLOEXEC implicitly. If the inheritability requested is Inheritable, remove CLOEXEC. + // SystemNative_MemfdCreate sets CLOEXEC implicitly. If the inheritability requested is Inheritable, remove CLOEXEC. if (inheritability == HandleInheritability.Inheritable && Interop.Sys.Fcntl.SetFD(fd, 0) == -1) { + Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); fd.Dispose(); - throw Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo()); + throw Interop.GetExceptionForIoErrno(errorInfo); } return fd; From 67ca808d1d4c855a5d84eb7b5a930732163a4479 Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Mon, 22 Jul 2024 19:02:58 +0300 Subject: [PATCH 07/13] Address CR feedback --- .../MemoryMappedFile.Unix.cs | 34 ++++++------------- 1 file changed, 11 insertions(+), 23 deletions(-) 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 017cc994735e30..ea52f93b945f63 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 @@ -162,11 +162,10 @@ private static FileAccess TranslateProtectionsToFileAccess(Interop.Sys.MemoryMap private static SafeFileHandle CreateSharedBackingObject(Interop.Sys.MemoryMappedProtections protections, long capacity, HandleInheritability inheritability) { - SafeFileHandle? handle = Interop.Sys.IsMemfdSupported ? + return Interop.Sys.IsMemfdSupported ? CreateSharedBackingObjectUsingMemoryMemfdCreate(protections, capacity, inheritability) : - CreateSharedBackingObjectUsingMemoryShmOpen(protections, capacity, inheritability); - - return handle ?? CreateSharedBackingObjectUsingFile(protections, capacity, inheritability); + CreateSharedBackingObjectUsingMemoryShmOpen(protections, capacity, inheritability) + ?? CreateSharedBackingObjectUsingFile(protections, capacity, inheritability); } private static SafeFileHandle? CreateSharedBackingObjectUsingMemoryShmOpen( @@ -237,7 +236,6 @@ private static SafeFileHandle CreateSharedBackingObject(Interop.Sys.MemoryMapped if (inheritability == HandleInheritability.Inheritable && Interop.Sys.Fcntl.SetFD(fd, 0) == -1) { - fd.Dispose(); throw Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo()); } @@ -272,25 +270,17 @@ private static string GenerateMapName() }); } - private static SafeFileHandle? CreateSharedBackingObjectUsingMemoryMemfdCreate( + private static SafeFileHandle CreateSharedBackingObjectUsingMemoryMemfdCreate( Interop.Sys.MemoryMappedProtections protections, long capacity, HandleInheritability inheritability) { - string mapName; - SafeFileHandle fd; - - do + SafeFileHandle fd = Interop.Sys.MemfdCreate(GenerateMapName()); + if (fd.IsInvalid) { - mapName = GenerateMapName(); - fd = Interop.Sys.MemfdCreate(mapName); - - if (fd.IsInvalid) - { - Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - fd.Dispose(); + Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); + fd.Dispose(); - throw Interop.GetExceptionForIoErrno(errorInfo); - } - } while (fd.IsInvalid); + throw Interop.GetExceptionForIoErrno(errorInfo); + } try { @@ -315,9 +305,7 @@ private static string GenerateMapName() if (inheritability == HandleInheritability.Inheritable && Interop.Sys.Fcntl.SetFD(fd, 0) == -1) { - Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - fd.Dispose(); - throw Interop.GetExceptionForIoErrno(errorInfo); + throw Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo()); } return fd; From 6a93c49eb37800b225482a65b6fa4f7a42270595 Mon Sep 17 00:00:00 2001 From: Adeel Mujahid <3840695+am11@users.noreply.github.com> Date: Mon, 22 Jul 2024 19:34:46 +0300 Subject: [PATCH 08/13] Update src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs Co-authored-by: Stephen Toub --- .../src/System/IO/MemoryMappedFiles/MemoryMappedFile.Unix.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 ea52f93b945f63..d4a77f57e6ebd2 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 @@ -290,9 +290,7 @@ private static SafeFileHandle CreateSharedBackingObjectUsingMemoryMemfdCreate( Interop.Sys.Fcntl.SetSealWrite(fd) == -1) { // seal write failed - Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - fd.Dispose(); - throw Interop.GetExceptionForIoErrno(errorInfo); + throw Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo()); } // Give it the right capacity. We do this directly with ftruncate rather From b8e89bcbeb404df5c1c13376686223460d6e06cc Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Mon, 22 Jul 2024 20:19:52 +0300 Subject: [PATCH 09/13] Remove extra P/Invoke call --- .../Unix/System.Native/Interop.Fcntl.cs | 3 --- .../Unix/System.Native/Interop.MemfdCreate.cs | 4 ++-- .../MemoryMappedFile.Unix.cs | 14 ++++---------- src/native/libs/System.Native/entrypoints.c | 3 +-- src/native/libs/System.Native/pal_io.c | 19 +++++++++---------- src/native/libs/System.Native/pal_io.h | 11 ++--------- 6 files changed, 18 insertions(+), 36 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Fcntl.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Fcntl.cs index 8ce3909a7ff3a8..71c97699fd07e7 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Fcntl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Fcntl.cs @@ -22,9 +22,6 @@ internal static partial class Fcntl [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_FcntlSetFD", SetLastError = true)] internal static partial int SetFD(SafeHandle fd, int flags); - [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_FcntlSetSealWrite", SetLastError = true)] - internal static partial int SetSealWrite(SafeHandle fd); - [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_FcntlGetFD", SetLastError = true)] internal static partial int GetFD(SafeHandle fd); diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MemfdCreate.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MemfdCreate.cs index ad05741923197b..5fdca42fdb4aae 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MemfdCreate.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MemfdCreate.cs @@ -10,9 +10,9 @@ internal static partial class Interop internal static partial class Sys { [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_MemfdCreate", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] - internal static partial SafeFileHandle MemfdCreate(string name); + internal static partial SafeFileHandle MemfdCreate(string name, int isReadonly); - [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_MemfdSupported", SetLastError = true)] + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_IsMemfdSupported", SetLastError = true)] private static partial int MemfdSupportedImpl(); private static volatile sbyte s_memfdSupported; 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 d4a77f57e6ebd2..ae9e24c5722973 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 @@ -273,7 +273,10 @@ private static string GenerateMapName() private static SafeFileHandle CreateSharedBackingObjectUsingMemoryMemfdCreate( Interop.Sys.MemoryMappedProtections protections, long capacity, HandleInheritability inheritability) { - SafeFileHandle fd = Interop.Sys.MemfdCreate(GenerateMapName()); + int isReadonly = ((protections & Interop.Sys.MemoryMappedProtections.PROT_READ) != 0 && + (protections & Interop.Sys.MemoryMappedProtections.PROT_WRITE) == 0) ? 1 : 0; + + SafeFileHandle fd = Interop.Sys.MemfdCreate(GenerateMapName(), isReadonly); if (fd.IsInvalid) { Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); @@ -284,15 +287,6 @@ private static SafeFileHandle CreateSharedBackingObjectUsingMemoryMemfdCreate( try { - // Add a write seal for readonly case when readonly protection requested - if ((protections & Interop.Sys.MemoryMappedProtections.PROT_READ) != 0 && - (protections & Interop.Sys.MemoryMappedProtections.PROT_WRITE) == 0 && - Interop.Sys.Fcntl.SetSealWrite(fd) == -1) - { - // seal write failed - throw Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo()); - } - // Give it the right capacity. We do this directly with ftruncate rather // than via FileStream.SetLength after the FileStream is created because, on some systems, // lseek fails on shared memory objects, causing the FileStream to think it's unseekable, diff --git a/src/native/libs/System.Native/entrypoints.c b/src/native/libs/System.Native/entrypoints.c index bb519f01b46cba..81f33c727c0457 100644 --- a/src/native/libs/System.Native/entrypoints.c +++ b/src/native/libs/System.Native/entrypoints.c @@ -62,7 +62,7 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_Close) DllImportEntry(SystemNative_Dup) DllImportEntry(SystemNative_Unlink) - DllImportEntry(SystemNative_MemfdSupported) + DllImportEntry(SystemNative_IsMemfdSupported) DllImportEntry(SystemNative_MemfdCreate) DllImportEntry(SystemNative_ShmOpen) DllImportEntry(SystemNative_ShmUnlink) @@ -71,7 +71,6 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_OpenDir) DllImportEntry(SystemNative_CloseDir) DllImportEntry(SystemNative_Pipe) - DllImportEntry(SystemNative_FcntlSetSealWrite) DllImportEntry(SystemNative_FcntlSetFD) DllImportEntry(SystemNative_FcntlGetFD) DllImportEntry(SystemNative_FcntlCanGetSetPipeSz) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index eb39892546b624..beb3e0e083d9db 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -10,6 +10,7 @@ #include "pal_types.h" #include +#include #include #include #include @@ -369,7 +370,7 @@ int32_t SystemNative_Unlink(const char* path) return result; } -int32_t SystemNative_MemfdSupported(void) +int32_t SystemNative_IsMemfdSupported(void) { #if HAVE_MEMFD_CREATE #ifdef TARGET_LINUX @@ -397,7 +398,7 @@ int32_t SystemNative_MemfdSupported(void) #endif } -intptr_t SystemNative_MemfdCreate(const char* name) +intptr_t SystemNative_MemfdCreate(const char* name, int32_t isReadonly) { #if HAVE_MEMFD_CREATE #if defined(SHM_NAME_MAX) // macOS @@ -406,7 +407,12 @@ intptr_t SystemNative_MemfdCreate(const char* name) assert(strlen(name) <= PATH_MAX); #endif - return memfd_create(name, MFD_CLOEXEC | MFD_ALLOW_SEALING); + int32_t fd = memfd_create(name, MFD_CLOEXEC | MFD_ALLOW_SEALING); + if (!isReadonly || fd < 0) return fd; + + // Add a write seal when readonly protection requested + while (fcntl(fd, F_ADD_SEALS, F_SEAL_WRITE) < 0 && errno == EINTR); + return fd; #else (void)name; errno = ENOTSUP; @@ -666,13 +672,6 @@ int32_t SystemNative_Pipe(int32_t pipeFds[2], int32_t flags) return result; } -int32_t SystemNative_FcntlSetSealWrite(intptr_t fd) -{ - int result; - while ((result = fcntl(ToFileDescriptor(fd), F_ADD_SEALS, F_SEAL_WRITE)) < 0 && errno == EINTR); - return result; -} - int32_t SystemNative_FcntlSetFD(intptr_t fd, int32_t flags) { int result; diff --git a/src/native/libs/System.Native/pal_io.h b/src/native/libs/System.Native/pal_io.h index 5df85e20641ed0..a07aa7c170219a 100644 --- a/src/native/libs/System.Native/pal_io.h +++ b/src/native/libs/System.Native/pal_io.h @@ -374,14 +374,14 @@ PALEXPORT int32_t SystemNative_Unlink(const char* path); * * Returns 1 if memfd_create is supported, 0 if not supported, or -1 on failure. Sets errno on failure. */ -PALEXPORT int32_t SystemNative_MemfdSupported(void); +PALEXPORT int32_t SystemNative_IsMemfdSupported(void); /** * Create an anonymous file descriptor. Implemented as shim to memfd_create(2). * * Returns file descriptor or -1 on failure. Sets errno on failure. */ -PALEXPORT intptr_t SystemNative_MemfdCreate(const char* name); +PALEXPORT intptr_t SystemNative_MemfdCreate(const char* name, int32_t isReadonly); /** * Open or create a shared memory object. Implemented as shim to shm_open(3). @@ -432,13 +432,6 @@ PALEXPORT int32_t SystemNative_Pipe(int32_t pipefd[2], // [out] pipefds[0] gets // for each command. This allows use to have strongly typed arguments and saves // complexity around converting command codes. -/** - * Sets the write seal on memfd_create file descriptor. - * - * Returns 0 for success; -1 for failure. Sets errno for failure. - */ -PALEXPORT int32_t SystemNative_FcntlSetSealWrite(intptr_t fd); - /** * Sets the O_CLOEXEC flag on a file descriptor. * From 4425b0ec1a01b1571fbbde2e3859856662995310 Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Mon, 22 Jul 2024 21:23:47 +0300 Subject: [PATCH 10/13] Fix CI build --- src/native/libs/System.Native/pal_io.c | 2 +- src/native/libs/configure.cmake | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index beb3e0e083d9db..7a7fafd1dfe501 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include @@ -415,6 +414,7 @@ intptr_t SystemNative_MemfdCreate(const char* name, int32_t isReadonly) return fd; #else (void)name; + (void)isReadonly; errno = ENOTSUP; return -1; #endif diff --git a/src/native/libs/configure.cmake b/src/native/libs/configure.cmake index 07f7918406d920..50a0f776c28409 100644 --- a/src/native/libs/configure.cmake +++ b/src/native/libs/configure.cmake @@ -139,6 +139,7 @@ check_symbol_exists( check_function_exists( memfd_create + fcntl.h HAVE_MEMFD_CREATE) check_function_exists( From 4c3b65bce4b0b2fc9d738dafada32e35e84698f1 Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Mon, 22 Jul 2024 22:30:38 +0300 Subject: [PATCH 11/13] Add support for older libc --- src/native/libs/Common/pal_config.h.in | 1 - src/native/libs/System.Native/pal_io.c | 24 ++++++++++++++++++++---- src/native/libs/configure.cmake | 5 ----- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/native/libs/Common/pal_config.h.in b/src/native/libs/Common/pal_config.h.in index 211bd2949ae42f..0960f6aa971c8e 100644 --- a/src/native/libs/Common/pal_config.h.in +++ b/src/native/libs/Common/pal_config.h.in @@ -11,7 +11,6 @@ #cmakedefine01 HAVE_F_DUPFD #cmakedefine01 HAVE_F_FULLFSYNC #cmakedefine01 HAVE_O_CLOEXEC -#cmakedefine01 HAVE_MEMFD_CREATE #cmakedefine01 HAVE_GETIFADDRS #cmakedefine01 HAVE_UTSNAME_DOMAINNAME #cmakedefine01 HAVE_STAT64 diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 7a7fafd1dfe501..3e7f1a7dd5a180 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -369,9 +369,25 @@ int32_t SystemNative_Unlink(const char* path) return result; } +#ifndef MFD_CLOEXEC +#define MFD_CLOEXEC 0x0001U +#endif + +#ifndef MFD_ALLOW_SEALING +#define MFD_ALLOW_SEALING 0x0002U +#endif + +#ifndef F_ADD_SEALS +#define F_ADD_SEALS (1024 + 9) +#endif + +#ifndef F_SEAL_WRITE +#define F_SEAL_WRITE 0x0008 +#endif + int32_t SystemNative_IsMemfdSupported(void) { -#if HAVE_MEMFD_CREATE +#ifdef __NR_memfd_create #ifdef TARGET_LINUX struct utsname uts; int32_t major, minor; @@ -386,7 +402,7 @@ int32_t SystemNative_IsMemfdSupported(void) // Note that the name has no affect on file descriptor behavior. From linux manpage: // Names do not affect the behavior of the file descriptor, and as such multiple files can have the same name without any side effects. - int32_t fd = memfd_create("test", MFD_CLOEXEC | MFD_ALLOW_SEALING); + int32_t fd = (int32_t)syscall(__NR_memfd_create, "test", MFD_CLOEXEC | MFD_ALLOW_SEALING); if (fd < 0) return 0; close(fd); @@ -399,14 +415,14 @@ int32_t SystemNative_IsMemfdSupported(void) intptr_t SystemNative_MemfdCreate(const char* name, int32_t isReadonly) { -#if HAVE_MEMFD_CREATE +#ifdef __NR_memfd_create #if defined(SHM_NAME_MAX) // macOS assert(strlen(name) <= SHM_NAME_MAX); #elif defined(PATH_MAX) // other Unixes assert(strlen(name) <= PATH_MAX); #endif - int32_t fd = memfd_create(name, MFD_CLOEXEC | MFD_ALLOW_SEALING); + int32_t fd = (int32_t)syscall(__NR_memfd_create, name, MFD_CLOEXEC | MFD_ALLOW_SEALING); if (!isReadonly || fd < 0) return fd; // Add a write seal when readonly protection requested diff --git a/src/native/libs/configure.cmake b/src/native/libs/configure.cmake index 50a0f776c28409..871e5bd2ea8099 100644 --- a/src/native/libs/configure.cmake +++ b/src/native/libs/configure.cmake @@ -137,11 +137,6 @@ check_symbol_exists( fcntl.h HAVE_F_FULLFSYNC) -check_function_exists( - memfd_create - fcntl.h - HAVE_MEMFD_CREATE) - check_function_exists( getifaddrs HAVE_GETIFADDRS) From 26f991e074fd31c582bf07e47651d1bfcae7cca6 Mon Sep 17 00:00:00 2001 From: Adeel Mujahid <3840695+am11@users.noreply.github.com> Date: Mon, 22 Jul 2024 22:35:07 +0300 Subject: [PATCH 12/13] Style nit --- src/native/libs/System.Native/pal_io.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 3e7f1a7dd5a180..a10aa3e81a4835 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -370,19 +370,16 @@ int32_t SystemNative_Unlink(const char* path) } #ifndef MFD_CLOEXEC -#define MFD_CLOEXEC 0x0001U +#define MFD_CLOEXEC 0x0001U #endif - #ifndef MFD_ALLOW_SEALING -#define MFD_ALLOW_SEALING 0x0002U +#define MFD_ALLOW_SEALING 0x0002U #endif - #ifndef F_ADD_SEALS -#define F_ADD_SEALS (1024 + 9) +#define F_ADD_SEALS (1024 + 9) #endif - #ifndef F_SEAL_WRITE -#define F_SEAL_WRITE 0x0008 +#define F_SEAL_WRITE 0x0008 #endif int32_t SystemNative_IsMemfdSupported(void) From 75faa7998d9e277021e0ee256904e6bff9a7c65c Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Mon, 22 Jul 2024 22:51:53 +0300 Subject: [PATCH 13/13] Fix osx build --- src/native/libs/System.Native/pal_io.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index a10aa3e81a4835..cc285430834a53 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -369,6 +369,7 @@ int32_t SystemNative_Unlink(const char* path) return result; } +#ifdef __NR_memfd_create #ifndef MFD_CLOEXEC #define MFD_CLOEXEC 0x0001U #endif @@ -381,6 +382,7 @@ int32_t SystemNative_Unlink(const char* path) #ifndef F_SEAL_WRITE #define F_SEAL_WRITE 0x0008 #endif +#endif int32_t SystemNative_IsMemfdSupported(void) {