Skip to content

Commit

Permalink
FileStatus.Unix/Process.Unix: align caching of user identity. (#60160)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
tmds committed Nov 18, 2021
1 parent 5181d12 commit 80ca504
Show file tree
Hide file tree
Showing 10 changed files with 112 additions and 111 deletions.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,13 @@ internal static partial class Interop
{
internal static partial class Sys
{
internal static unsafe uint[]? GetGroups()
internal static unsafe bool IsMemberOfGroup(uint gid)
{
if (gid == GetEGid())
{
return true;
}

const int InitialGroupsLength =
#if DEBUG
1;
Expand All @@ -28,7 +33,7 @@ internal static partial class Sys
if (rv >= 0)
{
// success
return groups.Slice(0, rv).ToArray();
return groups.Slice(0, rv).IndexOf(gid) >= 0;
}
else if (rv == -1 && Interop.Sys.GetLastError() == Interop.Error.EINVAL)
{
Expand All @@ -37,13 +42,16 @@ internal static partial class Sys
}
else
{
// failure
return null;
// failure (unexpected)
return false;
}
}
while (true);
}

[GeneratedDllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetEGid")]
private static partial uint GetEGid();

[GeneratedDllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetGroups", SetLastError = true)]
private static unsafe partial int GetGroups(int ngroups, uint* groups);
}
Expand Down
1 change: 0 additions & 1 deletion src/libraries/Native/Unix/System.Native/entrypoints.c
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,6 @@ static const Entry s_sysNative[] =
DllImportEntry(SystemNative_GetEGid)
DllImportEntry(SystemNative_SetEUid)
DllImportEntry(SystemNative_GetGroupList)
DllImportEntry(SystemNative_GetUid)
DllImportEntry(SystemNative_CreateAutoreleasePool)
DllImportEntry(SystemNative_DrainAutoreleasePool)
DllImportEntry(SystemNative_iOSSupportVersion)
Expand Down
5 changes: 0 additions & 5 deletions src/libraries/Native/Unix/System.Native/pal_uid.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,6 @@ int32_t SystemNative_SetEUid(uint32_t euid)
return seteuid(euid);
}

uint32_t SystemNative_GetUid()
{
return getuid();
}

#ifdef USE_GROUPLIST_LOCK
static pthread_mutex_t s_groupLock = PTHREAD_MUTEX_INITIALIZER;
#endif
Expand Down
8 changes: 0 additions & 8 deletions src/libraries/Native/Unix/System.Native/pal_uid.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,6 @@ PALEXPORT int32_t SystemNative_SetEUid(uint32_t euid);
*/
PALEXPORT int32_t SystemNative_GetGroupList(const char* name, uint32_t group, uint32_t* groups, int32_t* ngroups);

/**
* Gets and returns the real user's identity.
* Implemented as shim to getuid(2).
*
* Always succeeds.
*/
PALEXPORT uint32_t SystemNative_GetUid(void);

/**
* Gets groups associated with current process.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,6 @@
Link="Common\Interop\Unix\Interop.GetSetPriority.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetSid.cs"
Link="Common\Interop\Unix\Interop.GetSid.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetUid.cs"
Link="Common\Interop\Unix\Interop.GetUid.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.InitializeTerminalAndSignalHandling.cs"
Link="Common\Interop\Unix\Interop.InitializeTerminalAndSignalHandling.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Kill.cs"
Expand Down Expand Up @@ -281,10 +279,8 @@
Link="Common\Interop\Unix\Interop.Permissions.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEUid.cs"
Link="Common\Interop\Unix\Interop.GetEUid.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEGid.cs"
Link="Common\Interop\Unix\Interop.GetEGid.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetGroups.cs"
Link="Common\Interop\Linux\Interop.GetGroups.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.IsMemberOfGroup.cs"
Link="Common\Interop\Unix\Interop.IsMemberOfGroup.cs" />
</ItemGroup>
<ItemGroup Condition=" '$(TargetsUnix)' == 'true' and '$(IsiOSLike)' != 'true'">
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.ConfigureTerminalForChildProcess.cs"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ namespace System.Diagnostics
public partial class Process : IDisposable
{
private static volatile bool s_initialized;
private static uint s_euid;
private static uint s_egid;
private static uint[]? s_groups;
private static readonly object s_initializedGate = new object();
private static readonly ReaderWriterLockSlim s_processStartLock = new ReaderWriterLockSlim();

Expand Down Expand Up @@ -769,29 +766,48 @@ private static bool IsExecutable(string fullPath)
return false;
}

Interop.Sys.Permissions permissions = (Interop.Sys.Permissions)fileinfo.Mode;
Interop.Sys.Permissions permissions = ((Interop.Sys.Permissions)fileinfo.Mode) & Interop.Sys.Permissions.S_IXUGO;

if (s_euid == 0)
// Avoid checking user/group when permission.
if (permissions == Interop.Sys.Permissions.S_IXUGO)
{
// We're root.
return (permissions & Interop.Sys.Permissions.S_IXUGO) != 0;
return true;
}
else if (permissions == 0)
{
return false;
}

uint euid = Interop.Sys.GetEUid();

if (s_euid == fileinfo.Uid)
if (euid == 0)
{
return true; // We're root.
}

if (euid == fileinfo.Uid)
{
// We own the file.
return (permissions & Interop.Sys.Permissions.S_IXUSR) != 0;
}

if (s_egid == fileinfo.Gid ||
(s_groups != null && Array.BinarySearch(s_groups, fileinfo.Gid) >= 0))
bool groupCanExecute = (permissions & Interop.Sys.Permissions.S_IXGRP) != 0;
bool otherCanExecute = (permissions & Interop.Sys.Permissions.S_IXOTH) != 0;

// Avoid group check when group and other have same permissions.
if (groupCanExecute == otherCanExecute)
{
// A group we're a member of owns the file.
return (permissions & Interop.Sys.Permissions.S_IXGRP) != 0;
return groupCanExecute;
}

// Other.
return (permissions & Interop.Sys.Permissions.S_IXOTH) != 0;
if (Interop.Sys.IsMemberOfGroup(fileinfo.Gid))
{
return groupCanExecute;
}
else
{
return otherCanExecute;
}
}

private static long s_ticksPerSecond;
Expand Down Expand Up @@ -1063,14 +1079,6 @@ private static unsafe void EnsureInitialized()
throw new Win32Exception();
}

s_euid = Interop.Sys.GetEUid();
s_egid = Interop.Sys.GetEGid();
s_groups = Interop.Sys.GetGroups();
if (s_groups != null)
{
Array.Sort(s_groups);
}

// Register our callback.
Interop.Sys.RegisterForSigChld(&OnSigChild);
SetDelayedSigChildConsoleConfigurationHandler();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1969,9 +1969,6 @@
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetDefaultTimeZone.Android.cs" Condition="'$(TargetsAndroid)' == 'true'">
<Link>Common\Interop\Unix\System.Native\Interop.GetDefaultTimeZone.Android.cs</Link>
</Compile>
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEGid.cs">
<Link>Common\Interop\Unix\System.Native\Interop.GetEGid.cs</Link>
</Compile>
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetHostName.cs">
<Link>Common\Interop\Unix\System.Native\Interop.GetHostName.cs</Link>
</Compile>
Expand Down Expand Up @@ -2153,9 +2150,10 @@
<Compile Include="$(MSBuildThisFileDirectory)System\IO\FileStatus.SetTimes.OtherUnix.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetsUnix)' == 'true'">
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEUid.cs">
<Link>Common\Interop\Unix\System.Native\Interop.GetEUid.cs</Link>
</Compile>
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEUid.cs"
Link="Common\Interop\Unix\Interop.GetEUid.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.IsMemberOfGroup.cs"
Link="Common\Interop\Unix\Interop.IsMemberOfGroup.cs" />
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetPwUid.cs">
<Link>Common\Interop\Unix\System.Native\Interop.GetPwUid.cs</Link>
</Compile>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,6 @@ internal partial struct FileStatus
// Exists as of the last refresh
private bool _exists;

#if !TARGET_BROWSER
// Caching the euid/egid to avoid extra p/invokes
// Should get reset on refresh
private uint? _euid;
private uint? _egid;
#endif

private bool IsFileCacheInitialized => _initializedFileCache == 0;

// Check if the main path (without following symlinks) has the hidden attribute set
Expand All @@ -58,36 +51,76 @@ internal bool HasReadOnlyFlag
{
return false;
}

#if TARGET_BROWSER
const Interop.Sys.Permissions readBit = Interop.Sys.Permissions.S_IRUSR;
const Interop.Sys.Permissions writeBit = Interop.Sys.Permissions.S_IWUSR;
var mode = (Interop.Sys.Permissions)(_fileCache.Mode & (int)Interop.Sys.Permissions.Mask);
bool isUserReadOnly = (mode & Interop.Sys.Permissions.S_IRUSR) != 0 && // has read permission
(mode & Interop.Sys.Permissions.S_IWUSR) == 0; // but not write permission
return isUserReadOnly;
#else
Interop.Sys.Permissions readBit, writeBit;

if (_fileCache.Uid == (_euid ??= Interop.Sys.GetEUid()))
if (_isReadOnlyCache == 0)
{
// User effectively owns the file
readBit = Interop.Sys.Permissions.S_IRUSR;
writeBit = Interop.Sys.Permissions.S_IWUSR;
return false;
}
else if (_fileCache.Gid == (_egid ??= Interop.Sys.GetEGid()))
else if (_isReadOnlyCache == 1)
{
// User belongs to a group that effectively owns the file
readBit = Interop.Sys.Permissions.S_IRGRP;
writeBit = Interop.Sys.Permissions.S_IWGRP;
return true;
}
else
{
// Others permissions
readBit = Interop.Sys.Permissions.S_IROTH;
writeBit = Interop.Sys.Permissions.S_IWOTH;
bool isReadOnly = IsModeReadOnlyCore();
_isReadOnlyCache = isReadOnly ? 1 : 0;
return isReadOnly;
}
#endif
}
}

#if !TARGET_BROWSER
// HasReadOnlyFlag cache.
private int _isReadOnlyCache;

private bool IsModeReadOnlyCore()
{
var mode = (Interop.Sys.Permissions)(_fileCache.Mode & (int)Interop.Sys.Permissions.Mask);

bool isUserReadOnly = (mode & Interop.Sys.Permissions.S_IRUSR) != 0 && // has read permission
(mode & Interop.Sys.Permissions.S_IWUSR) == 0; // but not write permission
bool isGroupReadOnly = (mode & Interop.Sys.Permissions.S_IRGRP) != 0 && // has read permission
(mode & Interop.Sys.Permissions.S_IWGRP) == 0; // but not write permission
bool isOtherReadOnly = (mode & Interop.Sys.Permissions.S_IROTH) != 0 && // has read permission
(mode & Interop.Sys.Permissions.S_IWOTH) == 0; // but not write permission

// If they are all the same, no need to check user/group.
if ((isUserReadOnly == isGroupReadOnly) && (isGroupReadOnly == isOtherReadOnly))
{
return isUserReadOnly;
}

if (_fileCache.Uid == Interop.Sys.GetEUid())
{
// User owns the file.
return isUserReadOnly;
}

// System files often have the same permissions for group and other (umask 022).
if (isGroupReadOnly == isUserReadOnly)
{
return isGroupReadOnly;
}

return (_fileCache.Mode & (int)readBit) != 0 && // has read permission
(_fileCache.Mode & (int)writeBit) == 0; // but not write permission
if (Interop.Sys.IsMemberOfGroup(_fileCache.Gid))
{
// User belongs to group that owns the file.
return isGroupReadOnly;
}
else
{
// Other permissions.
return isOtherReadOnly;
}
}
#endif

// Checks if the main path is a symbolic link
// Only call if Refresh has been successfully called at least once
Expand Down Expand Up @@ -376,12 +409,6 @@ internal void RefreshCaches(ReadOnlySpan<char> path)
_isDirectory = CacheHasDirectoryFlag(target);
}

#if !TARGET_BROWSER
// These are cached and used when retrieving the read-only attribute
_euid = null;
_egid = null;
#endif

_exists = true;

// Checks if the specified cache has the directory attribute set
Expand Down Expand Up @@ -425,8 +452,13 @@ private static long UnixTimeSecondsToNanoseconds(DateTimeOffset time, long secon
return (time.UtcDateTime.Ticks - DateTimeOffset.UnixEpoch.Ticks - seconds * TicksPerSecond) * NanosecondsPerTick;
}

private bool TryRefreshFileCache(ReadOnlySpan<char> path) =>
VerifyStatCall(Interop.Sys.LStat(path, out _fileCache), out _initializedFileCache);
private bool TryRefreshFileCache(ReadOnlySpan<char> path)
{
#if !TARGET_BROWSER
_isReadOnlyCache = -1;
#endif
return VerifyStatCall(Interop.Sys.LStat(path, out _fileCache), out _initializedFileCache);
}

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

0 comments on commit 80ca504

Please sign in to comment.