Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions src/uu/mkdir/src/mkdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ use clap::builder::ValueParser;
use clap::parser::ValuesRef;
use clap::{Arg, ArgAction, ArgMatches, Command};
use std::ffi::OsString;
#[cfg(all(unix, not(target_os = "linux")))]
use std::os::unix::fs::MetadataExt;
use std::io::{Write, stdout};
use std::path::{Path, PathBuf};
#[cfg(all(unix, target_os = "linux"))]
#[cfg(not(windows))]
use uucore::error::FromIo;
use uucore::error::{UResult, USimpleError};
use uucore::translate;
Expand Down Expand Up @@ -196,8 +198,9 @@ 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"))]
/// Only needed on Linux to add ACL permission bits after directory creation and on
/// macos / bsd to inherit setgid bits
#[cfg(not(windows))]
fn chmod(path: &Path, mode: u32) -> UResult<()> {
use std::fs::{Permissions, set_permissions};
use std::os::unix::fs::PermissionsExt;
Expand Down Expand Up @@ -293,6 +296,25 @@ fn create_dir_with_mode(path: &Path, _mode: u32) -> std::io::Result<()> {
std::fs::create_dir(path)
}

// macos and bsd systems do not inherit the parents directories SETGID bit on the kernel
// level so we have to manually set the SETGID
#[cfg(all(unix, not(target_os = "linux")))]
fn inherit_setgid_bit(path: &Path) -> UResult<()> {
if let Some(parent) = path.parent() {
if let Ok(parent_metadata) = parent.metadata() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens in case of error on metadata() ?

if let Ok(paths_metadata) = path.metadata() {
if (parent_metadata.mode() & 0o2000) != (paths_metadata.mode() & 0o2000) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

condition checks if bits are different, should check if parent has setgid AND child doesn't: `(parent_metadata.mode() & 0o2000) != 0 && (paths_metadata.mode() & 0o2000) == 0

no ?

chmod(
path,
paths_metadata.mode() | parent_metadata.mode() & 0o2000,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be paths_metadata.mode() | 0o2000 since we already verified parent has the bit, no?

)?;
}
}
}
}
Ok(())
}

// Helper function to create a single directory with appropriate permissions
// `is_parent` argument is not used on windows
#[allow(unused_variables)]
Expand All @@ -319,6 +341,10 @@ fn create_single_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<(
translate!("mkdir-verbose-created-directory", "util_name" => uucore::util_name(), "path" => path.quote())
)?;
}
// macos and bsd systems do not inherit the parents directories SETGID bit on the kernel
// level so we have to manually set the SETGID
#[cfg(all(unix, not(target_os = "linux")))]
inherit_setgid_bit(path)?;

// 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.
Expand Down
2 changes: 1 addition & 1 deletion tests/by-util/test_mkdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,7 @@ fn test_mkdir_parent_mode_with_explicit_mode() {

/// Test that nested directories inherit the setgid bit with mkdir -p.
#[test]
#[cfg(target_os = "linux")]
#[cfg(not(windows))]
fn test_mkdir_parent_inherits_setgid() {
let (at, mut ucmd) = at_and_ucmd!();

Expand Down
Loading