mkfifo, mknod: set the umask through mode::with_umask

src/uu/mkdir/Cargo.toml

@@ -26,9 +26,6 @@ uucore = { workspace = true, features = [
   "quoting-style",
 ] }
 
-[target.'cfg(unix)'.dependencies]
-rustix = { workspace = true, features = ["process", "fs"] }
-
 [features]
 diagnostics = ["uucore/diagnostics"]
 selinux = ["uucore/selinux"]

src/uu/mkdir/src/mkdir.rs

@@ -3,7 +3,7 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-// spell-checker:ignore (ToDO) ugoa cmode RAII
+// spell-checker:ignore (ToDO) ugoa cmode
 
 use clap::builder::ValueParser;
 use clap::parser::ValuesRef;
@@ -251,43 +251,19 @@ fn create_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> {
     create_single_dir(path, is_parent, config)
 }
 
-/// RAII guard to restore umask on drop, ensuring cleanup even on panic.
-#[cfg(unix)]
-struct UmaskGuard(rustix::fs::Mode);
-
-#[cfg(unix)]
-impl UmaskGuard {
-    /// Set umask to the given value and return a guard that restores the original on drop.
-    fn set(new_mask: rustix::fs::Mode) -> Self {
-        let old_mask = rustix::process::umask(new_mask);
-        Self(old_mask)
-    }
-}
-
-#[cfg(unix)]
-impl Drop for UmaskGuard {
-    fn drop(&mut self) {
-        rustix::process::umask(self.0);
-    }
-}
-
 /// Create a directory with the exact mode specified, bypassing umask.
 ///
 /// GNU mkdir temporarily sets umask to a shaped umask before calling mkdir(2),
 /// ensuring the directory is created atomically with the correct permissions.
 /// This avoids a race condition where the directory briefly exists with
 /// umask-based permissions.
 #[cfg(unix)]
-fn create_dir_with_mode(
-    path: &Path,
-    mode: u32,
-    shaped_umask: rustix::fs::Mode,
-) -> std::io::Result<()> {
+fn create_dir_with_mode(path: &Path, mode: u32, shaped_umask: u32) -> std::io::Result<()> {
     use std::os::unix::fs::DirBuilderExt;
 
-    let _guard = UmaskGuard::set(shaped_umask);
-
-    std::fs::DirBuilder::new().mode(mode).create(path)
+    mode::with_umask(shaped_umask, || {
+        std::fs::DirBuilder::new().mode(mode).create(path)
+    })
 }
 
 #[cfg(not(unix))]
@@ -301,22 +277,21 @@ fn create_dir_with_mode(path: &Path, _mode: u32, _shaped_umask: u32) -> std::io:
 fn create_single_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> {
     #[cfg(unix)]
     let (mkdir_mode, shaped_umask) = {
-        let mode_bits = |x: u32| rustix::fs::Mode::from_bits_truncate(x as rustix::fs::RawMode);
-        let umask_bits = mode_bits(mode::get_umask());
+        let umask = mode::get_umask();
         if is_parent {
             // Parent directories are never affected by -m (matches GNU behavior).
             // We pass 0o777 as the mode and shape the umask so it cannot block
             // owner write or execute (u+wx), ensuring the owner can traverse and
             // write into the parent to create children. All other umask bits are
             // preserved so the kernel applies them — and any default ACL on the
             // grandparent — through the normal mkdir(2) path.
-            (DEFAULT_PERM, umask_bits & !mode_bits(0o300))
+            (DEFAULT_PERM, umask & !0o300)
         } else {
             match config.mode {
                 // Explicit -m: shape umask so it cannot block explicitly requested bits.
-                Some(m) => (m, umask_bits & !mode_bits(m)),
+                Some(m) => (m, umask & !m),
                 // No -m: leave umask fully intact; kernel applies umask + ACL naturally.
-                None => (DEFAULT_PERM, umask_bits),
+                None => (DEFAULT_PERM, umask),
             }
         }
     };

src/uu/mkfifo/Cargo.toml

@@ -22,7 +22,7 @@ fluent = { workspace = true }
 uucore = { workspace = true, features = ["fs", "mode"] }
 
 [target.'cfg(unix)'.dependencies]
-rustix = { workspace = true, features = ["fs", "process"] }
+rustix = { workspace = true, features = ["fs"] }
 
 [target.'cfg(target_vendor = "apple")'.dependencies]
 libc = { workspace = true }

src/uu/mkfifo/src/mkfifo.rs

@@ -4,8 +4,7 @@
 // file that was distributed with this source code.
 
 use clap::{Arg, ArgAction, Command, value_parser};
-use rustix::fs::{Mode, RawMode};
-use rustix::process::umask;
+use rustix::fs::RawMode;
 use std::ffi::OsString;
 use uucore::display::Quotable;
 use uucore::error::{ExitCode, UResult, USimpleError, strip_errno};
@@ -81,9 +80,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         // that used to follow this call closes the TOCTOU window an
         // attacker could use to swap the FIFO for a symlink between
         // mkfifo and chmod (issue #10020).
-        let prev_umask = umask(Mode::empty());
-        let mkfifo_result = create_fifo(f.as_str(), mode as RawMode);
-        umask(prev_umask);
+        let mkfifo_result =
+            uucore::mode::with_umask(0, || create_fifo(f.as_str(), mode as RawMode));
 
         if let Err(e) = mkfifo_result {
             show!(USimpleError::new(
@@ -156,7 +154,7 @@ pub fn uu_app() -> Command {
 #[cfg(not(target_vendor = "apple"))]
 fn create_fifo(path: &str, mode: RawMode) -> std::io::Result<()> {
     use rustix::fs;
-    fs::mkfifoat(fs::CWD, path, Mode::from_bits_truncate(mode)).map_err(Into::into)
+    fs::mkfifoat(fs::CWD, path, fs::Mode::from_bits_truncate(mode)).map_err(Into::into)
 }
 
 #[cfg(target_vendor = "apple")]

src/uu/mknod/src/mknod.rs

@@ -7,7 +7,7 @@
 
 use clap::{Arg, ArgAction, Command, value_parser};
 use nix::libc::{S_IRGRP, S_IROTH, S_IRUSR, S_IWGRP, S_IWOTH, S_IWUSR, mode_t};
-use nix::sys::stat::{Mode, SFlag, dev_t, mknod as nix_mknod, umask as nix_umask};
+use nix::sys::stat::{Mode, SFlag, dev_t, mknod as nix_mknod};
 use std::ffi::OsString;
 use std::io::{self, Write as _};
 
@@ -93,27 +93,22 @@ fn mknod(file_name: &str, config: Config) -> i32 {
         None
     };
 
-    // set umask to 0 and store previous umask
-    let have_prev_umask = if config.use_umask {
-        None
+    let create_node = || {
+        nix_mknod(
+            file_name,
+            config.file_type.as_sflag(),
+            config.mode,
+            config.dev,
+        )
+        .err()
+    };
+    let mknod_err = if config.use_umask {
+        create_node()
     } else {
-        Some(nix_umask(Mode::empty()))
+        uucore::mode::with_umask(0, create_node)
     };
-
-    let mknod_err = nix_mknod(
-        file_name,
-        config.file_type.as_sflag(),
-        config.mode,
-        config.dev,
-    )
-    .err();
     let errno = if mknod_err.is_some() { -1 } else { 0 };
 
-    // set umask back to original value
-    if let Some(prev_umask) = have_prev_umask {
-        nix_umask(prev_umask);
-    }
-
     if let Some(err) = mknod_err {
         let _ = writeln!(
             io::stderr(),

src/uu/sort/Cargo.toml

@@ -51,6 +51,7 @@ tempfile = { workspace = true }
 uucore = { workspace = true, features = [
   "benchmark",
   "fs",
+  "mode",
   "parser-size",
   "version-cmp",
   "i18n-collator",

src/uu/sort/src/tmp_dir.rs

@@ -221,36 +221,19 @@ mod tests {
         std::fs::metadata(path).unwrap().permissions().mode() & 0o777
     }
 
-    /// Restores the process umask on drop, so a panic in the test cannot leak the
-    /// value into the rest of the binary.
-    struct UmaskGuard(libc::mode_t);
-
-    impl UmaskGuard {
-        fn set(mask: libc::mode_t) -> Self {
-            // SAFETY: umask(2) has no failure mode; it returns the previous value.
-            Self(unsafe { libc::umask(mask) })
-        }
-    }
-
-    impl Drop for UmaskGuard {
-        fn drop(&mut self) {
-            unsafe { libc::umask(self.0) };
-        }
-    }
-
     #[test]
     fn tmp_files_are_private_regardless_of_umask() {
         // Pin a permissive umask: under 0077 the umask alone would produce 0700 and
-        // 0600, so the assertions would hold for a broken implementation too. The
-        // guard restores it, and the only other test here that creates files sets
-        // the modes it cares about explicitly.
-        let _umask = UmaskGuard::set(0o022);
-
-        let parent = tempfile::tempdir().unwrap();
-        let mut wrapper = TmpDirWrapper::new(parent.path().to_owned());
-        let (_file, path) = wrapper.next_file().unwrap();
-
-        assert_eq!(mode(path.parent().unwrap()), 0o700);
-        assert_eq!(mode(&path), 0o600);
+        // 0600, so the assertions would hold for a broken implementation too.
+        // `with_umask` restores it, also on a panic, and the only other test here
+        // that creates files sets the modes it cares about explicitly.
+        uucore::mode::with_umask(0o022, || {
+            let parent = tempfile::tempdir().unwrap();
+            let mut wrapper = TmpDirWrapper::new(parent.path().to_owned());
+            let (_file, path) = wrapper.next_file().unwrap();
+
+            assert_eq!(mode(path.parent().unwrap()), 0o700);
+            assert_eq!(mode(&path), 0o600);
+        });
     }
 }

src/uucore/src/lib/features/mode.rs

@@ -397,6 +397,23 @@ pub fn parse(mode_string: &str, considering_dir: bool, umask: u32) -> Result<u32
     parse_chmod(0, mode_string, considering_dir, umask)
 }
 
+#[cfg(unix)]
+static UMASK_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
+
+#[cfg(unix)]
+thread_local! {
+    /// The umask set by a [`with_umask`] running on this thread, which holds
+    /// `UMASK_LOCK`.
+    static HELD_UMASK: std::cell::Cell<Option<u32>> = const { std::cell::Cell::new(None) };
+}
+
+#[cfg(unix)]
+fn lock_umask() -> std::sync::MutexGuard<'static, ()> {
+    UMASK_LOCK
+        .lock()
+        .unwrap_or_else(std::sync::PoisonError::into_inner)
+}
+
 pub fn get_umask() -> u32 {
     // There's no portable way to read the umask without changing it.
     // We have to replace it and then quickly set it back, hopefully before
@@ -407,6 +424,10 @@ pub fn get_umask() -> u32 {
     {
         use rustix::fs::Mode;
         use rustix::process::umask;
+        if let Some(mask) = HELD_UMASK.get() {
+            return mask;
+        }
+        let _lock = lock_umask();
 
         let mask = umask(Mode::empty());
         let _ = umask(mask);
@@ -429,6 +450,47 @@ pub fn get_umask() -> u32 {
     }
 }
 
+#[cfg(unix)]
+struct UmaskGuard {
+    previous: rustix::fs::Mode,
+    held: Option<u32>,
+}
+
+#[cfg(unix)]
+impl UmaskGuard {
+    fn set(mask: u32) -> Self {
+        // `rustix::fs::RawMode` is u16 on some targets and u32 on others.
+        let mode = rustix::fs::Mode::from_bits_truncate(mask as rustix::fs::RawMode);
+        Self {
+            previous: rustix::process::umask(mode),
+            held: HELD_UMASK.replace(Some(mask & 0o777)),
+        }
+    }
+}
+
+#[cfg(unix)]
+impl Drop for UmaskGuard {
+    fn drop(&mut self) {
+        rustix::process::umask(self.previous);
+        HELD_UMASK.set(self.held);
+    }
+}
+
+/// Run an operation with a temporary process umask.
+///
+/// The previous umask is restored when the operation returns or unwinds.
+/// Calls through this module are serialized because the umask is process-wide.
+/// They nest: inside `operation`, [`get_umask`] returns `mask`, and `with_umask`
+/// can be called again. Other threads wait until `operation` returns, so it
+/// must not wait for one of them.
+#[cfg(unix)]
+pub fn with_umask<T>(mask: u32, operation: impl FnOnce() -> T) -> T {
+    // Only the outermost call on this thread takes the lock.
+    let _lock = HELD_UMASK.get().is_none().then(lock_umask);
+    let _guard = UmaskGuard::set(mask);
+    operation()
+}
+
 #[cfg(test)]
 mod tests {
 
@@ -568,4 +630,32 @@ mod tests {
         // First add user write, then set to 755 (should override)
         assert_eq!(parse("u+w,755", false, 0).unwrap(), 0o755);
     }
+
+    /// Reads the umask without going through the helpers under test.
+    #[cfg(unix)]
+    fn raw_umask() -> u32 {
+        let mask = rustix::process::umask(rustix::fs::Mode::empty());
+        rustix::process::umask(mask);
+        mask.bits() as u32
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_with_umask_sets_and_restores_mask() {
+        let before = super::get_umask();
+        assert_eq!(super::with_umask(0o027, raw_umask), 0o027);
+        assert_eq!(super::get_umask(), before);
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_with_umask_nests() {
+        let before = super::get_umask();
+        super::with_umask(0o027, || {
+            assert_eq!(super::get_umask(), 0o027);
+            assert_eq!(super::with_umask(0o077, raw_umask), 0o077);
+            assert_eq!(raw_umask(), 0o027);
+        });
+        assert_eq!(super::get_umask(), before);
+    }
 }