From 51aa4b019abdd61088968c4dac727c08f23df577 Mon Sep 17 00:00:00 2001 From: Nicholas Schell Date: Fri, 26 Jul 2019 11:07:38 -0700 Subject: [PATCH] [Mono.Posix] Parameters for chown should be signed integers not unsigned (#15828) Fixes https://github.com/mono/mono/issues/10748 The calls for chown, lchown, fchown, and fchownat (from `Mono.Unix.Native.Syscall`) are all declared with unsigned integers for the `owner` and `group` parameters. This is incorrect. Directly from the manpage `man 2 chown` 

...
       int chown(const char *pathname, uid_t owner, gid_t group);
       int fchown(int fd, uid_t owner, gid_t group);
       int lchown(const char *pathname, uid_t owner, gid_t group);

...

       int fchownat(int dirfd, const char *pathname,
                    uid_t owner, gid_t group, int flags);

...
...
       If the owner or group is specified as -1, then that ID is not changed.
...
Emphasis added on the documentation explaining how the parameters can take the value -1, clearly not an unsigned integer. The `uid_t` struct is not an unsigned integer. I am assuming this was just an oversite on whoever re-wrote `Mono.Unix.Native.Syscall`. The original calls from `Mono.Posix.Syscall` (now marked as Obsolete) all correctly use signed integers. ``` [DllImport ("libc", SetLastError=true)] public static extern int chown (string path, int owner, int group); [DllImport ("libc", SetLastError=true)] public static extern int lchown (string path, int owner, int group); ``` Even the documentation directly from `mcs/class/Mono.Posix/Documentation/en/Mono.Unix.Native/Syscall.xml` ``` One of the owner or group id's may be left unchanged by specifying it as -1. ``` --- external/api-snapshot | 2 +- .../Documentation/en/Mono.Unix.Native/Syscall.xml | 24 ++++++------- mcs/class/Mono.Posix/Mono.Unix.Native/Syscall.cs | 41 +++++++++++++++++++--- .../Mono.Posix/Mono.Unix/UnixFileSystemInfo.cs | 4 +-- mcs/class/Mono.Posix/Mono.Unix/UnixStream.cs | 2 +- .../Mono.Posix/Mono.Unix/UnixSymbolicLinkInfo.cs | 2 +- 6 files changed, 54 insertions(+), 21 deletions(-) diff --git a/external/api-snapshot b/external/api-snapshot index 5989f244c0d..f605f3d983b 160000 --- a/external/api-snapshot +++ b/external/api-snapshot @@ -1 +1 @@ -Subproject commit 5989f244c0dc41a8894bcb5a7bdceeb2c9c8bb56 +Subproject commit f605f3d983b225bcff429dee166cb1375d8d4869 diff --git a/mcs/class/Mono.Posix/Documentation/en/Mono.Unix.Native/Syscall.xml b/mcs/class/Mono.Posix/Documentation/en/Mono.Unix.Native/Syscall.xml index e28929ff29e..7d8ca92f4a4 100644 --- a/mcs/class/Mono.Posix/Documentation/en/Mono.Unix.Native/Syscall.xml +++ b/mcs/class/Mono.Posix/Documentation/en/Mono.Unix.Native/Syscall.xml @@ -695,8 +695,8 @@ - - + + Method 1.0.5000.0 @@ -708,8 +708,8 @@ - - + + To be added. @@ -1835,8 +1835,8 @@ - - + + Method 1.0.5000.0 @@ -1848,8 +1848,8 @@ - - + + To be added. @@ -4354,8 +4354,8 @@ - - + + Method 1.0.5000.0 @@ -4367,8 +4367,8 @@ - - + + To be added. diff --git a/mcs/class/Mono.Posix/Mono.Unix.Native/Syscall.cs b/mcs/class/Mono.Posix/Mono.Unix.Native/Syscall.cs index a919644f85d..957c3c4635e 100644 --- a/mcs/class/Mono.Posix/Mono.Unix.Native/Syscall.cs +++ b/mcs/class/Mono.Posix/Mono.Unix.Native/Syscall.cs @@ -4576,19 +4576,37 @@ namespace Mono.Unix.Native { [DllImport (LIBC, SetLastError=true)] public static extern int chown ( [MarshalAs (UnmanagedType.CustomMarshaler, MarshalTypeRef=typeof(FileNameMarshaler))] - string path, uint owner, uint group); + string path, int owner, int group); // fchown(2) // int fchown(int fd, uid_t owner, gid_t group); [DllImport (LIBC, SetLastError=true)] - public static extern int fchown (int fd, uint owner, uint group); + public static extern int fchown (int fd, int owner, int group); // lchown(2) // int lchown(const char *path, uid_t owner, gid_t group); [DllImport (LIBC, SetLastError=true)] public static extern int lchown ( [MarshalAs (UnmanagedType.CustomMarshaler, MarshalTypeRef=typeof(FileNameMarshaler))] - string path, uint owner, uint group); + string path, int owner, int group); + + #region UInt32 overloads for initial incorrect implementation + [Obsolete("Use Int32 overload")] + [DllImport (LIBC, SetLastError=true)] + public static extern int chown ( + [MarshalAs (UnmanagedType.CustomMarshaler, MarshalTypeRef=typeof(FileNameMarshaler))] + string path, uint owner, uint group); + + [Obsolete("Use Int32 overload")] + [DllImport (LIBC, SetLastError=true)] + public static extern int fchown (int fd, uint owner, uint group); + + [Obsolete("Use Int32 overload")] + [DllImport (LIBC, SetLastError=true)] + public static extern int lchown ( + [MarshalAs (UnmanagedType.CustomMarshaler, MarshalTypeRef=typeof(FileNameMarshaler))] + string path, uint owner, uint group); + #endregion [DllImport (LIBC, SetLastError=true)] public static extern int chdir ( @@ -5161,13 +5179,28 @@ namespace Mono.Unix.Native { [DllImport (LIBC, SetLastError=true, EntryPoint="fchownat")] private static extern int sys_fchownat (int dirfd, [MarshalAs (UnmanagedType.CustomMarshaler, MarshalTypeRef=typeof(FileNameMarshaler))] - string pathname, uint owner, uint group, int flags); + string pathname, int owner, int group, int flags); + + public static int fchownat (int dirfd, string pathname, int owner, int group, AtFlags flags) + { + int _flags = NativeConvert.FromAtFlags (flags); + return sys_fchownat (dirfd, pathname, owner, group, _flags); + } + #region UInt32 overloads for initial incorrect implementation + [Obsolete("Use Int32 overload")] + [DllImport (LIBC, SetLastError=true, EntryPoint="fchownat")] + private static extern int sys_fchownat (int dirfd, + [MarshalAs (UnmanagedType.CustomMarshaler, MarshalTypeRef=typeof(FileNameMarshaler))] + string pathname, uint owner, uint group, int flags); + + [Obsolete("Use Int32 overload")] public static int fchownat (int dirfd, string pathname, uint owner, uint group, AtFlags flags) { int _flags = NativeConvert.FromAtFlags (flags); return sys_fchownat (dirfd, pathname, owner, group, _flags); } + #endregion [DllImport (LIBC, SetLastError=true, EntryPoint="linkat")] private static extern int sys_linkat (int olddirfd, diff --git a/mcs/class/Mono.Posix/Mono.Unix/UnixFileSystemInfo.cs b/mcs/class/Mono.Posix/Mono.Unix/UnixFileSystemInfo.cs index 149f79b84ed..c16ce3a4cdd 100644 --- a/mcs/class/Mono.Posix/Mono.Unix/UnixFileSystemInfo.cs +++ b/mcs/class/Mono.Posix/Mono.Unix/UnixFileSystemInfo.cs @@ -325,8 +325,8 @@ namespace Mono.Unix { public virtual void SetOwner (long owner, long group) { - uint _owner = Convert.ToUInt32 (owner); - uint _group = Convert.ToUInt32 (group); + int _owner = Convert.ToInt32 (owner); + int _group = Convert.ToInt32 (group); int r = Native.Syscall.chown (FullPath, _owner, _group); UnixMarshal.ThrowExceptionForLastErrorIf (r); } diff --git a/mcs/class/Mono.Posix/Mono.Unix/UnixStream.cs b/mcs/class/Mono.Posix/Mono.Unix/UnixStream.cs index 8a94a7d19ed..f570513b2a5 100644 --- a/mcs/class/Mono.Posix/Mono.Unix/UnixStream.cs +++ b/mcs/class/Mono.Posix/Mono.Unix/UnixStream.cs @@ -356,7 +356,7 @@ namespace Mono.Unix { AssertNotDisposed (); int r = Native.Syscall.fchown (fileDescriptor, - Convert.ToUInt32 (user), Convert.ToUInt32 (group)); + Convert.ToInt32 (user), Convert.ToInt32 (group)); UnixMarshal.ThrowExceptionForLastErrorIf (r); } diff --git a/mcs/class/Mono.Posix/Mono.Unix/UnixSymbolicLinkInfo.cs b/mcs/class/Mono.Posix/Mono.Unix/UnixSymbolicLinkInfo.cs index 242a294a2a1..617eff7cf79 100644 --- a/mcs/class/Mono.Posix/Mono.Unix/UnixSymbolicLinkInfo.cs +++ b/mcs/class/Mono.Posix/Mono.Unix/UnixSymbolicLinkInfo.cs @@ -94,7 +94,7 @@ namespace Mono.Unix { public override void SetOwner (long owner, long group) { - int r = Native.Syscall.lchown (FullPath, Convert.ToUInt32 (owner), Convert.ToUInt32 (group)); + int r = Native.Syscall.lchown (FullPath, Convert.ToInt32 (owner), Convert.ToInt32 (group)); UnixMarshal.ThrowExceptionForLastErrorIf (r); } -- 2.11.4.GIT