diff --git a/src/uu/mkdir/src/mkdir.rs b/src/uu/mkdir/src/mkdir.rs index 9d632115c54..03c2681e2e1 100644 --- a/src/uu/mkdir/src/mkdir.rs +++ b/src/uu/mkdir/src/mkdir.rs @@ -11,8 +11,6 @@ use clap::{Arg, ArgAction, ArgMatches, Command}; use std::ffi::OsString; use std::io::{Write, stdout}; use std::path::{Path, PathBuf}; -#[cfg(all(unix, target_os = "linux"))] -use uucore::error::FromIo; use uucore::error::{UResult, USimpleError}; use uucore::translate; @@ -37,8 +35,8 @@ pub struct Config<'a> { /// Create parent directories as needed. pub recursive: bool, - /// File permissions (octal). - pub mode: u32, + /// File permissions (octal) if provided via -m + pub mode: Option, /// Print message for each created directory. pub verbose: bool, @@ -55,18 +53,18 @@ pub struct Config<'a> { clippy::unnecessary_wraps, reason = "fn sig must match on all platforms" )] -fn get_mode(_matches: &ArgMatches) -> Result { - Ok(DEFAULT_PERM) +fn get_mode(_matches: &ArgMatches) -> Result, String> { + Ok(None) } #[cfg(not(windows))] -fn get_mode(matches: &ArgMatches) -> Result { +fn get_mode(matches: &ArgMatches) -> Result, String> { // Not tested on Windows if let Some(m) = matches.get_one::(options::MODE) { - mode::parse_chmod(DEFAULT_PERM, m, true, mode::get_umask()) + mode::parse_chmod(DEFAULT_PERM, m, true, mode::get_umask()).map(Some) } else { - // If no mode argument is specified return the mode derived from umask - Ok(!mode::get_umask() & DEFAULT_PERM) + // If no mode argument, let the kernel apply umask and ACLs naturally. + Ok(None) } } @@ -196,17 +194,6 @@ pub fn mkdir(path: &Path, config: &Config) -> UResult<()> { create_dir(path, false, config) } -/// 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; - let mode = Permissions::from_mode(mode); - set_permissions(path, mode).map_err_context( - || translate!("mkdir-error-cannot-set-permissions", "path" => path.quote()), - ) -} - // 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<()> { @@ -272,22 +259,24 @@ impl Drop for UmaskGuard { /// Create a directory with the exact mode specified, bypassing umask. /// -/// GNU mkdir temporarily sets umask to 0 before calling mkdir(2), ensuring the +/// 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. #[cfg(unix)] -fn create_dir_with_mode(path: &Path, mode: u32) -> std::io::Result<()> { +fn create_dir_with_mode( + path: &Path, + mode: u32, + shaped_umask: rustix::fs::Mode, +) -> 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(rustix::fs::Mode::empty()); + let _guard = UmaskGuard::set(shaped_umask); std::fs::DirBuilder::new().mode(mode).create(path) } #[cfg(not(unix))] -fn create_dir_with_mode(path: &Path, _mode: u32) -> std::io::Result<()> { +fn create_dir_with_mode(path: &Path, _mode: u32, _shaped_umask: u32) -> std::io::Result<()> { std::fs::create_dir(path) } @@ -295,20 +284,37 @@ fn create_dir_with_mode(path: &Path, _mode: u32) -> std::io::Result<()> { // `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(); - - // 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 + let (mkdir_mode, shaped_umask) = { + let umask = mode::get_umask(); + let umask_bits = rustix::fs::Mode::from_bits_truncate(umask as rustix::fs::RawMode); + 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 & !rustix::fs::Mode::from_bits_truncate(0o300 as rustix::fs::RawMode), + ) + } 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), + ), + // No -m: leave umask fully intact; kernel applies umask + ACL naturally. + None => (DEFAULT_PERM, umask_bits), + } + } }; #[cfg(not(unix))] - let create_mode = config.mode; + let (mkdir_mode, shaped_umask) = (config.mode.unwrap_or(DEFAULT_PERM), 0u32); - match create_dir_with_mode(path, create_mode) { + match create_dir_with_mode(path, mkdir_mode, shaped_umask) { Ok(()) => { if config.verbose { writeln!( @@ -318,18 +324,6 @@ 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"))] - 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 acl_perm_bits != 0 { - chmod(path, create_mode | acl_perm_bits)?; - } - } - // Apply SELinux context if requested #[cfg(feature = "selinux")] if config.set_security_context && uucore::selinux::is_selinux_enabled() { diff --git a/tests/by-util/test_mkdir.rs b/tests/by-util/test_mkdir.rs index 9cd164b7411..2e1f9939839 100644 --- a/tests/by-util/test_mkdir.rs +++ b/tests/by-util/test_mkdir.rs @@ -360,6 +360,60 @@ fn test_mkdir_acl() { assert_eq!(perms, 16893); } +#[test] +#[cfg(target_os = "linux")] +fn test_mkdir_acl_inheritance_with_restrictive_mask() { + use rustc_hash::FxHashMap; + use std::ffi::OsString; + + let (at, mut ucmd) = at_and_ucmd!(); + + at.mkdir("parent"); + + let mut map: FxHashMap> = FxHashMap::default(); + + // Default ACL with mask::r-x (0o5) — more restrictive than a umask of 0o022 would allow. + // With umask 0o022, group bits would be r-x already, but the mask enforces this + // regardless of what umask would permit. With umask 0o000, without ACL mask the + // child would get rwx for group, but mask caps it to r-x. + // + // Encoding: header(0x0002) + entries: + // ACL_USER_OBJ (0x0001) perm=7 (rwx) + // ACL_GROUP_OBJ (0x0004) perm=7 (rwx) — would be rwx without mask + // ACL_MASK (0x0010) perm=5 (r-x) — restricts group effective to r-x + // ACL_OTHER (0x0020) perm=0 (---) + let xattr_val: Vec = vec![ + 2, 0, 0, 0, // header + 1, 0, 7, 0, 255, 255, 255, 255, // ACL_USER_OBJ rwx + 4, 0, 7, 0, 255, 255, 255, 255, // ACL_GROUP_OBJ rwx (masked to r-x) + 16, 0, 5, 0, 255, 255, 255, 255, // ACL_MASK r-x + 32, 0, 0, 0, 255, 255, 255, 255, // ACL_OTHER --- + ]; + + map.insert(OsString::from("system.posix_acl_default"), xattr_val); + uucore::fsxattr::apply_xattrs(at.plus("parent"), map).unwrap(); + + // umask 0o000 — without correct ACL inheritance, group would get rwx (7) + // With correct inheritance the mask restricts group to r-x (5) + ucmd.arg("-p").arg("parent/child").umask(0o000).succeeds(); + + let perms = at.metadata("parent/child").permissions().mode(); + // Expected: user=rwx(7), group=r-x(5 from mask), other=---(0) + // mode bits: 0o750 = 0o40750 with directory bit + assert_eq!( + perms & 0o777, + 0o750, + "Expected group bits capped to r-x by ACL mask, got {:o}", + perms & 0o777 + ); + + // Verify the child itself has an ACL (indicated by presence of xattr) + assert!( + uucore::fsxattr::has_acl(at.plus("parent/child")), + "Child directory should have inherited ACL entries" + ); +} + #[test] fn test_mkdir_trailing_dot() { new_ucmd!().arg("-p").arg("-v").arg("test_dir").succeeds();