From 04d3fb06a3b240d0787e236a36192e103903342d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Cant=C3=BA?= Date: Thu, 12 Jun 2025 21:57:16 +0000 Subject: [PATCH 1/2] Increase d_name size --- .../Unix/System.Native/Interop.ReadDir.cs | 10 +--- .../IO/Enumeration/FileSystemEntry.Unix.cs | 5 +- .../System/TimeZoneInfo.Unix.NonAndroid.cs | 11 ---- .../Directory/EnumerableTests.cs | 21 +++++++ .../ManualTests/NtfsOnLinuxSetup.cs | 56 +++++++++++++++++++ .../ManualTests/NtfsOnLinuxTests.cs | 29 ++++++++++ .../System.IO.FileSystem.Manual.Tests.csproj | 2 + src/native/libs/System.Native/pal_io.c | 11 +++- 8 files changed, 122 insertions(+), 23 deletions(-) create mode 100644 src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/NtfsOnLinuxSetup.cs create mode 100644 src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/NtfsOnLinuxTests.cs diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ReadDir.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ReadDir.cs index 9362a84a4c280f..013287e5ee0c07 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ReadDir.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ReadDir.cs @@ -29,26 +29,20 @@ internal unsafe struct DirectoryEntry internal byte* Name; internal int NameLength; internal NodeType InodeType; - internal const int NameBufferSize = 256; // sizeof(dirent->d_name) == NAME_MAX + 1 internal ReadOnlySpan GetName(Span buffer) { - // -1 for null terminator (buffer will not include one), - // and -1 because GetMaxCharCount pessimistically assumes the buffer may start with a partial surrogate - Debug.Assert(buffer.Length >= Encoding.UTF8.GetMaxCharCount(NameBufferSize - 1 - 1)); - Debug.Assert(Name != null, "should not have a null name"); ReadOnlySpan nameBytes = (NameLength == -1) - // In this case the struct was allocated via struct dirent *readdir(DIR *dirp); - ? new ReadOnlySpan(Name, new ReadOnlySpan(Name, NameBufferSize).IndexOf(0)) + ? MemoryMarshal.CreateReadOnlySpanFromNullTerminated(Name) : new ReadOnlySpan(Name, NameLength); Debug.Assert(nameBytes.Length > 0, "we shouldn't have gotten a garbage value from the OS"); int charCount = Encoding.UTF8.GetChars(nameBytes, buffer); ReadOnlySpan value = buffer.Slice(0, charCount); - Debug.Assert(NameLength != -1 || !value.Contains('\0'), "should not have embedded nulls if we parsed the end of string"); + Debug.Assert(!value.Contains('\0'), "should not have embedded nulls"); return value; } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs index be406ecf507146..96fd73796840d7 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs @@ -11,6 +11,7 @@ namespace System.IO.Enumeration /// public unsafe ref partial struct FileSystemEntry { + private const int DecodedNameMaxLength = 256; private Interop.Sys.DirectoryEntry _directoryEntry; private bool _isDirectory; private FileStatus _status; @@ -22,7 +23,7 @@ public unsafe ref partial struct FileSystemEntry // Wrap the fixed buffer to workaround visibility issues in api compat verification private struct FileNameBuffer { - internal fixed char _buffer[Interop.Sys.DirectoryEntry.NameBufferSize]; + internal fixed char _buffer[DecodedNameMaxLength]; } internal static FileAttributes Initialize( @@ -95,7 +96,7 @@ public ReadOnlySpan FileName { if (_directoryEntry.NameLength != 0 && _fileName.Length == 0) { - Span buffer = MemoryMarshal.CreateSpan(ref _fileNameBuffer._buffer[0], Interop.Sys.DirectoryEntry.NameBufferSize); + Span buffer = MemoryMarshal.CreateSpan(ref _fileNameBuffer._buffer[0], DecodedNameMaxLength); _fileName = _directoryEntry.GetName(buffer); } diff --git a/src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.NonAndroid.cs b/src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.NonAndroid.cs index 39f0df8a4b1ee4..7969fbd99c9a0b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.NonAndroid.cs +++ b/src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.NonAndroid.cs @@ -260,17 +260,6 @@ private static List ParseTimeZoneIds(StreamReader reader) return id; } - private static string? GetDirectoryEntryFullPath(ref Interop.Sys.DirectoryEntry dirent, string currentPath) - { - ReadOnlySpan direntName = dirent.GetName(stackalloc char[Interop.Sys.DirectoryEntry.NameBufferSize]); - - if ((direntName.Length == 1 && direntName[0] == '.') || - (direntName.Length == 2 && direntName[0] == '.' && direntName[1] == '.')) - return null; - - return Path.Join(currentPath.AsSpan(), direntName); - } - private static bool CompareTimeZoneFile(string filePath, byte[] buffer, byte[] rawData) { try diff --git a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/EnumerableTests.cs b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/EnumerableTests.cs index dc0d6d9a58c62f..20f61989594b09 100644 --- a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/EnumerableTests.cs +++ b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/EnumerableTests.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.Collections.Concurrent; using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; @@ -34,6 +35,26 @@ public void FileEnumeratorIsThreadSafe() } } + [Fact] + public void FileEnumeratorIsThreadSafe_ParallelForEach() + { + List expected = []; + string directory = Directory.CreateDirectory(GetTestFilePath()).FullName; + for (int i = 0; i < 100; i++) + { + string file = Path.Join(directory, GetTestFileName()); + File.Create(file).Dispose(); + expected.Add(file); + } + + for (int i = 0; i < 100; i++) // test multiple times to ensure thread safety. + { + ConcurrentBag result = []; + ParallelLoopResult parallelResult = Parallel.ForEach(Directory.EnumerateFiles(directory), f => result.Add(f)); + AssertExtensions.CollectionEqual(expected, result, StringComparer.Ordinal); + } + } + [Fact] public void EnumerateDirectories_NonBreakingSpace() { diff --git a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/NtfsOnLinuxSetup.cs b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/NtfsOnLinuxSetup.cs new file mode 100644 index 00000000000000..76bb3304a2f793 --- /dev/null +++ b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/NtfsOnLinuxSetup.cs @@ -0,0 +1,56 @@ +// 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 Xunit.Sdk; + +namespace System.IO.ManualTests +{ + public class NtfsOnLinuxSetup : IDisposable + { + public NtfsOnLinuxSetup() + { + if (!NtfsOnLinuxTests.IsManualTestsEnabledAndElevated) + throw new XunitException("Set MANUAL_TESTS envvar and run as elevated to execute this test setup."); + + ExecuteShell(""" + dd if=/dev/zero of=my_loop_device.img bs=1M count=100 + losetup /dev/loop99 my_loop_device.img + mkfs -t ntfs /dev/loop99 + mkdir -p /mnt/ntfs + mount /dev/loop99 /mnt/ntfs + """); + } + + public void Dispose() + { + ExecuteShell(""" + umount /mnt/ntfs + losetup -d /dev/loop99 + rm my_loop_device.img + """); + } + + private static void ExecuteShell(string command) + { + using Process process = new Process + { + StartInfo = new ProcessStartInfo + { + FileName = "/bin/sh", + ArgumentList = { "-c", command }, + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false + } + }; + process.OutputDataReceived += (sender, e) => Console.WriteLine($"[OUTPUT] {e.Data}"); + process.ErrorDataReceived += (sender, e) => Console.WriteLine($"[ERROR] {e.Data}"); + + process.Start(); + process.BeginOutputReadLine(); + process.BeginErrorReadLine(); + process.WaitForExit(); + } + } +} diff --git a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/NtfsOnLinuxTests.cs b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/NtfsOnLinuxTests.cs new file mode 100644 index 00000000000000..3888321e9f58ec --- /dev/null +++ b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/NtfsOnLinuxTests.cs @@ -0,0 +1,29 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Linq; +using System.Text; +using Xunit; + +namespace System.IO.ManualTests +{ + public class NtfsOnLinuxTests : IClassFixture + { + internal static bool IsManualTestsEnabledAndElevated => FileSystemManualTests.ManualTestsEnabled && AdminHelpers.IsProcessElevated(); + + [ConditionalTheory(nameof(IsManualTestsEnabledAndElevated))] + [PlatformSpecific(TestPlatforms.Linux)] + [InlineData("Ω", 255)] + [InlineData("あ", 255)] + [InlineData("😀", 127)] + public void NtfsOnLinux_FilenamesLongerThan255Bytes_FileEnumerationSucceeds(string codePoint, int maxAllowedLength) + { + string filename = string.Concat(Enumerable.Repeat(codePoint, maxAllowedLength)); + Assert.True(Encoding.UTF8.GetByteCount(filename) > 255); + + string filePath = $"/mnt/ntfs/{filename}"; + File.Create(filePath).Dispose(); + Assert.Contains(filePath, Directory.GetFiles("/mnt/ntfs")); + } + } +} diff --git a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/System.IO.FileSystem.Manual.Tests.csproj b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/System.IO.FileSystem.Manual.Tests.csproj index 62623a6e9757cc..4f840033a65599 100644 --- a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/System.IO.FileSystem.Manual.Tests.csproj +++ b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/System.IO.FileSystem.Manual.Tests.csproj @@ -4,5 +4,7 @@ + + diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 3777ad55151f9d..507fad692ec240 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -506,12 +507,18 @@ static const size_t dirent_alignment = 8; int32_t SystemNative_GetReadDirRBufferSize(void) { #if HAVE_READDIR_R - size_t result = sizeof(struct dirent); + size_t result = offsetof(struct dirent, d_name); #ifdef TARGET_SUNOS // The d_name array is declared with only a single byte in it. // We have to add pathconf("dir", _PC_NAME_MAX) more bytes. // MAXNAMELEN is the largest possible value returned from pathconf. result += MAXNAMELEN; +#else + // Don't rely on sizeof(dirent->d_name) or NAME_MAX + 1, there's several filesystems where the filename size is larger than 255 bytes. + // 1024 is what macOS uses and it can hold the largest filename we are aware of (OpenZFS). + // https://github.com/apple-oss-distributions/Libc/blob/63976b830a836a22649b806fe62e8614fe3e5555/exclave/dirent.h#L69 + // pathconf("dir", _PC_NAME_MAX) was also considered but it returned 255 on NTFS and ZFS directories, which is wrong. + result += 1024; #endif // dirent should be under 2k in size assert(result < 2048); @@ -539,7 +546,7 @@ int32_t SystemNative_ReadDirR(DIR* dir, uint8_t* buffer, int32_t bufferSize, Dir struct dirent* entry = (struct dirent*)((size_t)(buffer + dirent_alignment - 1) & ~(dirent_alignment - 1)); // check there is dirent size available at entry - if ((buffer + bufferSize) < ((uint8_t*)entry + sizeof(struct dirent))) + if ((buffer + bufferSize) < ((uint8_t*)entry + SystemNative_GetReadDirRBufferSize())) { assert(false && "Buffer size too small; use GetReadDirRBufferSize to get required buffer size"); return ERANGE; From fdf2aea68d8ff21cba64d4b881fb829434446703 Mon Sep 17 00:00:00 2001 From: Jozkee Date: Thu, 12 Jun 2025 22:16:48 -0500 Subject: [PATCH 2/2] Handle filenames that wouldn't fit in a 256 decoding buffer --- .../src/Interop/Unix/System.Native/Interop.ReadDir.cs | 11 +++++++---- .../src/System/IO/Enumeration/FileSystemEntry.Unix.cs | 9 ++++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ReadDir.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ReadDir.cs index 013287e5ee0c07..86633a4414696f 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ReadDir.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ReadDir.cs @@ -40,10 +40,13 @@ internal ReadOnlySpan GetName(Span buffer) Debug.Assert(nameBytes.Length > 0, "we shouldn't have gotten a garbage value from the OS"); - int charCount = Encoding.UTF8.GetChars(nameBytes, buffer); - ReadOnlySpan value = buffer.Slice(0, charCount); - Debug.Assert(!value.Contains('\0'), "should not have embedded nulls"); - return value; + ReadOnlySpan result = !Encoding.UTF8.TryGetChars(nameBytes, buffer, out int charsWritten) + ? Encoding.UTF8.GetString(nameBytes) // Fallback to allocation since this is a rare case + : buffer.Slice(0, charsWritten); + + Debug.Assert(!result.Contains('\0'), "should not have embedded nulls"); + + return result; } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs index 96fd73796840d7..fc543a72dcf91a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs @@ -11,7 +11,10 @@ namespace System.IO.Enumeration /// public unsafe ref partial struct FileSystemEntry { - private const int DecodedNameMaxLength = 256; + // This value originated from NAME_MAX + 1 and was used as the size for DirectoryEntry.Name (d_name). + // It was later revealed that some filesystems can have filenames larger than that; however, + // it's still a reasonable size for a decoding buffer. If it's not enough, we fall back to allocating a string. + private const int DecodedNameBufferLength = 256; private Interop.Sys.DirectoryEntry _directoryEntry; private bool _isDirectory; private FileStatus _status; @@ -23,7 +26,7 @@ public unsafe ref partial struct FileSystemEntry // Wrap the fixed buffer to workaround visibility issues in api compat verification private struct FileNameBuffer { - internal fixed char _buffer[DecodedNameMaxLength]; + internal fixed char _buffer[DecodedNameBufferLength]; } internal static FileAttributes Initialize( @@ -96,7 +99,7 @@ public ReadOnlySpan FileName { if (_directoryEntry.NameLength != 0 && _fileName.Length == 0) { - Span buffer = MemoryMarshal.CreateSpan(ref _fileNameBuffer._buffer[0], DecodedNameMaxLength); + Span buffer = MemoryMarshal.CreateSpan(ref _fileNameBuffer._buffer[0], DecodedNameBufferLength); _fileName = _directoryEntry.GetName(buffer); }