From 10908994caf755a236c9d3d0fed2dfa199630eb9 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 18 May 2022 13:41:22 +0200 Subject: [PATCH 1/6] disregard the upper bytes, fixes #61175 --- src/native/libs/System.Native/pal_io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index d196cbf33970f1..d659120e2b5cb0 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -1480,7 +1480,7 @@ int64_t SystemNative_GetFileSystemType(intptr_t fd) // for our needs (get file system type) statfs is always enough and there is no need to use statfs64 // which got deprecated in macOS 10.6, in favor of statfs while ((statfsRes = fstatfs(ToFileDescriptor(fd), &statfsArgs)) == -1 && errno == EINTR) ; - return statfsRes == -1 ? (int64_t)-1 : (int64_t)statfsArgs.f_type; + return statfsRes == -1 ? (int64_t)-1 : (int64_t)((uint64_t)statfsArgs.f_type ^ 0xFFFFFFFF00000000); // disregard the upper bytes #61175 #elif !HAVE_NON_LEGACY_STATFS int statfsRes; struct statvfs statfsArgs; From ca72260d985154305a85e559095b2ce5e3d78fee Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 18 May 2022 13:42:04 +0200 Subject: [PATCH 2/6] add assert to verify the values --- .../Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs index 984ca44c35c023..03557e7aef24a5 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; +using System.Diagnostics; using System.Runtime.InteropServices; using Microsoft.Win32.SafeHandles; @@ -152,6 +154,7 @@ internal static bool TryGetFileSystemType(SafeFileHandle fd, out UnixFileSystemT { long fstatfsResult = GetFileSystemType(fd); fileSystemType = (UnixFileSystemTypes)fstatfsResult; + Debug.Assert(Enum.IsDefined(fileSystemType) || fstatfsResult == -1, $"GetFileSystemType returned {fstatfsResult}"); return fstatfsResult != -1; } } From cb3017e0aa6f5144c7f5f5ecb511f1d4addb96e2 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 18 May 2022 16:53:02 +0200 Subject: [PATCH 3/6] address code review feedback: use uint32 --- .../Unix/System.Native/Interop.UnixFileSystemTypes.cs | 10 +++++----- src/native/libs/System.Native/pal_io.c | 10 +++++----- src/native/libs/System.Native/pal_io.h | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs index 03557e7aef24a5..5d37d650c8f5d0 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs @@ -18,7 +18,7 @@ internal static partial class Sys /// where this enum must be a subset of the GetDriveType list, with the enum /// values here exactly matching a string there. /// - internal enum UnixFileSystemTypes : long + internal enum UnixFileSystemTypes : uint { adfs = 0xADF5, affs = 0xADFF, @@ -148,14 +148,14 @@ internal enum UnixFileSystemTypes : long } [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetFileSystemType")] - private static partial long GetFileSystemType(SafeFileHandle fd); + private static partial uint GetFileSystemType(SafeFileHandle fd); internal static bool TryGetFileSystemType(SafeFileHandle fd, out UnixFileSystemTypes fileSystemType) { - long fstatfsResult = GetFileSystemType(fd); + uint fstatfsResult = GetFileSystemType(fd); fileSystemType = (UnixFileSystemTypes)fstatfsResult; - Debug.Assert(Enum.IsDefined(fileSystemType) || fstatfsResult == -1, $"GetFileSystemType returned {fstatfsResult}"); - return fstatfsResult != -1; + Debug.Assert(Enum.IsDefined(fileSystemType) || fstatfsResult == 0, $"GetFileSystemType returned {fstatfsResult}"); + return fstatfsResult != 0; } } } diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index d659120e2b5cb0..fcfcdc5c89de77 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -1472,7 +1472,7 @@ static int16_t ConvertLockType(int16_t managedLockType) } } -int64_t SystemNative_GetFileSystemType(intptr_t fd) +uint32_t SystemNative_GetFileSystemType(intptr_t fd) { #if HAVE_STATFS_VFS || HAVE_STATFS_MOUNT int statfsRes; @@ -1480,14 +1480,14 @@ int64_t SystemNative_GetFileSystemType(intptr_t fd) // for our needs (get file system type) statfs is always enough and there is no need to use statfs64 // which got deprecated in macOS 10.6, in favor of statfs while ((statfsRes = fstatfs(ToFileDescriptor(fd), &statfsArgs)) == -1 && errno == EINTR) ; - return statfsRes == -1 ? (int64_t)-1 : (int64_t)((uint64_t)statfsArgs.f_type ^ 0xFFFFFFFF00000000); // disregard the upper bytes #61175 + return statfsRes == -1 ? 0 : (uint32_t)statfsArgs.f_type; // disregard the upper bytes #61175 #elif !HAVE_NON_LEGACY_STATFS int statfsRes; struct statvfs statfsArgs; while ((statfsRes = fstatvfs(ToFileDescriptor(fd), &statfsArgs)) == -1 && errno == EINTR) ; - if (statfsRes == -1) return (int64_t)-1; + if (statfsRes == -1) return 0; - int64_t result = -1; + uint32_t result = 0; if (strcmp(statfsArgs.f_basetype, "adfs") == 0) result = 0xADF5; else if (strcmp(statfsArgs.f_basetype, "affs") == 0) result = 0xADFF; @@ -1613,7 +1613,7 @@ int64_t SystemNative_GetFileSystemType(intptr_t fd) else if (strcmp(statfsArgs.f_basetype, "udev") == 0) result = 0x01021994; else if (strcmp(statfsArgs.f_basetype, "zfs") == 0) result = 0x2FC12FC1; - assert(result != -1); + assert(result != 0); return result; #else #error "Platform doesn't support fstatfs or fstatvfs" diff --git a/src/native/libs/System.Native/pal_io.h b/src/native/libs/System.Native/pal_io.h index 720d7623fee79f..30dc534635c4f6 100644 --- a/src/native/libs/System.Native/pal_io.h +++ b/src/native/libs/System.Native/pal_io.h @@ -739,9 +739,9 @@ PALEXPORT char* SystemNative_RealPath(const char* path); PALEXPORT int32_t SystemNative_GetPeerID(intptr_t socket, uid_t* euid); /** -* Returns file system type on success, or -1 on error. +* Returns file system type on success, or 0 on error. */ -PALEXPORT int64_t SystemNative_GetFileSystemType(intptr_t fd); +PALEXPORT uint32_t SystemNative_GetFileSystemType(intptr_t fd); /** * Attempts to lock/unlock the region of the file "fd" specified by the offset and length. lockType From 2b8142082d2b7feb097691db9047c68e8cb0f975 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 18 May 2022 20:47:04 +0200 Subject: [PATCH 4/6] add apple file system (apfs) to the magic numbers list (idenitified by the new asserts) --- .../Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs | 1 + src/native/libs/System.Native/pal_io.c | 1 + 2 files changed, 2 insertions(+) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs index 5d37d650c8f5d0..23e733425748a5 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs @@ -24,6 +24,7 @@ internal enum UnixFileSystemTypes : uint affs = 0xADFF, afs = 0x5346414F, anoninode = 0x09041934, + apfs = 0x1A, aufs = 0x61756673, autofs = 0x0187, autofs4 = 0x6D4A556D, diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index fcfcdc5c89de77..455bf9edf2a0e1 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -1493,6 +1493,7 @@ uint32_t SystemNative_GetFileSystemType(intptr_t fd) else if (strcmp(statfsArgs.f_basetype, "affs") == 0) result = 0xADFF; else if (strcmp(statfsArgs.f_basetype, "afs") == 0) result = 0x5346414F; else if (strcmp(statfsArgs.f_basetype, "anoninode") == 0) result = 0x09041934; + else if (strcmp(statfsArgs.f_basetype, "apfs") == 0) result = 0x1A; else if (strcmp(statfsArgs.f_basetype, "aufs") == 0) result = 0x61756673; else if (strcmp(statfsArgs.f_basetype, "autofs") == 0) result = 0x0187; else if (strcmp(statfsArgs.f_basetype, "autofs4") == 0) result = 0x6D4A556D; From 0771bd598ad6d929c055b4d65ea172969dfa27d9 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 26 May 2022 16:04:41 +0200 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Tom Deseyn --- .../Unix/System.Native/Interop.UnixFileSystemTypes.cs | 2 +- src/native/libs/System.Native/pal_io.c | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs index 23e733425748a5..7d02ae4c405c4c 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs @@ -155,7 +155,7 @@ internal static bool TryGetFileSystemType(SafeFileHandle fd, out UnixFileSystemT { uint fstatfsResult = GetFileSystemType(fd); fileSystemType = (UnixFileSystemTypes)fstatfsResult; - Debug.Assert(Enum.IsDefined(fileSystemType) || fstatfsResult == 0, $"GetFileSystemType returned {fstatfsResult}"); + Debug.Assert(Enum.IsDefined(fileSystemType) || fstatfsResult == 0 || OperatingSystem.IsOSXLike(), $"GetFileSystemType returned {fstatfsResult}"); return fstatfsResult != 0; } } diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 455bf9edf2a0e1..f51a95b1178cf4 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -1480,7 +1480,13 @@ uint32_t SystemNative_GetFileSystemType(intptr_t fd) // for our needs (get file system type) statfs is always enough and there is no need to use statfs64 // which got deprecated in macOS 10.6, in favor of statfs while ((statfsRes = fstatfs(ToFileDescriptor(fd), &statfsArgs)) == -1 && errno == EINTR) ; - return statfsRes == -1 ? 0 : (uint32_t)statfsArgs.f_type; // disregard the upper bytes #61175 + if (statfsRes == -1) return 0; + + // On Linux, f_type is signed. This causes some filesystem types to be represented as + // negative numbers on 32-bit platforms. We cast to uint32_t to make them positive. + uint32_t result = (uint32_t)statfsArgs.f_type; + assert(result == statfsArgs.f_type); + return result; #elif !HAVE_NON_LEGACY_STATFS int statfsRes; struct statvfs statfsArgs; From 9dbfac1ecc17de44aa02db1649d1ffae69e1304c Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 26 May 2022 17:46:23 +0200 Subject: [PATCH 6/6] Apply suggestions from code review --- .../Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs | 2 +- src/native/libs/System.Native/pal_io.c | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs index 7d02ae4c405c4c..ce0dffd7372f85 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs @@ -155,7 +155,7 @@ internal static bool TryGetFileSystemType(SafeFileHandle fd, out UnixFileSystemT { uint fstatfsResult = GetFileSystemType(fd); fileSystemType = (UnixFileSystemTypes)fstatfsResult; - Debug.Assert(Enum.IsDefined(fileSystemType) || fstatfsResult == 0 || OperatingSystem.IsOSXLike(), $"GetFileSystemType returned {fstatfsResult}"); + Debug.Assert(Enum.IsDefined(fileSystemType) || fstatfsResult == 0 || !OperatingSystem.IsLinux(), $"GetFileSystemType returned {fstatfsResult}"); return fstatfsResult != 0; } } diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index f51a95b1178cf4..a721025fe688f5 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -1485,7 +1485,6 @@ uint32_t SystemNative_GetFileSystemType(intptr_t fd) // On Linux, f_type is signed. This causes some filesystem types to be represented as // negative numbers on 32-bit platforms. We cast to uint32_t to make them positive. uint32_t result = (uint32_t)statfsArgs.f_type; - assert(result == statfsArgs.f_type); return result; #elif !HAVE_NON_LEGACY_STATFS int statfsRes;