From 54d1a4a1fe92aaf0576bcbe15c4e89ae86a1b01f Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 9 Jun 2026 20:12:00 +0200 Subject: [PATCH 1/3] uucore: add path-based set_file_times that never opens the target filetime::set_file_times opens the file before setting times, which blocks forever on a FIFO with no writer. Add uucore::fs::set_file_times using a path-based utimensat (like GNU), shared by cp and touch. --- Cargo.lock | 1 + fuzz/Cargo.lock | 11 +++++++ src/uucore/Cargo.toml | 3 +- src/uucore/src/lib/features/fs.rs | 53 +++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 9decf7cc6c1..d783fc4db9f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4480,6 +4480,7 @@ dependencies = [ "digest 0.11.3", "dns-lookup", "dunce", + "filetime", "fluent", "fluent-syntax", "glob", diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock index 8b2870c66f3..db1a575b503 100644 --- a/fuzz/Cargo.lock +++ b/fuzz/Cargo.lock @@ -580,6 +580,16 @@ version = "2.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f1f227452a390804cdb637b74a86990f2a7d7ba4b7d5693aac9b4dd6defd8d6" +[[package]] +name = "filetime" +version = "0.2.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c287a33c7f0a620c38e641e7f60827713987b3c0f26e8ddc9462cc69cf75759" +dependencies = [ + "cfg-if", + "libc", +] + [[package]] name = "find-msvc-tools" version = "0.1.9" @@ -2121,6 +2131,7 @@ dependencies = [ "data-encoding-macro", "digest 0.11.3", "dunce", + "filetime", "fluent", "fluent-syntax", "glob", diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index 37a14830b4d..8ffd2d7b8c4 100644 --- a/src/uucore/Cargo.toml +++ b/src/uucore/Cargo.toml @@ -28,6 +28,7 @@ uucore_procs = { workspace = true } unit-prefix = { workspace = true, optional = true } dns-lookup = { workspace = true, optional = true } dunce = { version = "1.0.4", optional = true } +filetime = { workspace = true, optional = true } glob = { workspace = true, optional = true } itertools = { workspace = true, optional = true } jiff = { workspace = true, optional = true, features = [ @@ -139,7 +140,7 @@ encoding = ["data-encoding", "data-encoding-macro", "z85", "base64-simd"] entries = ["libc"] extendedbigdecimal = ["bigdecimal", "num-traits"] fast-inc = [] -fs = ["dunce", "libc", "winapi-util", "windows-sys"] +fs = ["dunce", "filetime", "libc", "winapi-util", "windows-sys"] fsext = ["libc", "windows-sys", "bstr"] fsxattr = ["xattr", "itertools", "libc"] hardware = [] diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index 7cd9041dd62..d8ed80c64d2 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -7,6 +7,7 @@ // spell-checker:ignore backport +use filetime::FileTime; #[cfg(all(unix, not(target_os = "redox")))] pub use libc::{major, makedev, minor}; use std::collections::HashSet; @@ -902,6 +903,58 @@ pub fn makedev(maj: libc::c_uint, min: libc::c_uint) -> libc::dev_t { (min & 0xff) | ((maj & 0xfff) << 8) | ((min & !0xff) << 12) | ((maj & !0xfff) << 32) } +/// Build a rustix [`Timestamps`](rustix::fs::Timestamps) from an access and a +/// modification [`FileTime`]. +#[cfg(all(unix, not(target_os = "redox")))] +fn to_timestamps(atime: FileTime, mtime: FileTime) -> rustix::fs::Timestamps { + rustix::fs::Timestamps { + last_access: rustix::fs::Timespec { + tv_sec: atime.unix_seconds(), + tv_nsec: atime.nanoseconds() as _, + }, + last_modification: rustix::fs::Timespec { + tv_sec: mtime.unix_seconds(), + tv_nsec: mtime.nanoseconds() as _, + }, + } +} + +/// Set the access and modification times of `path` without opening it. +/// +/// On Unix this uses a path-based `utimensat` (relative to `AT_FDCWD`), matching +/// GNU. `filetime::set_file_times` instead opens the target first, which blocks +/// forever on a FIFO that has no writer — hanging e.g. `cp -a` or `touch` on a +/// named pipe. When `follow_symlinks` is false the times are set on a symlink +/// itself rather than its target. +pub fn set_file_times( + path: &Path, + atime: FileTime, + mtime: FileTime, + follow_symlinks: bool, +) -> IOResult<()> { + #[cfg(all(unix, not(target_os = "redox")))] + { + use rustix::fs::{AtFlags, CWD, utimensat}; + + let timestamps = to_timestamps(atime, mtime); + let flags = if follow_symlinks { + AtFlags::empty() + } else { + AtFlags::SYMLINK_NOFOLLOW + }; + utimensat(CWD, path, ×tamps, flags) + .map_err(|e| Error::from_raw_os_error(e.raw_os_error())) + } + #[cfg(not(all(unix, not(target_os = "redox"))))] + { + if follow_symlinks { + filetime::set_file_times(path, atime, mtime) + } else { + filetime::set_symlink_file_times(path, atime, mtime) + } + } +} + #[cfg(test)] mod tests { // Note this useful idiom: importing names from outer (for mod tests) scope. From c6fa290e176b8481ac8a2bfdb75930b733ac73cc Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 9 Jun 2026 20:34:00 +0200 Subject: [PATCH 2/3] cp: don't hang preserving timestamps on a FIFO Use uucore::fs::set_file_times instead of filetime::set_file_times, which opens the destination first and so blocks forever on a FIFO with no writer. Should make test tests/cp/preserve-mode.sh pass --- src/uu/cp/src/cp.rs | 9 ++++----- tests/by-util/test_cp.rs | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 6fd2abe9ef9..01ac5b39ed9 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1877,11 +1877,10 @@ pub(crate) fn copy_attributes( handle_preserve(attributes.timestamps, || -> CopyResult<()> { let atime = FileTime::from_last_access_time(&source_metadata); let mtime = FileTime::from_last_modification_time(&source_metadata); - if dest.is_symlink() { - filetime::set_symlink_file_times(dest, atime, mtime)?; - } else { - filetime::set_file_times(dest, atime, mtime)?; - } + // Use uucore's path-based setter rather than `filetime::set_file_times`, + // which opens the destination first and so blocks forever on a FIFO with + // no writer, hanging `cp -a` on a named pipe. + uucore::fs::set_file_times(dest, atime, mtime, !dest.is_symlink())?; Ok(()) })?; diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 1b3fde67814..7676174df9a 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -3192,6 +3192,21 @@ fn test_cp_fifo() { assert_eq!(permission, "prwx-wx--x".to_string()); } +// `cp -a` preserves timestamps; setting them must not open the FIFO (which +// would block forever waiting for a writer). Regression test for that hang. +#[test] +#[cfg(unix)] +fn test_cp_archive_fifo_no_hang() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkfifo("fifo"); + ucmd.arg("-a") + .arg("fifo") + .arg("fifo_copy") + .succeeds() + .no_output(); + assert!(at.is_fifo("fifo_copy")); +} + #[rstest] #[case::recursive("-R")] #[case::archive("-a")] From a42d3f1ef2395d89844a485289a35d5daa9c7622 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 9 Jun 2026 20:51:00 +0200 Subject: [PATCH 3/3] touch: don't hang setting timestamps on a FIFO Use uucore::fs::set_file_times instead of filetime, which opens the target first and so blocks forever on a FIFO with no writer. Should make test tests/touch/fifo.sh pass --- src/uu/touch/Cargo.toml | 2 +- src/uu/touch/src/touch.rs | 8 +++++--- src/uucore/src/lib/features/fs.rs | 2 +- tests/by-util/test_touch.rs | 11 +++++++++++ 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/uu/touch/Cargo.toml b/src/uu/touch/Cargo.toml index 447f795d1bf..81c816957f0 100644 --- a/src/uu/touch/Cargo.toml +++ b/src/uu/touch/Cargo.toml @@ -25,7 +25,7 @@ clap = { workspace = true } jiff = { workspace = true } parse_datetime = { workspace = true } thiserror = { workspace = true } -uucore = { workspace = true, features = ["libc", "parser"] } +uucore = { workspace = true, features = ["fs", "libc", "parser"] } fluent = { workspace = true } [dev-dependencies] diff --git a/src/uu/touch/src/touch.rs b/src/uu/touch/src/touch.rs index 4d1c9772c59..9e57cded0ce 100644 --- a/src/uu/touch/src/touch.rs +++ b/src/uu/touch/src/touch.rs @@ -10,7 +10,7 @@ pub mod error; use clap::builder::{PossibleValue, ValueParser}; use clap::{Arg, ArgAction, ArgGroup, ArgMatches, Command}; -use filetime::{FileTime, set_file_times, set_symlink_file_times}; +use filetime::FileTime; use jiff::civil::Time; use jiff::fmt::strtime; use jiff::tz::TimeZone; @@ -600,7 +600,7 @@ fn update_times( // The filename, access time (atime), and modification time (mtime) are provided as inputs. if opts.no_deref && !is_stdout { - return set_symlink_file_times(path, atime, mtime).map_err_context( + return uucore::fs::set_file_times(path, atime, mtime, false).map_err_context( || translate!("touch-error-setting-times-of-path", "path" => path.quote()), ); } @@ -613,7 +613,9 @@ fn update_times( } } - set_file_times(path, atime, mtime) + // Path-based (never opens the target, unlike `filetime::set_file_times`), + // so it does not block on a FIFO with no writer. + uucore::fs::set_file_times(path, atime, mtime, true) .map_err_context(|| translate!("touch-error-setting-times-of-path", "path" => path.quote())) } diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index d8ed80c64d2..cad266543a7 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -5,7 +5,7 @@ //! Set of functions to manage regular files, special files, and links. -// spell-checker:ignore backport +// spell-checker:ignore backport utimensat FDCWD use filetime::FileTime; #[cfg(all(unix, not(target_os = "redox")))] diff --git a/tests/by-util/test_touch.rs b/tests/by-util/test_touch.rs index fbabdb75c65..bed4d321214 100644 --- a/tests/by-util/test_touch.rs +++ b/tests/by-util/test_touch.rs @@ -435,6 +435,17 @@ fn test_touch_no_dereference() { assert_eq!(mtime, start_of_year); } +// Touching a FIFO must not block: setting times must not open the pipe +// (which would wait for a writer). Regression test for that hang. +#[test] +#[cfg(unix)] +fn test_touch_fifo_no_hang() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkfifo("fifo"); + ucmd.arg("fifo").succeeds().no_output(); + assert!(at.is_fifo("fifo")); +} + #[test] fn test_touch_reference() { let scenario = TestScenario::new(util_name!());