Skip to content

Commit 712e45c

Browse files
tmdstmds
committed
FileStatus.Unix/Process.Unix: align implementation.
Process: remove the user identity caching and extend the logic to avoid retrieving the identity in most cases by checking if all x-bits are set or not set. FileStatus: use same group check as Process. FileStatus: cache the read only flag instead of caching the identity.
1 parent 8275c37 commit 712e45c

File tree

10 files changed

+120
-107
lines changed

10 files changed

+120
-107
lines changed

‎src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetEGid.cs‎

Lines changed: 0 additions & 14 deletions
This file was deleted.

‎src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetUid.cs‎

Lines changed: 0 additions & 13 deletions
This file was deleted.

‎src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetGroups.cs‎ renamed to ‎src/libraries/Common/src/Interop/Unix/System.Native/Interop.IsMemberOfGroup.cs‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,26 @@ internal static partial class Interop
88
{
99
internal static partial class Sys
1010
{
11-
internal static unsafe uint[]? GetGroups()
11+
internal static bool IsMemberOfGroup(uint gid)
12+
{
13+
if (gid == GetEGid())
14+
{
15+
return true;
16+
}
17+
18+
uint[]? groups = GetGroups();
19+
if (groups == null)
20+
{
21+
return false;
22+
}
23+
24+
return Array.IndexOf(groups, gid) >= 0;
25+
}
26+
27+
[GeneratedDllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetEGid")]
28+
private static partial uint GetEGid();
29+
30+
private static unsafe uint[]? GetGroups()
1231
{
1332
const int InitialGroupsLength =
1433
#if DEBUG

‎src/libraries/Native/Unix/System.Native/entrypoints.c‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,6 @@ static const Entry s_sysNative[] =
244244
DllImportEntry(SystemNative_GetEGid)
245245
DllImportEntry(SystemNative_SetEUid)
246246
DllImportEntry(SystemNative_GetGroupList)
247-
DllImportEntry(SystemNative_GetUid)
248247
DllImportEntry(SystemNative_CreateAutoreleasePool)
249248
DllImportEntry(SystemNative_DrainAutoreleasePool)
250249
DllImportEntry(SystemNative_iOSSupportVersion)

‎src/libraries/Native/Unix/System.Native/pal_uid.c‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,6 @@ int32_t SystemNative_SetEUid(uint32_t euid)
102102
return seteuid(euid);
103103
}
104104

105-
uint32_t SystemNative_GetUid()
106-
{
107-
return getuid();
108-
}
109-
110105
#ifdef USE_GROUPLIST_LOCK
111106
static pthread_mutex_t s_groupLock = PTHREAD_MUTEX_INITIALIZER;
112107
#endif

‎src/libraries/Native/Unix/System.Native/pal_uid.h‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,6 @@ PALEXPORT int32_t SystemNative_SetEUid(uint32_t euid);
7474
*/
7575
PALEXPORT int32_t SystemNative_GetGroupList(const char* name, uint32_t group, uint32_t* groups, int32_t* ngroups);
7676

77-
/**
78-
* Gets and returns the real user's identity.
79-
* Implemented as shim to getuid(2).
80-
*
81-
* Always succeeds.
82-
*/
83-
PALEXPORT uint32_t SystemNative_GetUid(void);
84-
8577
/**
8678
* Gets groups associated with current process.
8779
*

‎src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -251,8 +251,6 @@
251251
Link="Common\Interop\Unix\Interop.GetSetPriority.cs" />
252252
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetSid.cs"
253253
Link="Common\Interop\Unix\Interop.GetSid.cs" />
254-
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetUid.cs"
255-
Link="Common\Interop\Unix\Interop.GetUid.cs" />
256254
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.InitializeTerminalAndSignalHandling.cs"
257255
Link="Common\Interop\Unix\Interop.InitializeTerminalAndSignalHandling.cs" />
258256
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Kill.cs"
@@ -281,10 +279,8 @@
281279
Link="Common\Interop\Unix\Interop.Permissions.cs" />
282280
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEUid.cs"
283281
Link="Common\Interop\Unix\Interop.GetEUid.cs" />
284-
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEGid.cs"
285-
Link="Common\Interop\Unix\Interop.GetEGid.cs" />
286-
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetGroups.cs"
287-
Link="Common\Interop\Linux\Interop.GetGroups.cs" />
282+
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.IsMemberOfGroup.cs"
283+
Link="Common\Interop\Unix\Interop.IsMemberOfGroup.cs" />
288284
</ItemGroup>
289285
<ItemGroup Condition=" '$(TargetsUnix)' == 'true' and '$(IsiOSLike)' != 'true'">
290286
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.ConfigureTerminalForChildProcess.cs"

‎src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@ namespace System.Diagnostics
1717
public partial class Process : IDisposable
1818
{
1919
private static volatile bool s_initialized;
20-
private static uint s_euid;
21-
private static uint s_egid;
22-
private static uint[]? s_groups;
2320
private static readonly object s_initializedGate = new object();
2421
private static readonly ReaderWriterLockSlim s_processStartLock = new ReaderWriterLockSlim();
2522

@@ -769,29 +766,49 @@ private static bool IsExecutable(string fullPath)
769766
return false;
770767
}
771768

772-
Interop.Sys.Permissions permissions = (Interop.Sys.Permissions)fileinfo.Mode;
769+
Interop.Sys.Permissions permissions = ((Interop.Sys.Permissions)fileinfo.Mode) & Interop.Sys.Permissions.S_IXUGO;
773770

774-
if (s_euid == 0)
771+
// Avoid checking user/group when permission.
772+
if (permissions == Interop.Sys.Permissions.S_IXUGO)
773+
{
774+
return true;
775+
}
776+
else if (permissions == 0)
777+
{
778+
return false;
779+
}
780+
781+
uint euid = Interop.Sys.GetEUid();
782+
783+
if (euid == 0)
775784
{
776785
// We're root.
777-
return (permissions & Interop.Sys.Permissions.S_IXUGO) != 0;
786+
return true;
778787
}
779788

780-
if (s_euid == fileinfo.Uid)
789+
if (euid == fileinfo.Uid)
781790
{
782791
// We own the file.
783792
return (permissions & Interop.Sys.Permissions.S_IXUSR) != 0;
784793
}
785794

786-
if (s_egid == fileinfo.Gid ||
787-
(s_groups != null && Array.BinarySearch(s_groups, fileinfo.Gid) >= 0))
795+
bool groupCanExecute = (permissions & Interop.Sys.Permissions.S_IXGRP) != 0;
796+
bool otherCanExecute = (permissions & Interop.Sys.Permissions.S_IXOTH) != 0;
797+
798+
// Avoid group check when group and other have same permissions.
799+
if (groupCanExecute == otherCanExecute)
788800
{
789-
// A group we're a member of owns the file.
790-
return (permissions & Interop.Sys.Permissions.S_IXGRP) != 0;
801+
return groupCanExecute;
791802
}
792803

793-
// Other.
794-
return (permissions & Interop.Sys.Permissions.S_IXOTH) != 0;
804+
if (Interop.Sys.IsMemberOfGroup(fileinfo.Gid))
805+
{
806+
return groupCanExecute;
807+
}
808+
else
809+
{
810+
return otherCanExecute;
811+
}
795812
}
796813

797814
private static long s_ticksPerSecond;
@@ -1063,14 +1080,6 @@ private static unsafe void EnsureInitialized()
10631080
throw new Win32Exception();
10641081
}
10651082

1066-
s_euid = Interop.Sys.GetEUid();
1067-
s_egid = Interop.Sys.GetEGid();
1068-
s_groups = Interop.Sys.GetGroups();
1069-
if (s_groups != null)
1070-
{
1071-
Array.Sort(s_groups);
1072-
}
1073-
10741083
// Register our callback.
10751084
Interop.Sys.RegisterForSigChld(&OnSigChild);
10761085
SetDelayedSigChildConsoleConfigurationHandler();

‎src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1969,9 +1969,6 @@
19691969
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetDefaultTimeZone.Android.cs" Condition="'$(TargetsAndroid)' == 'true'">
19701970
<Link>Common\Interop\Unix\System.Native\Interop.GetDefaultTimeZone.Android.cs</Link>
19711971
</Compile>
1972-
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEGid.cs">
1973-
<Link>Common\Interop\Unix\System.Native\Interop.GetEGid.cs</Link>
1974-
</Compile>
19751972
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetHostName.cs">
19761973
<Link>Common\Interop\Unix\System.Native\Interop.GetHostName.cs</Link>
19771974
</Compile>
@@ -2153,9 +2150,10 @@
21532150
<Compile Include="$(MSBuildThisFileDirectory)System\IO\FileStatus.SetTimes.OtherUnix.cs" />
21542151
</ItemGroup>
21552152
<ItemGroup Condition="'$(TargetsUnix)' == 'true'">
2156-
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEUid.cs">
2157-
<Link>Common\Interop\Unix\System.Native\Interop.GetEUid.cs</Link>
2158-
</Compile>
2153+
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEUid.cs"
2154+
Link="Common\Interop\Unix\Interop.GetEUid.cs" />
2155+
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.IsMemberOfGroup.cs"
2156+
Link="Common\Interop\Unix\Interop.IsMemberOfGroup.cs" />
21592157
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetPwUid.cs">
21602158
<Link>Common\Interop\Unix\System.Native\Interop.GetPwUid.cs</Link>
21612159
</Compile>

‎src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs‎

Lines changed: 64 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,6 @@ internal partial struct FileStatus
3030
// Exists as of the last refresh
3131
private bool _exists;
3232

33-
#if !TARGET_BROWSER
34-
// Caching the euid/egid to avoid extra p/invokes
35-
// Should get reset on refresh
36-
private uint? _euid;
37-
private uint? _egid;
38-
#endif
39-
4033
private bool IsFileCacheInitialized => _initializedFileCache == 0;
4134

4235
// Check if the main path (without following symlinks) has the hidden attribute set
@@ -58,36 +51,76 @@ internal bool HasReadOnlyFlag
5851
{
5952
return false;
6053
}
54+
6155
#if TARGET_BROWSER
62-
const Interop.Sys.Permissions readBit = Interop.Sys.Permissions.S_IRUSR;
63-
const Interop.Sys.Permissions writeBit = Interop.Sys.Permissions.S_IWUSR;
56+
var mode = (Interop.Sys.Permissions)(_fileCache.Mode & (int)Interop.Sys.Permissions.Mask);
57+
bool isUserReadOnly = (mode & Interop.Sys.Permissions.S_IRUSR) != 0 && // has read permission
58+
(mode & Interop.Sys.Permissions.S_IWUSR) == 0; // but not write permission
59+
return isUserReadOnly;
6460
#else
65-
Interop.Sys.Permissions readBit, writeBit;
66-
67-
if (_fileCache.Uid == (_euid ??= Interop.Sys.GetEUid()))
61+
if (_isReadOnlyCache == 0)
6862
{
69-
// User effectively owns the file
70-
readBit = Interop.Sys.Permissions.S_IRUSR;
71-
writeBit = Interop.Sys.Permissions.S_IWUSR;
63+
return false;
7264
}
73-
else if (_fileCache.Gid == (_egid ??= Interop.Sys.GetEGid()))
65+
else if (_isReadOnlyCache == 1)
7466
{
75-
// User belongs to a group that effectively owns the file
76-
readBit = Interop.Sys.Permissions.S_IRGRP;
77-
writeBit = Interop.Sys.Permissions.S_IWGRP;
67+
return true;
7868
}
7969
else
8070
{
81-
// Others permissions
82-
readBit = Interop.Sys.Permissions.S_IROTH;
83-
writeBit = Interop.Sys.Permissions.S_IWOTH;
71+
bool isReadOnly = IsModeReadOnlyCore();
72+
_isReadOnlyCache = isReadOnly ? 1 : 0;
73+
return isReadOnly;
8474
}
8575
#endif
76+
}
77+
}
78+
79+
#if !TARGET_BROWSER
80+
// HasReadOnlyFlag cache.
81+
private int _isReadOnlyCache;
82+
83+
private bool IsModeReadOnlyCore()
84+
{
85+
var mode = (Interop.Sys.Permissions)(_fileCache.Mode & (int)Interop.Sys.Permissions.Mask);
86+
87+
bool isUserReadOnly = (mode & Interop.Sys.Permissions.S_IRUSR) != 0 && // has read permission
88+
(mode & Interop.Sys.Permissions.S_IWUSR) == 0; // but not write permission
89+
bool isGroupReadOnly = (mode & Interop.Sys.Permissions.S_IRGRP) != 0 && // has read permission
90+
(mode & Interop.Sys.Permissions.S_IWGRP) == 0; // but not write permission
91+
bool isOtherReadOnly = (mode & Interop.Sys.Permissions.S_IROTH) != 0 && // has read permission
92+
(mode & Interop.Sys.Permissions.S_IWOTH) == 0; // but not write permission
93+
94+
// If they are all the same, no need to check user/group.
95+
if ((isUserReadOnly == isGroupReadOnly) && (isGroupReadOnly == isOtherReadOnly))
96+
{
97+
return isUserReadOnly;
98+
}
99+
100+
if (_fileCache.Uid == Interop.Sys.GetEUid())
101+
{
102+
// User owns the file.
103+
return isUserReadOnly;
104+
}
105+
106+
// System files often have the same permissions for group and other (umask 022).
107+
if (isGroupReadOnly == isUserReadOnly)
108+
{
109+
return isGroupReadOnly;
110+
}
86111

87-
return (_fileCache.Mode & (int)readBit) != 0 && // has read permission
88-
(_fileCache.Mode & (int)writeBit) == 0; // but not write permission
112+
if (Interop.Sys.IsMemberOfGroup(_fileCache.Gid))
113+
{
114+
// User belongs to group that owns the file.
115+
return isGroupReadOnly;
116+
}
117+
else
118+
{
119+
// Other permissions.
120+
return isOtherReadOnly;
89121
}
90122
}
123+
#endif
91124

92125
// Checks if the main path is a symbolic link
93126
// Only call if Refresh has been successfully called at least once
@@ -373,12 +406,6 @@ internal void RefreshCaches(ReadOnlySpan<char> path)
373406
_isDirectory = CacheHasDirectoryFlag(target);
374407
}
375408

376-
#if !TARGET_BROWSER
377-
// These are cached and used when retrieving the read-only attribute
378-
_euid = null;
379-
_egid = null;
380-
#endif
381-
382409
_exists = true;
383410

384411
// Checks if the specified cache has the directory attribute set
@@ -422,8 +449,13 @@ private static long UnixTimeSecondsToNanoseconds(DateTimeOffset time, long secon
422449
return (time.UtcDateTime.Ticks - DateTimeOffset.UnixEpoch.Ticks - seconds * TicksPerSecond) * NanosecondsPerTick;
423450
}
424451

425-
private bool TryRefreshFileCache(ReadOnlySpan<char> path) =>
426-
VerifyStatCall(Interop.Sys.LStat(path, out _fileCache), out _initializedFileCache);
452+
private bool TryRefreshFileCache(ReadOnlySpan<char> path)
453+
{
454+
#if !TARGET_BROWSER
455+
_isReadOnlyCache = -1;
456+
#endif
457+
return VerifyStatCall(Interop.Sys.LStat(path, out _fileCache), out _initializedFileCache);
458+
}
427459

428460
// Receives the return value of a stat or lstat call.
429461
// If the call is unsuccessful, sets the initialized parameter to a positive number representing the last error info.

0 commit comments

Comments
 (0)