mkdir: create directories atomically with correct permissions

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