From 523035e9a3c1acab3c864d45f4c5c2ffdc2e01fd Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 24 May 2026 18:34:58 +0200 Subject: [PATCH] mkdir: simplify umask shaping and add regression tests --- src/uu/mkdir/src/mkdir.rs | 23 +++++++++-------------- tests/by-util/test_mkdir.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/src/uu/mkdir/src/mkdir.rs b/src/uu/mkdir/src/mkdir.rs index 33c369e0995..ff46ad59556 100644 --- a/src/uu/mkdir/src/mkdir.rs +++ b/src/uu/mkdir/src/mkdir.rs @@ -259,9 +259,10 @@ impl Drop for UmaskGuard { /// Create a directory with the exact mode specified, bypassing umask. /// -/// GNU mkdir temporarily sets umask to shaped mask 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. +/// 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, @@ -282,12 +283,12 @@ fn create_dir_with_mode(path: &Path, _mode: u32, _shaped_umask: u32) -> std::io: // Helper function to create a single directory with appropriate permissions // `is_parent` argument is not used on windows -#[allow(unused_variables)] +#[cfg_attr(not(unix), allow(unused_variables))] fn create_single_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> { #[cfg(unix)] let (mkdir_mode, shaped_umask) = { - let umask = mode::get_umask(); - let umask_bits = rustix::fs::Mode::from_bits_truncate(umask as rustix::fs::RawMode); + let mode_bits = |x: u32| rustix::fs::Mode::from_bits_truncate(x as rustix::fs::RawMode); + let umask_bits = mode_bits(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 @@ -295,17 +296,11 @@ fn create_single_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<( // 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 & !rustix::fs::Mode::from_bits_truncate(0o300 as rustix::fs::RawMode), - ) + (DEFAULT_PERM, umask_bits & !mode_bits(0o300)) } else { match config.mode { // Explicit -m: shape umask so it cannot block explicitly requested bits. - Some(m) => ( - m, - umask_bits & !rustix::fs::Mode::from_bits_truncate(m as rustix::fs::RawMode), - ), + Some(m) => (m, umask_bits & !mode_bits(m)), // No -m: leave umask fully intact; kernel applies umask + ACL naturally. None => (DEFAULT_PERM, umask_bits), } diff --git a/tests/by-util/test_mkdir.rs b/tests/by-util/test_mkdir.rs index 500b165d8f0..5df3c778122 100644 --- a/tests/by-util/test_mkdir.rs +++ b/tests/by-util/test_mkdir.rs @@ -414,6 +414,38 @@ fn test_mkdir_acl_inheritance_with_restrictive_mask() { ); } +#[test] +#[cfg(unix)] +fn test_mkdir_p_respects_umask_without_acl() { + use std::os::unix::fs::PermissionsExt; + let (at, mut ucmd) = at_and_ucmd!(); + ucmd.arg("-p").arg("a/b/c").umask(0o022).succeeds(); + let perms = at.metadata("a/b/c").permissions().mode(); + assert_eq!(perms & 0o777, 0o755); +} + +#[test] +#[cfg(unix)] +fn test_mkdir_explicit_mode_zero() { + use std::os::unix::fs::PermissionsExt; + let (at, mut ucmd) = at_and_ucmd!(); + ucmd.arg("-m").arg("0").arg("d").umask(0o022).succeeds(); + let perms = at.metadata("d").permissions().mode(); + assert_eq!(perms & 0o777, 0o000); +} + +#[test] +#[cfg(unix)] +fn test_mkdir_explicit_mode_with_umask() { + // -m must win over umask: requesting 0o777 with a restrictive umask must + // still yield 0o777, since the umask is shaped to not block requested bits. + use std::os::unix::fs::PermissionsExt; + let (at, mut ucmd) = at_and_ucmd!(); + ucmd.arg("-m").arg("777").arg("d").umask(0o077).succeeds(); + let perms = at.metadata("d").permissions().mode(); + assert_eq!(perms & 0o777, 0o777); +} + #[test] fn test_mkdir_trailing_dot() { new_ucmd!().arg("-p").arg("-v").arg("test_dir").succeeds();