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.

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

[release/6.0] Make sure that shared memory object name meets the length requiremens #64266

Merged
merged 1 commit into from
Feb 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 6 additions & 0 deletions src/libraries/Native/Unix/System.Native/pal_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 :
Expand All @@ -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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, it's a little weird to Dispose of the SafeHandle and then continue to use it, but technically in this case it's ok to do so, given how it's being used. It's just unusual to see and thus a little disconcerting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a comment here would help clarify the unusual usage?


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
{
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ namespace System.IO.MemoryMappedFiles.Tests
/// <summary>
/// Tests for MemoryMappedFile.CreateNew.
/// </summary>
[ActiveIssue("https://github.com/dotnet/runtime/issues/49104", typeof(PlatformDetection), nameof(PlatformDetection.IsMacOsAppleSilicon))]
public class MemoryMappedFileTests_CreateNew : MemoryMappedFilesTestBase
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ namespace System.IO.MemoryMappedFiles.Tests
/// <summary>
/// Tests for MemoryMappedViewAccessor.
/// </summary>
[ActiveIssue("https://github.com/dotnet/runtime/issues/49104", typeof(PlatformDetection), nameof(PlatformDetection.IsMacOsAppleSilicon))]
public class MemoryMappedViewAccessorTests : MemoryMappedFilesTestBase
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ namespace System.IO.MemoryMappedFiles.Tests
/// <summary>
/// Tests for MemoryMappedViewStream.
/// </summary>
[ActiveIssue("https://github.com/dotnet/runtime/issues/49104", typeof(PlatformDetection), nameof(PlatformDetection.IsMacOsAppleSilicon))]
public class MemoryMappedViewStreamTests : MemoryMappedFilesTestBase
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ private Task<Stream> 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) =>
Expand Down