mkdir: create directories atomically with correct permissions (#10036)

* mkdir: create directories atomically with correct permissions

Fix #10022: mkdir -m MODE was creating directories with umask-based
permissions first, then calling chmod afterward. This left a brief
window where the directory existed with wrong permissions.

Now we match GNU mkdir behavior by temporarily setting umask to 0
before the mkdir syscall, passing the exact requested mode to the
kernel, then restoring the original umask. The directory is created
atomically with the correct permissions.

Before (two syscalls, race condition):
  mkdir("dir", 0777)  -> created with 0755 (umask applied)
  chmod("dir", 0700)  -> fixed afterward

After (single syscall, atomic):
  umask(0)
  mkdir("dir", 0700)  -> created with exact mode
  umask(original)

Also added tests to verify -m MODE bypasses umask correctly.

* mkdir: add UmaskGuard for panic-safe umask restoration

Add RAII guard to ensure umask is restored even on panic. Encapsulate
unsafe umask calls in UmaskGuard::set() for a safe interface.

* mkdir: fix clippy and spelling warnings

- Add RAII to spell-checker ignore list
- Narrow chmod cfg to Linux only (only used for ACL bits)

* mkdir: fix unused import on non-Linux

FromIo is only used in chmod, which is now Linux-only.

Author: rynewang

src/uu/mkdir/src/mkdir.rs

@@ -3,14 +3,14 @@
 // 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
+// spell-checker:ignore (ToDO) ugoa cmode RAII
 
 use clap::builder::ValueParser;
 use clap::parser::ValuesRef;
 use clap::{Arg, ArgAction, ArgMatches, Command};
 use std::ffi::OsString;
 use std::path::{Path, PathBuf};
-#[cfg(not(windows))]
+#[cfg(all(unix, target_os = "linux"))]
 use uucore::error::FromIo;
 use uucore::error::{UResult, USimpleError};
 use uucore::translate;
@@ -191,7 +191,8 @@ pub fn mkdir(path: &Path, config: &Config) -> UResult<()> {
     create_dir(path, false, config)
 }
 
-#[cfg(any(unix, target_os = "redox"))]
+/// Only needed on Linux to add ACL permission bits after directory creation.
+#[cfg(all(unix, target_os = "linux"))]
 fn chmod(path: &Path, mode: u32) -> UResult<()> {
     use std::fs::{Permissions, set_permissions};
     use std::os::unix::fs::PermissionsExt;
@@ -201,12 +202,6 @@ fn chmod(path: &Path, mode: u32) -> UResult<()> {
     )
 }
 
-#[cfg(windows)]
-fn chmod(_path: &Path, _mode: u32) -> UResult<()> {
-    // chmod on Windows only sets the readonly flag, which isn't even honored on directories
-    Ok(())
-}
-
 // Create a directory at the given path.
 // Uses iterative approach instead of recursion to avoid stack overflow with deep nesting.
 fn create_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> {
@@ -250,13 +245,67 @@ 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(uucore::libc::mode_t);
+
+#[cfg(unix)]
+impl UmaskGuard {
+    /// Set umask to the given value and return a guard that restores the original on drop.
+    fn set(new_mask: uucore::libc::mode_t) -> Self {
+        let old_mask = unsafe { uucore::libc::umask(new_mask) };
+        Self(old_mask)
+    }
+}
+
+#[cfg(unix)]
+impl Drop for UmaskGuard {
+    fn drop(&mut self) {
+        unsafe {
+            uucore::libc::umask(self.0);
+        }
+    }
+}
+
+/// Create a directory with the exact mode specified, bypassing umask.
+///
+/// GNU mkdir temporarily sets umask to 0 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) -> std::io::Result<()> {
+    use std::os::unix::fs::DirBuilderExt;
+
+    // Temporarily set umask to 0 so the directory is created with the exact mode.
+    // The guard restores the original umask on drop, even if we panic.
+    let _guard = UmaskGuard::set(0);
+
+    std::fs::DirBuilder::new().mode(mode).create(path)
+}
+
+#[cfg(not(unix))]
+fn create_dir_with_mode(path: &Path, _mode: u32) -> std::io::Result<()> {
+    std::fs::create_dir(path)
+}
+
 // Helper function to create a single directory with appropriate permissions
 // `is_parent` argument is not used on windows
 #[allow(unused_variables)]
 fn create_single_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> {
     let path_exists = path.exists();
 
-    match std::fs::create_dir(path) {
+    // Calculate the mode to use for directory creation
+    #[cfg(unix)]
+    let create_mode = if is_parent {
+        // For parent directories with -p, use umask-derived mode with u+wx
+        (!mode::get_umask() & 0o777) | 0o300
+    } else {
+        config.mode
+    };
+    #[cfg(not(unix))]
+    let create_mode = config.mode;
+
+    match create_dir_with_mode(path, create_mode) {
         Ok(()) => {
             if config.verbose {
                 println!(
@@ -265,30 +314,17 @@ fn create_single_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<(
                 );
             }
 
+            // On Linux, we may need to add ACL permission bits via chmod.
+            // On other Unix systems, the directory was already created with the correct mode.
             #[cfg(all(unix, target_os = "linux"))]
-            let new_mode = if path_exists {
-                config.mode
-            } else {
+            if !path_exists {
                 // TODO: Make this macos and freebsd compatible by creating a function to get permission bits from
                 // acl in extended attributes
                 let acl_perm_bits = uucore::fsxattr::get_acl_perm_bits_from_xattr(path);
-
-                if is_parent {
-                    (!mode::get_umask() & 0o777) | 0o300 | acl_perm_bits
-                } else {
-                    config.mode | acl_perm_bits
+                if acl_perm_bits != 0 {
+                    chmod(path, create_mode | acl_perm_bits)?;
                 }
-            };
-            #[cfg(all(unix, not(target_os = "linux")))]
-            let new_mode = if is_parent {
-                (!mode::get_umask() & 0o777) | 0o300
-            } else {
-                config.mode
-            };
-            #[cfg(windows)]
-            let new_mode = config.mode;
-
-            chmod(path, new_mode)?;
+            }
 
             // Apply SELinux context if requested
             #[cfg(feature = "selinux")]

tests/by-util/test_mkdir.rs

@@ -788,6 +788,126 @@ fn test_mkdir_environment_expansion() {
     }
 }
 
+/// Test that mkdir -m creates directories with the exact requested mode,
+/// bypassing umask. This verifies the fix for issue #10022.
+///
+/// Previously, mkdir would create the directory with umask-based permissions
+/// and then chmod afterward, leaving a brief window with wrong permissions.
+/// Now it temporarily sets umask to 0 and creates with the exact mode.
+#[cfg(not(windows))]
+#[test]
+fn test_mkdir_mode_ignores_umask() {
+    // Test that -m 0700 with restrictive umask still creates 0700
+    {
+        let (at, mut ucmd) = at_and_ucmd!();
+        let restrictive_umask: mode_t = 0o077; // Would normally block group/other
+
+        ucmd.arg("-m")
+            .arg("0700")
+            .arg("test_700")
+            .umask(restrictive_umask)
+            .succeeds();
+
+        let perms = at.metadata("test_700").permissions().mode() as mode_t;
+        assert_eq!(perms, 0o40700, "Expected 0700, got {:o}", perms & 0o777);
+    }
+
+    // Test that -m 0777 is honored even with umask 022
+    // This is the key test: without the fix, 0777 & ~022 = 0755
+    {
+        let (at, mut ucmd) = at_and_ucmd!();
+        let common_umask: mode_t = 0o022;
+
+        ucmd.arg("-m")
+            .arg("0777")
+            .arg("test_777")
+            .umask(common_umask)
+            .succeeds();
+
+        let perms = at.metadata("test_777").permissions().mode() as mode_t;
+        assert_eq!(
+            perms,
+            0o40777,
+            "Expected 0777 (umask should be ignored with -m), got {:o}",
+            perms & 0o777
+        );
+    }
+
+    // Test that -m 0755 with umask 077 still creates 0755
+    {
+        let (at, mut ucmd) = at_and_ucmd!();
+        let very_restrictive_umask: mode_t = 0o077;
+
+        ucmd.arg("-m")
+            .arg("0755")
+            .arg("test_755")
+            .umask(very_restrictive_umask)
+            .succeeds();
+
+        let perms = at.metadata("test_755").permissions().mode() as mode_t;
+        assert_eq!(perms, 0o40755, "Expected 0755, got {:o}", perms & 0o777);
+    }
+
+    // Test symbolic mode also ignores umask
+    {
+        let (at, mut ucmd) = at_and_ucmd!();
+        let umask: mode_t = 0o022;
+
+        ucmd.arg("-m")
+            .arg("a=rwx")
+            .arg("test_symbolic")
+            .umask(umask)
+            .succeeds();
+
+        let perms = at.metadata("test_symbolic").permissions().mode() as mode_t;
+        assert_eq!(perms, 0o40777, "Expected 0777, got {:o}", perms & 0o777);
+    }
+}
+
+/// Test that mkdir -p -m applies mode correctly:
+/// - Parent directories use umask-derived permissions (with u+wx)
+/// - Final directory uses the exact requested mode (ignoring umask)
+#[cfg(not(windows))]
+#[test]
+fn test_mkdir_parent_mode_with_explicit_mode() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    let umask: mode_t = 0o022;
+
+    ucmd.arg("-p")
+        .arg("-m")
+        .arg("0700")
+        .arg("parent/child/target")
+        .umask(umask)
+        .succeeds();
+
+    // Parent directories created by -p use umask-derived mode with u+wx
+    let parent_perms = at.metadata("parent").permissions().mode() as mode_t;
+    let expected_parent = ((!umask & 0o777) | 0o300) + 0o40000;
+    assert_eq!(
+        parent_perms,
+        expected_parent,
+        "Parent should have umask-derived mode, got {:o}",
+        parent_perms & 0o777
+    );
+
+    let child_perms = at.metadata("parent/child").permissions().mode() as mode_t;
+    assert_eq!(
+        child_perms,
+        expected_parent,
+        "Intermediate dir should have umask-derived mode, got {:o}",
+        child_perms & 0o777
+    );
+
+    // Final directory should have exactly the requested mode
+    let target_perms = at.metadata("parent/child/target").permissions().mode() as mode_t;
+    assert_eq!(
+        target_perms,
+        0o40700,
+        "Target should have exact requested mode 0700, got {:o}",
+        target_perms & 0o777
+    );
+}
+
 #[test]
 fn test_mkdir_concurrent_creation() {
     // Test concurrent mkdir -p operations: 10 iterations, 8 threads, 40 levels nesting