From f10599dc969781f8b5313013f5e8ecc6adbebd0d Mon Sep 17 00:00:00 2001 From: mattsu Date: Sat, 25 Oct 2025 20:31:37 +0900 Subject: [PATCH 1/4] refactor: use execvp directly instead of process::Command - Replace process::Command execution with direct execvp call via libc for improved performance by avoiding process forking - Add validation to detect and error on commands containing null bytes - Capture and handle execution errors using the last OS error after execvp failure --- src/uu/chroot/src/chroot.rs | 51 ++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/src/uu/chroot/src/chroot.rs b/src/uu/chroot/src/chroot.rs index 0ac59df17be..7e5e891d168 100644 --- a/src/uu/chroot/src/chroot.rs +++ b/src/uu/chroot/src/chroot.rs @@ -9,14 +9,14 @@ mod error; use crate::error::ChrootError; use clap::{Arg, ArgAction, Command}; use std::ffi::CString; -use std::io::Error; +use std::io::{Error, ErrorKind}; use std::os::unix::prelude::OsStrExt; use std::path::{Path, PathBuf}; -use std::process; +use std::ptr; use uucore::entries::{Locate, Passwd, grp2gid, usr2uid}; -use uucore::error::{UResult, UUsageError, set_exit_code}; +use uucore::error::{UResult, UUsageError}; use uucore::fs::{MissingHandling, ResolveMode, canonicalize}; -use uucore::libc::{self, chroot, setgid, setgroups, setuid}; +use uucore::libc::{self, chroot, execvp, setgid, setgroups, setuid}; use uucore::{format_usage, show}; use uucore::translate; @@ -205,33 +205,38 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { assert!(!command.is_empty()); let chroot_command = command[0]; - let chroot_args = &command[1..]; // NOTE: Tests can only trigger code beyond this point if they're invoked with root permissions set_context(&options)?; - let pstatus = match process::Command::new(chroot_command) - .args(chroot_args) - .status() - { - Ok(status) => status, - Err(e) => { - return Err(if e.kind() == std::io::ErrorKind::NotFound { - ChrootError::CommandNotFound(command[0].to_string(), e) - } else { - ChrootError::CommandFailed(command[0].to_string(), e) + let mut argv_storage = Vec::with_capacity(command.len()); + for &arg in &command { + match CString::new(arg) { + Ok(value) => argv_storage.push(value), + Err(_) => { + let error = Error::new( + ErrorKind::InvalidInput, + "command contains interior null byte", + ); + return Err(ChrootError::CommandFailed(chroot_command.to_owned(), error).into()); } - .into()); } - }; + } + + let mut argv: Vec<*const libc::c_char> = argv_storage.iter().map(|c| c.as_ptr()).collect(); + argv.push(ptr::null()); - let code = if pstatus.success() { - 0 + unsafe { + execvp(argv_storage[0].as_ptr(), argv.as_ptr()); + } + + let err = Error::last_os_error(); + Err(if err.kind() == ErrorKind::NotFound { + ChrootError::CommandNotFound(chroot_command.to_owned(), err) } else { - pstatus.code().unwrap_or(-1) - }; - set_exit_code(code); - Ok(()) + ChrootError::CommandFailed(chroot_command.to_owned(), err) + } + .into()) } pub fn uu_app() -> Command { From 09d739036f72a7f30c148a33768703363f1e9c2f Mon Sep 17 00:00:00 2001 From: mattsu Date: Sat, 25 Oct 2025 21:37:51 +0900 Subject: [PATCH 2/4] refactor(chroot): replace manual execvp with ProcessCommand for safer execution Simplified the command execution by using Rust's standard ProcessCommand instead of direct libc calls, improving code readability and reducing manual memory management. --- src/uu/chroot/src/chroot.rs | 29 ++++++----------------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/src/uu/chroot/src/chroot.rs b/src/uu/chroot/src/chroot.rs index 7e5e891d168..d079ea8b8a0 100644 --- a/src/uu/chroot/src/chroot.rs +++ b/src/uu/chroot/src/chroot.rs @@ -11,12 +11,13 @@ use clap::{Arg, ArgAction, Command}; use std::ffi::CString; use std::io::{Error, ErrorKind}; use std::os::unix::prelude::OsStrExt; +use std::os::unix::process::CommandExt; use std::path::{Path, PathBuf}; -use std::ptr; +use std::process::Command as ProcessCommand; use uucore::entries::{Locate, Passwd, grp2gid, usr2uid}; use uucore::error::{UResult, UUsageError}; use uucore::fs::{MissingHandling, ResolveMode, canonicalize}; -use uucore::libc::{self, chroot, execvp, setgid, setgroups, setuid}; +use uucore::libc::{self, chroot, setgid, setgroups, setuid}; use uucore::{format_usage, show}; use uucore::translate; @@ -209,28 +210,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // NOTE: Tests can only trigger code beyond this point if they're invoked with root permissions set_context(&options)?; - let mut argv_storage = Vec::with_capacity(command.len()); - for &arg in &command { - match CString::new(arg) { - Ok(value) => argv_storage.push(value), - Err(_) => { - let error = Error::new( - ErrorKind::InvalidInput, - "command contains interior null byte", - ); - return Err(ChrootError::CommandFailed(chroot_command.to_owned(), error).into()); - } - } - } - - let mut argv: Vec<*const libc::c_char> = argv_storage.iter().map(|c| c.as_ptr()).collect(); - argv.push(ptr::null()); - - unsafe { - execvp(argv_storage[0].as_ptr(), argv.as_ptr()); - } + let err = ProcessCommand::new(chroot_command) + .args(&command[1..]) + .exec(); - let err = Error::last_os_error(); Err(if err.kind() == ErrorKind::NotFound { ChrootError::CommandNotFound(chroot_command.to_owned(), err) } else { From 9dabf96fa7eb6f9109e826ae06b261ab3dd4b4e4 Mon Sep 17 00:00:00 2001 From: mattsu Date: Mon, 27 Oct 2025 11:19:35 +0900 Subject: [PATCH 3/4] test(chroot): add tests for error handling and UID/GID retention Added three new tests to verify chroot behavior: - Test for command not found, expecting exit code 127 and "No such file or directory" error. - Test for permission denied, expecting exit code 126 and "Permission denied" error. - Test to confirm UID and GID are retained as root (0) inside the chroot. These tests enhance coverage for edge cases and ensure robust error reporting, requiring root privileges to run. --- tests/by-util/test_chroot.rs | 68 ++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/tests/by-util/test_chroot.rs b/tests/by-util/test_chroot.rs index 38c3727b1cd..adeaf32bf6c 100644 --- a/tests/by-util/test_chroot.rs +++ b/tests/by-util/test_chroot.rs @@ -188,6 +188,53 @@ fn test_default_shell() { } } +#[test] +fn test_chroot_command_not_found_error() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + let dir = "CHROOT_DIR"; + at.mkdir(dir); + + let missing = "definitely_missing_command"; + + if let Ok(result) = run_ucmd_as_root(&ts, &[dir, missing]) { + result + .failure() + .code_is(127) + .stderr_contains(format!("failed to run command '{missing}'")) + .stderr_contains("No such file or directory"); + } else { + print!("Test skipped; requires root user"); + } +} + +#[test] +fn test_chroot_command_permission_denied_error() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + let dir = "CHROOT_DIR"; + at.mkdir(dir); + + let script_path = format!("{dir}/noexec.sh"); + at.write(&script_path, "#!/bin/sh\necho unreachable\n"); + #[cfg(not(windows))] + { + at.set_mode(&script_path, 0o644); + } + + if let Ok(result) = run_ucmd_as_root(&ts, &[dir, "/noexec.sh"]) { + result + .failure() + .code_is(126) + .stderr_contains("failed to run command '/noexec.sh'") + .stderr_contains("Permission denied"); + } else { + print!("Test skipped; requires root user"); + } +} + #[test] fn test_chroot() { let ts = TestScenario::new(util_name!()); @@ -208,6 +255,27 @@ fn test_chroot() { } } +#[test] +fn test_chroot_retains_uid_gid() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + let dir = "CHROOT_DIR"; + at.mkdir(dir); + + if let Ok(result) = run_ucmd_as_root(&ts, &[dir, "id", "-u"]) { + result.success().no_stderr().stdout_is("0"); + } else { + print!("Test skipped; requires root user"); + } + + if let Ok(result) = run_ucmd_as_root(&ts, &[dir, "id", "-g"]) { + result.success().no_stderr().stdout_is("0"); + } else { + print!("Test skipped; requires root user"); + } +} + #[test] fn test_chroot_skip_chdir_not_root() { let (at, mut ucmd) = at_and_ucmd!(); From 6524f506ebf3afbfa2b076eaf0d4d773f9c101be Mon Sep 17 00:00:00 2001 From: mattsu Date: Mon, 27 Oct 2025 11:25:45 +0900 Subject: [PATCH 4/4] chore: add 'noexec' to cspell jargon wordlist for spell checking Added 'noexec' to the jargon wordlist to recognize this technical term and avoid false positives during spell checking in the project. --- .vscode/cspell.dictionaries/jargon.wordlist.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/.vscode/cspell.dictionaries/jargon.wordlist.txt b/.vscode/cspell.dictionaries/jargon.wordlist.txt index cacaad1af19..2fd55eaf887 100644 --- a/.vscode/cspell.dictionaries/jargon.wordlist.txt +++ b/.vscode/cspell.dictionaries/jargon.wordlist.txt @@ -89,6 +89,7 @@ nocache nocreat noctty noerror +noexec nofollow nolinks nonblock