install: create ancestor directories at 0755 regardless of umask

src/uu/install/src/install.rs

@@ -37,7 +37,7 @@ use uucore::translate;
 use uucore::{format_usage, show, show_error, show_if_err};
 
 #[cfg(unix)]
-use std::os::unix::fs::MetadataExt;
+use std::os::unix::fs::{DirBuilderExt, MetadataExt};
 #[cfg(unix)]
 use std::os::unix::prelude::OsStrExt;
 
@@ -205,10 +205,12 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
 
     let behavior = behavior(&matches, diag_args.as_deref())?;
 
-    match behavior.main_function {
+    // Like GNU, work with the umask cleared: new ancestor directories get
+    // exactly DEFAULT_MODE and the strip program sees a umask of 0.
+    uucore::mode::with_umask(0, || match behavior.main_function {
         MainFunction::Directory => directory(&paths, &behavior),
         MainFunction::Standard => standard(paths, &behavior),
-    }
+    })
 }
 
 pub fn uu_app() -> Command {
@@ -506,11 +508,11 @@ fn directory(paths: &[OsString], b: &Behavior) -> UResult<()> {
             // create all ancestors (or components) of a directory
             // regardless of the presence of the "-D" flag.
             //
-            // NOTE: the GNU "install" sets the expected mode only for the
-            // target directory. All created ancestor directories will have
-            // the default mode. Hence it is safe to use fs::create_dir_all
-            // and then only modify the target's dir mode.
-            if let Err(e) = fs::create_dir_all(&path_to_create).map_err_context(
+            // Only the target gets the requested mode; newly created ancestors
+            // use DEFAULT_MODE, which create_dir_all would request as 0777.
+            let mut builder = fs::DirBuilder::new();
+            builder.recursive(true).mode(DEFAULT_MODE);
+            if let Err(e) = builder.create(&path_to_create).map_err_context(
                 || translate!("install-error-create-dir-failed", "path" => path_to_create.quote()),
             ) {
                 show!(e);
@@ -713,8 +715,6 @@ fn standard(mut paths: Vec<OsString>, b: &Behavior) -> UResult<()> {
 
                 #[cfg(unix)]
                 {
-                    // Use DEFAULT_MODE (0o755) for created directories - this matches GNU install
-                    // behavior. The actual mode will be modified by umask at the kernel level.
                     match create_dir_all_safe(to_create, DEFAULT_MODE) {
                         Ok(dir_fd) => {
                             if b.target_dir.is_none()

src/uu/mkdir/Cargo.toml

@@ -21,9 +21,6 @@ clap = { workspace = true }
 fluent = { workspace = true }
 uucore = { workspace = true, features = ["fs", "fsxattr", "mode"] }
 
-[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;
@@ -245,43 +245,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))]
@@ -295,22 +271,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/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

@@ -355,6 +355,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
@@ -365,6 +382,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);
@@ -387,6 +408,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 {
 
@@ -526,4 +588,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);
+    }
 }