Correct some `Credentials` behavior

kernel/src/process/credentials/credentials_.rs

@@ -76,10 +76,6 @@ impl Credentials_ {
         }
     }
 
-    fn is_privileged(&self) -> bool {
-        self.euid.is_root()
-    }
-
     //  ******* Uid methods *******
 
     pub(super) fn ruid(&self) -> Uid {
@@ -98,39 +94,25 @@ impl Credentials_ {
         self.fsuid.load(Ordering::Relaxed)
     }
 
-    pub(super) fn set_uid(&self, uid: Uid) {
+    pub(super) fn set_uid(&self, uid: Uid) -> Result<()> {
-        if self.is_privileged() {
+        if self.effective_capset().contains(CapSet::SETUID) {
             self.set_resuid_unchecked(Some(uid), Some(uid), Some(uid));
+            Ok(())
         } else {
-            // Unprivileged processes can only switch between ruid, euid, suid
+            self.set_resuid(None, Some(uid), None)
-            if uid != self.ruid.load(Ordering::Relaxed)
-                && uid != self.euid.load(Ordering::Relaxed)
-                && uid != self.suid.load(Ordering::Relaxed)
-            {
-                // No permission to set to this UID
-                return;
-            }
-            self.set_resuid_unchecked(None, Some(uid), None)
         }
-
-        self.set_fsuid_unchecked(uid)
     }
 
     pub(super) fn set_reuid(&self, ruid: Option<Uid>, euid: Option<Uid>) -> Result<()> {
         self.check_uid_perm(ruid.as_ref(), euid.as_ref(), None, false)?;
 
         let should_set_suid = ruid.is_some() || euid.is_some_and(|euid| euid != self.ruid());
-
+        let suid = if should_set_suid {
-        self.set_resuid_unchecked(ruid, euid, None);
+            Some(euid.unwrap_or_else(|| self.euid()))
-
+        } else {
-        if should_set_suid {
+            None
-            self.suid.store(self.euid(), Ordering::Release);
+        };
-        }
+        self.set_resuid_unchecked(ruid, euid, suid);
-
-        // FIXME: should we set fsuid here? The linux document for syscall `setfsuid` is contradictory
-        // with the document of syscall `setreuid`. The `setfsuid` document says the `fsuid` is always
-        // the same as `euid`, but `setreuid` does not mention the `fsuid` should be set.
-        self.set_fsuid_unchecked(self.euid());
 
         Ok(())
     }
@@ -145,28 +127,24 @@ impl Credentials_ {
 
         self.set_resuid_unchecked(ruid, euid, suid);
 
-        self.set_fsuid_unchecked(self.euid());
-
         Ok(())
     }
 
-    pub(super) fn set_fsuid(&self, fsuid: Option<Uid>) -> Result<Uid> {
+    pub(super) fn set_fsuid(&self, fsuid: Option<Uid>) -> core::result::Result<Uid, Uid> {
         let old_fsuid = self.fsuid();
 
         let Some(fsuid) = fsuid else {
             return Ok(old_fsuid);
         };
 
-        if self.is_privileged() {
+        if self.effective_capset().contains(CapSet::SETUID) {
-            self.fsuid.store(fsuid, Ordering::Release);
+            self.set_fsuid_unchecked(fsuid);
             return Ok(old_fsuid);
         }
 
         if fsuid != self.ruid() && fsuid != self.euid() && fsuid != self.suid() {
-            return_errno_with_message!(
+            // The new filesystem UID is not one of the associated UIDs.
-                Errno::EPERM,
+            return Err(old_fsuid);
-                "fsuid can only be one of old ruid, old euid and old suid."
-            )
         }
 
         self.set_fsuid_unchecked(fsuid);
@@ -191,7 +169,7 @@ impl Credentials_ {
         suid: Option<&Uid>,
         ruid_may_be_old_suid: bool,
     ) -> Result<()> {
-        if self.is_privileged() {
+        if self.effective_capset().contains(CapSet::SETUID) {
             return Ok(());
         }
 
@@ -257,45 +235,43 @@ impl Credentials_ {
             old_suid
         };
 
+        self.set_fsuid_unchecked(new_euid);
+
         // If the `SECBIT_NO_SETUID_FIXUP` bit is set, do not adjust capabilities.
         // Reference: The "SECBIT_NO_SETUID_FIXUP" section in
-        // https://man7.org/linux/man-pages/man7/capabilities.7.html
+        // <https://man7.org/linux/man-pages/man7/capabilities.7.html>.
         if self.securebits().no_setuid_fixup() {
             return;
         }
 
         // Begin to adjust capabilities.
         // Reference: The "Effect of user ID changes on capabilities" section in
-        // https://man7.org/linux/man-pages/man7/capabilities.7.html
+        // <https://man7.org/linux/man-pages/man7/capabilities.7.html>.
+
         let had_root = old_ruid.is_root() || old_euid.is_root() || old_suid.is_root();
         let all_nonroot = !new_ruid.is_root() && !new_euid.is_root() && !new_suid.is_root();
+        if had_root && all_nonroot && !self.keep_capabilities() {
+            self.set_permitted_capset(CapSet::empty());
+            self.set_inheritable_capset(CapSet::empty());
+            // TODO: Clear ambient capabilities when we support it. Note that ambient capabilities
+            // should be cleared even if `keep_capabilities` is true.
+        }
 
-        if had_root && all_nonroot {
+        if old_euid.is_root() && !new_euid.is_root() {
-            if !self.keep_capabilities() {
-                self.set_permitted_capset(CapSet::empty());
-                self.set_inheritable_capset(CapSet::empty());
-                // TODO: Also need to clear ambient capabilities when we support it
-            }
-
             self.set_effective_capset(CapSet::empty());
-        } else {
+        } else if !old_euid.is_root() && new_euid.is_root() {
-            if old_euid.is_root() && !new_euid.is_root() {
+            let permitted = self.permitted_capset();
-                self.set_effective_capset(CapSet::empty());
+            self.set_effective_capset(permitted);
-            }
-
-            if !old_euid.is_root() && new_euid.is_root() {
-                let permitted = self.permitted_capset();
-                self.set_effective_capset(permitted);
-            }
         }
     }
 
     fn set_fsuid_unchecked(&self, fsuid: Uid) {
-        let old_uid = self.fsuid.swap(fsuid, Ordering::Relaxed);
+        let old_fsuid = self.fsuid();
+        self.fsuid.store(fsuid, Ordering::Relaxed);
 
-        if old_uid.is_root() && !fsuid.is_root() {
+        if old_fsuid.is_root() && !fsuid.is_root() {
             // Reference: The "Effect of user ID changes on capabilities" section in
-            // https://man7.org/linux/man-pages/man7/capabilities.7.html
+            // <https://man7.org/linux/man-pages/man7/capabilities.7.html>.
             let cap_to_remove = CapSet::CHOWN
                 | CapSet::DAC_OVERRIDE
                 | CapSet::FOWNER
@@ -327,15 +303,12 @@ impl Credentials_ {
         self.fsgid.load(Ordering::Relaxed)
     }
 
-    pub(super) fn set_gid(&self, gid: Gid) {
+    pub(super) fn set_gid(&self, gid: Gid) -> Result<()> {
-        if self.is_privileged() {
+        if self.effective_capset().contains(CapSet::SETGID) {
-            self.rgid.store(gid, Ordering::Relaxed);
+            self.set_resgid_unchecked(Some(gid), Some(gid), Some(gid));
-            self.egid.store(gid, Ordering::Relaxed);
+            Ok(())
-            self.sgid.store(gid, Ordering::Relaxed);
-            self.fsgid.store(gid, Ordering::Relaxed);
         } else {
-            self.egid.store(gid, Ordering::Relaxed);
+            self.set_resgid(None, Some(gid), None)
-            self.fsgid.store(gid, Ordering::Relaxed);
         }
     }
 
@@ -343,14 +316,12 @@ impl Credentials_ {
         self.check_gid_perm(rgid.as_ref(), egid.as_ref(), None, false)?;
 
         let should_set_sgid = rgid.is_some() || egid.is_some_and(|egid| egid != self.rgid());
-
+        let sgid = if should_set_sgid {
-        self.set_resgid_unchecked(rgid, egid, None);
+            Some(egid.unwrap_or_else(|| self.egid()))
-
+        } else {
-        if should_set_sgid {
+            None
-            self.sgid.store(self.egid(), Ordering::Relaxed);
+        };
-        }
+        self.set_resgid_unchecked(rgid, egid, sgid);
-
-        self.fsgid.store(self.egid(), Ordering::Relaxed);
 
         Ok(())
     }
@@ -365,41 +336,41 @@ impl Credentials_ {
 
         self.set_resgid_unchecked(rgid, egid, sgid);
 
-        self.fsgid.store(self.egid(), Ordering::Relaxed);
-
         Ok(())
     }
 
-    pub(super) fn set_fsgid(&self, fsgid: Option<Gid>) -> Result<Gid> {
+    pub(super) fn set_fsgid(&self, fsgid: Option<Gid>) -> core::result::Result<Gid, Gid> {
         let old_fsgid = self.fsgid();
 
         let Some(fsgid) = fsgid else {
             return Ok(old_fsgid);
         };
 
-        if self.is_privileged() {
+        if fsgid == old_fsgid {
-            self.fsgid.store(fsgid, Ordering::Relaxed);
+            return Ok(old_fsgid);
+        }
+
+        if self.effective_capset().contains(CapSet::SETGID) {
+            self.set_fsgid_unchecked(fsgid);
             return Ok(old_fsgid);
         }
 
         if fsgid != self.rgid() && fsgid != self.egid() && fsgid != self.sgid() {
-            return_errno_with_message!(
+            // The new filesystem GID is not one of the associated GIDs.
-                Errno::EPERM,
+            return Err(old_fsgid);
-                "fsuid can only be one of old ruid, old euid and old suid."
-            )
         }
 
-        self.fsgid.store(fsgid, Ordering::Relaxed);
+        self.set_fsgid_unchecked(fsgid);
 
         Ok(old_fsgid)
     }
 
     pub(super) fn set_egid(&self, egid: Gid) {
-        self.egid.store(egid, Ordering::Relaxed);
+        self.set_resgid_unchecked(None, Some(egid), None);
     }
 
     pub(super) fn set_sgid(&self, sgid: Gid) {
-        self.sgid.store(sgid, Ordering::Relaxed);
+        self.set_resgid_unchecked(None, None, Some(sgid));
     }
 
     // For `setregid`, rgid can *NOT* be set to old sgid,
@@ -411,7 +382,7 @@ impl Credentials_ {
         sgid: Option<&Gid>,
         rgid_may_be_old_sgid: bool,
     ) -> Result<()> {
-        if self.is_privileged() {
+        if self.effective_capset().contains(CapSet::SETGID) {
             return Ok(());
         }
 
@@ -463,6 +434,12 @@ impl Credentials_ {
         if let Some(sgid) = sgid {
             self.sgid.store(sgid, Ordering::Relaxed);
         }
+
+        self.set_fsgid_unchecked(self.egid());
+    }
+
+    fn set_fsgid_unchecked(&self, fsuid: Gid) {
+        self.fsgid.store(fsuid, Ordering::Relaxed);
     }
 
     //  ******* Supplementary groups methods *******

kernel/src/process/credentials/static_cap.rs

@@ -82,8 +82,8 @@ impl<R: TRights> Credentials<R> {
     ///
     /// This method requires the `Write` right.
     #[require(R > Write)]
-    pub fn set_uid(&self, uid: Uid) {
+    pub fn set_uid(&self, uid: Uid) -> Result<()> {
-        self.0.set_uid(uid);
+        self.0.set_uid(uid)
     }
 
     /// Sets real, effective user ids as `ruid`, `euid` respectively. if `ruid` or `euid`
@@ -114,7 +114,7 @@ impl<R: TRights> Credentials<R> {
     ///
     /// This method requires the `Write` right.
     #[require(R > Write)]
-    pub fn set_fsuid(&self, fsuid: Option<Uid>) -> Result<Uid> {
+    pub fn set_fsuid(&self, fsuid: Option<Uid>) -> core::result::Result<Uid, Uid> {
         self.0.set_fsuid(fsuid)
     }
 
@@ -176,8 +176,8 @@ impl<R: TRights> Credentials<R> {
     ///
     /// This method requires the `Write` right.
     #[require(R > Write)]
-    pub fn set_gid(&self, gid: Gid) {
+    pub fn set_gid(&self, gid: Gid) -> Result<()> {
-        self.0.set_gid(gid);
+        self.0.set_gid(gid)
     }
 
     /// Sets real, effective group ids as `rgid`, `egid` respectively. if `rgid` or `egid`
@@ -208,7 +208,7 @@ impl<R: TRights> Credentials<R> {
     ///
     /// This method requires the `Write` right.
     #[require(R > Write)]
-    pub fn set_fsgid(&self, fsgid: Option<Gid>) -> Result<Gid> {
+    pub fn set_fsgid(&self, fsgid: Option<Gid>) -> core::result::Result<Gid, Gid> {
         self.0.set_fsgid(fsgid)
     }
 

kernel/src/process/credentials/user.rs

@@ -56,14 +56,8 @@ define_atomic_version_of_integer_like_type!(Uid, {
     pub(super) struct AtomicUid(AtomicU32);
 });
 
-impl AtomicUid {
-    pub fn is_root(&self) -> bool {
-        self.load(Ordering::Acquire).is_root()
-    }
-}
-
 impl Clone for AtomicUid {
     fn clone(&self) -> Self {
-        Self::new(self.load(Ordering::Acquire))
+        Self::new(self.load(Ordering::Relaxed))
     }
 }

kernel/src/syscall/setfsgid.rs

@@ -4,17 +4,19 @@ use super::SyscallReturn;
 use crate::{prelude::*, process::Gid};
 
 pub fn sys_setfsgid(gid: i32, ctx: &Context) -> Result<SyscallReturn> {
-    debug!("gid = {}", gid);
+    let fsgid = if gid >= 0 {
-
+        Some(Gid::new(gid.cast_unsigned()))
-    let fsgid = if gid < 0 {
-        None
     } else {
-        Some(Gid::new(gid as u32))
+        None
     };
 
+    debug!("fsgid = {:?}", fsgid);
+
     let old_fsgid = {
         let credentials = ctx.posix_thread.credentials_mut();
-        credentials.set_fsgid(fsgid)?
+        credentials
+            .set_fsgid(fsgid)
+            .unwrap_or_else(|old_fsgid| old_fsgid)
     };
 
     Ok(SyscallReturn::Return(

kernel/src/syscall/setfsuid.rs

@@ -4,17 +4,19 @@ use super::SyscallReturn;
 use crate::{prelude::*, process::Uid};
 
 pub fn sys_setfsuid(uid: i32, ctx: &Context) -> Result<SyscallReturn> {
-    debug!("uid = {}", uid);
+    let fsuid = if uid >= 0 {
-
+        Some(Uid::new(uid.cast_unsigned()))
-    let fsuid = if uid < 0 {
-        None
     } else {
-        Some(Uid::new(uid as u32))
+        None
     };
 
+    debug!("fsuid = {:?}", fsuid);
+
     let old_fsuid = {
         let credentials = ctx.posix_thread.credentials_mut();
-        credentials.set_fsuid(fsuid)?
+        credentials
+            .set_fsuid(fsuid)
+            .unwrap_or_else(|old_fsuid| old_fsuid)
     };
 
     Ok(SyscallReturn::Return(

kernel/src/syscall/setgid.rs

@@ -4,16 +4,15 @@ use super::SyscallReturn;
 use crate::{prelude::*, process::Gid};
 
 pub fn sys_setgid(gid: i32, ctx: &Context) -> Result<SyscallReturn> {
-    debug!("gid = {}", gid);
-
     if gid < 0 {
-        return_errno_with_message!(Errno::EINVAL, "gid cannot be negative");
+        return_errno_with_message!(Errno::EINVAL, "GIDs cannot be negative");
     }
 
-    let gid = Gid::new(gid as u32);
+    let gid = Gid::new(gid.cast_unsigned());
+    debug!("gid = {:?}", gid);
 
     let credentials = ctx.posix_thread.credentials_mut();
-    credentials.set_gid(gid);
+    credentials.set_gid(gid)?;
 
     Ok(SyscallReturn::Return(0))
 }

kernel/src/syscall/setregid.rs

@@ -4,20 +4,20 @@ use super::SyscallReturn;
 use crate::{prelude::*, process::Gid};
 
 pub fn sys_setregid(rgid: i32, egid: i32, ctx: &Context) -> Result<SyscallReturn> {
-    debug!("rgid = {}, egid = {}", rgid, egid);
+    let rgid = if rgid >= 0 {
-
+        Some(Gid::new(rgid.cast_unsigned()))
-    let rgid = if rgid > 0 {
-        Some(Gid::new(rgid as u32))
     } else {
         None
     };
 
-    let egid = if egid > 0 {
+    let egid = if egid >= 0 {
-        Some(Gid::new(egid as u32))
+        Some(Gid::new(egid.cast_unsigned()))
     } else {
         None
     };
 
+    debug!("rgid = {:?}, egid = {:?}", rgid, egid);
+
     let credentials = ctx.posix_thread.credentials_mut();
     credentials.set_regid(rgid, egid)?;
 

kernel/src/syscall/setresgid.rs

@@ -4,20 +4,20 @@ use super::SyscallReturn;
 use crate::{prelude::*, process::Gid};
 
 pub fn sys_setresgid(rgid: i32, egid: i32, sgid: i32, ctx: &Context) -> Result<SyscallReturn> {
-    let rgid = if rgid > 0 {
+    let rgid = if rgid >= 0 {
-        Some(Gid::new(rgid as u32))
+        Some(Gid::new(rgid.cast_unsigned()))
     } else {
         None
     };
 
-    let egid = if egid > 0 {
+    let egid = if egid >= 0 {
-        Some(Gid::new(egid as u32))
+        Some(Gid::new(egid.cast_unsigned()))
     } else {
         None
     };
 
-    let sgid = if sgid > 0 {
+    let sgid = if sgid >= 0 {
-        Some(Gid::new(sgid as u32))
+        Some(Gid::new(sgid.cast_unsigned()))
     } else {
         None
     };

kernel/src/syscall/setresuid.rs

@@ -4,20 +4,20 @@ use super::SyscallReturn;
 use crate::{prelude::*, process::Uid};
 
 pub fn sys_setresuid(ruid: i32, euid: i32, suid: i32, ctx: &Context) -> Result<SyscallReturn> {
-    let ruid = if ruid > 0 {
+    let ruid = if ruid >= 0 {
-        Some(Uid::new(ruid as u32))
+        Some(Uid::new(ruid.cast_unsigned()))
     } else {
         None
     };
 
-    let euid = if euid > 0 {
+    let euid = if euid >= 0 {
-        Some(Uid::new(euid as u32))
+        Some(Uid::new(euid.cast_unsigned()))
     } else {
         None
     };
 
-    let suid = if suid > 0 {
+    let suid = if suid >= 0 {
-        Some(Uid::new(suid as u32))
+        Some(Uid::new(suid.cast_unsigned()))
     } else {
         None
     };
@@ -25,7 +25,6 @@ pub fn sys_setresuid(ruid: i32, euid: i32, suid: i32, ctx: &Context) -> Result<S
     debug!("ruid = {:?}, euid = {:?}, suid = {:?}", ruid, euid, suid);
 
     let credentials = ctx.posix_thread.credentials_mut();
-
     credentials.set_resuid(ruid, euid, suid)?;
 
     Ok(SyscallReturn::Return(0))

kernel/src/syscall/setreuid.rs

@@ -4,20 +4,20 @@ use super::SyscallReturn;
 use crate::{prelude::*, process::Uid};
 
 pub fn sys_setreuid(ruid: i32, euid: i32, ctx: &Context) -> Result<SyscallReturn> {
-    debug!("ruid = {}, euid = {}", ruid, euid);
+    let ruid = if ruid >= 0 {
-
+        Some(Uid::new(ruid.cast_unsigned()))
-    let ruid = if ruid > 0 {
-        Some(Uid::new(ruid as u32))
     } else {
         None
     };
 
-    let euid = if euid > 0 {
+    let euid = if euid >= 0 {
-        Some(Uid::new(euid as u32))
+        Some(Uid::new(euid.cast_unsigned()))
     } else {
         None
     };
 
+    debug!("ruid = {:?}, euid = {:?}", ruid, euid);
+
     let credentials = ctx.posix_thread.credentials_mut();
     credentials.set_reuid(ruid, euid)?;
 

kernel/src/syscall/setuid.rs

@@ -4,16 +4,15 @@ use super::SyscallReturn;
 use crate::{prelude::*, process::Uid};
 
 pub fn sys_setuid(uid: i32, ctx: &Context) -> Result<SyscallReturn> {
-    debug!("uid = {}", uid);
-
     if uid < 0 {
-        return_errno_with_message!(Errno::EINVAL, "uid cannot be negative");
+        return_errno_with_message!(Errno::EINVAL, "UIDs cannot be negative");
     }
 
-    let uid = Uid::new(uid as u32);
+    let uid = Uid::new(uid.cast_unsigned());
+    debug!("uid = {:?}", uid);
 
     let credentials = ctx.posix_thread.credentials_mut();
-    credentials.set_uid(uid);
+    credentials.set_uid(uid)?;
 
     Ok(SyscallReturn::Return(0))
 }