diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index e5d905a4a46..b6e7b782c9f 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -1089,6 +1089,23 @@ jobs: - name: Run libc interposition verification run: ./util/check-libc-interposition.sh + test_open_once: + name: Redundant Open Check + runs-on: ubuntu-latest + needs: [ min_version, deps ] + + steps: + - uses: actions/checkout@v7.0.1 + with: + persist-credentials: false + - uses: Swatinem/rust-cache@v2 + - name: Install strace + run: sudo apt-get update && sudo apt-get install -y strace + - name: Build utilities for redundant open checks + run: cargo build --profile=release-small -p uu_cp -p uu_mv + - name: Run redundant open verification + run: ./util/check-open-once.sh + test_toctou: name: TOCTOU Security Check runs-on: ubuntu-latest diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 98afa6d95a8..fc32a4be5da 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -22,26 +22,15 @@ use crate::{ is_stream, }; -// Replacement for `std::fs::copy` that uses the safe-copy primitives but -// only applies `O_NOFOLLOW` to the *source* open. The destination is -// followed if it is a pre-existing symlink, matching GNU cp -d/-P which -// only forbid dereferencing on the source side. -fn fs_copy(source: P, dest: Q, source_nofollow: bool, context: &str) -> CopyResult<()> -where - P: AsRef, - Q: AsRef, -{ - let mut src = open_source(source, source_nofollow) - .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; - let mut dst = create_dest_restrictive(&dest, false).map_err(|e| { +// Create the destination. It is followed when it is a pre-existing symlink, +// matching GNU cp -d/-P which only forbid dereferencing on the source side. +fn create_dest(dest: &Path) -> CopyResult { + create_dest_restrictive(dest, false).map_err(|e| { CpError::IoErrContext( e, - translate!("cp-error-cannot-create-regular-file", "path" => dest.as_ref().quote()), + translate!("cp-error-cannot-create-regular-file", "path" => dest.quote()), ) - })?; - buf_copy::copy_fast(&mut src, &mut dst) - .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; - Ok(()) + }) } /// The fallback behavior for [`clone`] on failed system call. @@ -69,87 +58,89 @@ enum CloneFallback { enum CopyMethod { /// Do a sparse copy SparseCopy, - /// Use [`std::fs::copy`]. + /// Copy the bytes with [`buf_copy::copy_fast`]. FSCopy, /// Default (can either be [`CopyMethod::SparseCopy`] or [`CopyMethod::FSCopy`]) Default, - /// Use [`sparse_copy_without_hole`] + /// Use [`sparse_copy_without_hole_fd`] SparseCopyWithoutHole, } /// Use the Linux `ioctl_ficlone` API to do a copy-on-write clone. /// /// `fallback` controls what to do if the system call fails. -fn clone

( - source: P, - dest: P, +fn clone( + src_file: &mut File, + dest: &Path, fallback: CloneFallback, - nofollow: bool, context: &str, -) -> CopyResult<()> -where - P: AsRef, -{ +) -> CopyResult<()> { // Only needed to decide whether a failed --reflink=always clone should // clean up the dest, so skip the lstat for the other fallbacks. - let dest_existed = - matches!(fallback, CloneFallback::Error) && dest.as_ref().symlink_metadata().is_ok(); - let mut src_file = - open_source(&source, nofollow).map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; - let mut dst_file = create_dest_restrictive(&dest, false).map_err(|e| { - CpError::IoErrContext( - e, - translate!("cp-error-cannot-create-regular-file", "path" => dest.as_ref().quote()), - ) - })?; - if let Err(err) = ioctl_ficlone(&dst_file, &src_file) { + let dest_existed = matches!(fallback, CloneFallback::Error) && dest.symlink_metadata().is_ok(); + let mut dst_file = create_dest(dest)?; + if let Err(err) = ioctl_ficlone(&dst_file, &*src_file) { // Reuse the already-open descriptors: the dest was just created with // a restrictive mode that the umask may have stripped of write bits, // so re-opening it by path can fail with EACCES (LP: #2164777). return match fallback { CloneFallback::Error => { // GNU cp removes a dest it created itself, but keeps a - // pre-existing (now truncated) one. - if !dest_existed { - let _ = std::fs::remove_file(&dest); + // pre-existing (now truncated) one. Only unlink while the + // path still names the inode we created: it may have been + // replaced since, and removing blindly would drop an + // unrelated file. + if !dest_existed && path_still_refers_to(dest, &dst_file) { + let _ = std::fs::remove_file(dest); } Err(CpError::IoErrContext(err.into(), context.to_owned())) } - CloneFallback::FSCopy => buf_copy::copy_fast(&mut src_file, &mut dst_file) + CloneFallback::FSCopy => buf_copy::copy_fast(src_file, &mut dst_file) .map_err(|e| CpError::IoErrContext(e, context.to_owned())), - CloneFallback::SparseCopy => sparse_copy_fd(&mut src_file, &dst_file, context), + CloneFallback::SparseCopy => sparse_copy_fd(src_file, &dst_file, context), CloneFallback::SparseCopyWithoutHole => { - sparse_copy_without_hole_fd(&src_file, &dst_file, context) + sparse_copy_without_hole_fd(src_file, &dst_file, context) } }; } Ok(()) } +/// Whether `path` still resolves to the inode behind `file`. +fn path_still_refers_to(path: &Path, file: &File) -> bool { + let (Ok(current), Ok(opened)) = (path.symlink_metadata(), file.metadata()) else { + return false; + }; + current.dev() == opened.dev() && current.ino() == opened.ino() +} + /// Checks whether a file contains any non null bytes i.e. any byte != 0x0 /// This function returns a tuple of (bool, u64, u64) signifying a tuple of (whether a file has /// data, its size, no of blocks it has allocated in disk) -fn check_for_data(source: &Path, nofollow: bool) -> io::Result<(bool, u64, u64)> { - let mut src_file = open_source(source, nofollow)?; +fn check_for_data(src_file: &mut File) -> io::Result<(bool, u64, u64)> { let metadata = src_file.metadata()?; let size = metadata.size(); let blocks = metadata.blocks(); // checks edge case of virtual files in /proc which have a size of zero but contains data - if size == 0 { + let (has_data, blocks) = if size == 0 { let mut buf: Vec = vec![0; metadata.blksize() as usize]; // Directly use metadata.blksize() - let _ = src_file.read(&mut buf)?; - return Ok((buf.iter().any(|&x| x != 0x0), size, 0)); - } - let has_data = seek(src_file, SeekFrom::Data(0)).is_ok(); + let read = src_file.read(&mut buf)?; + (buf[..read].iter().any(|&x| x != 0x0), 0) + } else { + (seek(&*src_file, SeekFrom::Data(0)).is_ok(), blocks) + }; + + // The probe moved the descriptor; the copy that follows reads it + // sequentially, so hand it back positioned at the start. + seek(&*src_file, SeekFrom::Start(0))?; Ok((has_data, size, blocks)) } /// Checks whether a file is sparse i.e. it contains holes, uses the crude heuristic blocks < size / 512 /// Reference:`` -fn check_sparse_detection(source: &Path, nofollow: bool) -> io::Result { - let src_file = open_source(source, nofollow)?; +fn check_sparse_detection(src_file: &File) -> io::Result { let metadata = src_file.metadata()?; let size = metadata.size(); let blocks = metadata.blocks(); @@ -157,23 +148,8 @@ fn check_sparse_detection(source: &Path, nofollow: bool) -> io::Result { Ok(blocks < size / 512) } -/// Optimized [`sparse_copy`] doesn't create holes for large sequences of zeros in non `sparse_files` +/// Optimized [`sparse_copy_fd`] doesn't create holes for large sequences of zeros in non `sparse_files` /// Used when `--sparse=auto` -fn sparse_copy_without_hole

(source: P, dest: P, nofollow: bool, context: &str) -> CopyResult<()> -where - P: AsRef, -{ - let src_file = - open_source(&source, nofollow).map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; - let dst_file = create_dest_restrictive(&dest, false).map_err(|e| { - CpError::IoErrContext( - e, - translate!("cp-error-cannot-create-regular-file", "path" => dest.as_ref().quote()), - ) - })?; - sparse_copy_without_hole_fd(&src_file, &dst_file, context) -} - fn sparse_copy_without_hole_fd(src_file: &File, dst_file: &File, context: &str) -> CopyResult<()> { let ctx_err = |e: io::Error| CpError::IoErrContext(e, context.to_owned()); @@ -212,21 +188,6 @@ fn sparse_copy_without_hole_fd(src_file: &File, dst_file: &File, context: &str) } /// Perform a sparse copy from one file to another. /// Creates a holes for large sequences of zeros in `non_sparse_files`, used for `--sparse=always` -fn sparse_copy

(source: P, dest: P, nofollow: bool, context: &str) -> CopyResult<()> -where - P: AsRef, -{ - let mut src_file = - open_source(&source, nofollow).map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; - let dst_file = create_dest_restrictive(&dest, false).map_err(|e| { - CpError::IoErrContext( - e, - translate!("cp-error-cannot-create-regular-file", "path" => dest.as_ref().quote()), - ) - })?; - sparse_copy_fd(&mut src_file, &dst_file, context) -} - fn sparse_copy_fd(src_file: &mut File, dst_file: &File, context: &str) -> CopyResult<()> { let ctx_err = |e: io::Error| CpError::IoErrContext(e, context.to_owned()); @@ -321,6 +282,12 @@ where } /// Copies `source` to `dest` using copy-on-write if possible. +/// +/// The source is opened once and the descriptor is threaded through both the +/// sparseness probe and the copy itself. Probing and copying used to open the +/// path separately, costing three opens per copy and letting the strategy be +/// decided from one file while the bytes came from whatever the path named +/// later (#13185). pub(crate) fn copy_on_write( source: &Path, dest: &Path, @@ -344,16 +311,20 @@ pub(crate) fn copy_on_write( copy_debug.offload = OffloadReflinkDebug::Avoided; copy_stream(source, dest, nofollow, context) } else { + let mut src_file = open_source(source, nofollow) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; let mut copy_method = CopyMethod::Default; - let result = handle_reflink_never_sparse_always(source, dest, nofollow); + let result = handle_reflink_never_sparse_always(&mut src_file, dest); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; } + let mut dst_file = create_dest(dest)?; match copy_method { - CopyMethod::FSCopy => fs_copy(source, dest, nofollow, context), - _ => sparse_copy(source, dest, nofollow, context), + CopyMethod::FSCopy => buf_copy::copy_fast(&mut src_file, &mut dst_file) + .map_err(|e| CpError::IoErrContext(e, context.to_owned())), + _ => sparse_copy_fd(&mut src_file, &dst_file, context), } } } @@ -364,11 +335,15 @@ pub(crate) fn copy_on_write( copy_debug.offload = OffloadReflinkDebug::Avoided; copy_stream(source, dest, nofollow, context) } else { - let result = handle_reflink_never_sparse_never(source, nofollow); + let mut src_file = open_source(source, nofollow) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; + let result = handle_reflink_never_sparse_never(&mut src_file); if let Ok(debug) = result { copy_debug = debug; } - fs_copy(source, dest, nofollow, context) + let mut dst_file = create_dest(dest)?; + buf_copy::copy_fast(&mut src_file, &mut dst_file) + .map_err(|e| CpError::IoErrContext(e, context.to_owned())) } } (ReflinkMode::Never, SparseMode::Auto) => { @@ -378,18 +353,22 @@ pub(crate) fn copy_on_write( copy_debug.offload = OffloadReflinkDebug::Avoided; copy_stream(source, dest, nofollow, context) } else { + let mut src_file = open_source(source, nofollow) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; let mut copy_method = CopyMethod::Default; - let result = handle_reflink_never_sparse_auto(source, dest, nofollow); + let result = handle_reflink_never_sparse_auto(&mut src_file, dest); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; } + let mut dst_file = create_dest(dest)?; match copy_method { CopyMethod::SparseCopyWithoutHole => { - sparse_copy_without_hole(source, dest, nofollow, context) + sparse_copy_without_hole_fd(&src_file, &dst_file, context) } - _ => fs_copy(source, dest, nofollow, context), + _ => buf_copy::copy_fast(&mut src_file, &mut dst_file) + .map_err(|e| CpError::IoErrContext(e, context.to_owned())), } } } @@ -400,8 +379,10 @@ pub(crate) fn copy_on_write( copy_debug.offload = OffloadReflinkDebug::Avoided; copy_stream(source, dest, nofollow, context) } else { + let mut src_file = open_source(source, nofollow) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; let mut copy_method = CopyMethod::Default; - let result = handle_reflink_auto_sparse_always(source, dest, nofollow); + let result = handle_reflink_auto_sparse_always(&mut src_file, dest); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; @@ -409,9 +390,9 @@ pub(crate) fn copy_on_write( match copy_method { CopyMethod::FSCopy => { - clone(source, dest, CloneFallback::FSCopy, nofollow, context) + clone(&mut src_file, dest, CloneFallback::FSCopy, context) } - _ => clone(source, dest, CloneFallback::SparseCopy, nofollow, context), + _ => clone(&mut src_file, dest, CloneFallback::SparseCopy, context), } } } @@ -422,12 +403,14 @@ pub(crate) fn copy_on_write( copy_debug.offload = OffloadReflinkDebug::Avoided; copy_stream(source, dest, nofollow, context) } else { - let result = handle_reflink_auto_sparse_never(source, nofollow); + let mut src_file = open_source(source, nofollow) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; + let result = handle_reflink_auto_sparse_never(&mut src_file); if let Ok(debug) = result { copy_debug = debug; } - clone(source, dest, CloneFallback::FSCopy, nofollow, context) + clone(&mut src_file, dest, CloneFallback::FSCopy, context) } } (ReflinkMode::Auto, SparseMode::Auto) => { @@ -435,8 +418,10 @@ pub(crate) fn copy_on_write( copy_debug.offload = OffloadReflinkDebug::Unsupported; copy_stream(source, dest, nofollow, context) } else { + let mut src_file = open_source(source, nofollow) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; let mut copy_method = CopyMethod::Default; - let result = handle_reflink_auto_sparse_auto(source, dest, nofollow); + let result = handle_reflink_auto_sparse_auto(&mut src_file, dest); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; @@ -444,13 +429,12 @@ pub(crate) fn copy_on_write( match copy_method { CopyMethod::SparseCopyWithoutHole => clone( - source, + &mut src_file, dest, CloneFallback::SparseCopyWithoutHole, - nofollow, context, ), - _ => clone(source, dest, CloneFallback::FSCopy, nofollow, context), + _ => clone(&mut src_file, dest, CloneFallback::FSCopy, context), } } } @@ -459,7 +443,9 @@ pub(crate) fn copy_on_write( copy_debug.sparse_detection = SparseDebug::No; copy_debug.reflink = OffloadReflinkDebug::Yes; - clone(source, dest, CloneFallback::Error, nofollow, context) + let mut src_file = open_source(source, nofollow) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; + clone(&mut src_file, dest, CloneFallback::Error, context) } (ReflinkMode::Always, _) => { return Err(translate!("cp-error-reflink-always-sparse-auto").into()); @@ -472,9 +458,8 @@ pub(crate) fn copy_on_write( /// Handles debug results when flags are "--reflink=auto" and "--sparse=always" and specifies what /// type of copy should be used fn handle_reflink_auto_sparse_always( - source: &Path, + src_file: &mut File, dest: &Path, - nofollow: bool, ) -> io::Result<(CopyDebug, CopyMethod)> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, @@ -482,8 +467,8 @@ fn handle_reflink_auto_sparse_always( sparse_detection: SparseDebug::Zeros, }; let mut copy_method = CopyMethod::Default; - let (data_flag, size, blocks) = check_for_data(source, nofollow)?; - let sparse_flag = check_sparse_detection(source, nofollow)?; + let (data_flag, size, blocks) = check_for_data(src_file)?; + let sparse_flag = check_sparse_detection(src_file)?; if data_flag || size < 512 { copy_debug.offload = OffloadReflinkDebug::Avoided; @@ -508,14 +493,14 @@ fn handle_reflink_auto_sparse_always( /// Handles debug results when flags are "--reflink=auto" and "--sparse=auto" and specifies what /// type of copy should be used -fn handle_reflink_never_sparse_never(source: &Path, nofollow: bool) -> io::Result { +fn handle_reflink_never_sparse_never(src_file: &mut File) -> io::Result { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::No, sparse_detection: SparseDebug::No, }; - let (data_flag, size, _blocks) = check_for_data(source, nofollow)?; - let sparse_flag = check_sparse_detection(source, nofollow)?; + let (data_flag, size, _blocks) = check_for_data(src_file)?; + let sparse_flag = check_sparse_detection(src_file)?; if sparse_flag { copy_debug.sparse_detection = SparseDebug::SeekHole; @@ -528,16 +513,16 @@ fn handle_reflink_never_sparse_never(source: &Path, nofollow: bool) -> io::Resul } /// Handles debug results when flags are "--reflink=auto" and "--sparse=never", files will be copied -/// through cloning them with fallback switching to [`std::fs::copy`] -fn handle_reflink_auto_sparse_never(source: &Path, nofollow: bool) -> io::Result { +/// through cloning them with fallback switching to [`buf_copy::copy_fast`] +fn handle_reflink_auto_sparse_never(src_file: &mut File) -> io::Result { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::No, sparse_detection: SparseDebug::No, }; - let (data_flag, size, _blocks) = check_for_data(source, nofollow)?; - let sparse_flag = check_sparse_detection(source, nofollow)?; + let (data_flag, size, _blocks) = check_for_data(src_file)?; + let sparse_flag = check_sparse_detection(src_file)?; if sparse_flag { copy_debug.sparse_detection = SparseDebug::SeekHole; @@ -552,9 +537,8 @@ fn handle_reflink_auto_sparse_never(source: &Path, nofollow: bool) -> io::Result /// Handles debug results when flags are "--reflink=auto" and "--sparse=auto" and specifies what /// type of copy should be used fn handle_reflink_auto_sparse_auto( - source: &Path, + src_file: &mut File, dest: &Path, - nofollow: bool, ) -> io::Result<(CopyDebug, CopyMethod)> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, @@ -563,8 +547,8 @@ fn handle_reflink_auto_sparse_auto( }; let mut copy_method = CopyMethod::Default; - let (data_flag, size, blocks) = check_for_data(source, nofollow)?; - let sparse_flag = check_sparse_detection(source, nofollow)?; + let (data_flag, size, blocks) = check_for_data(src_file)?; + let sparse_flag = check_sparse_detection(src_file)?; if (data_flag && size != 0) || (size > 0 && size < 512) { copy_debug.offload = OffloadReflinkDebug::Yes; @@ -596,9 +580,8 @@ fn handle_reflink_auto_sparse_auto( /// Handles debug results when flags are "--reflink=never" and "--sparse=auto" and specifies what /// type of copy should be used fn handle_reflink_never_sparse_auto( - source: &Path, + src_file: &mut File, dest: &Path, - nofollow: bool, ) -> io::Result<(CopyDebug, CopyMethod)> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, @@ -606,8 +589,8 @@ fn handle_reflink_never_sparse_auto( sparse_detection: SparseDebug::No, }; - let (data_flag, size, blocks) = check_for_data(source, nofollow)?; - let sparse_flag = check_sparse_detection(source, nofollow)?; + let (data_flag, size, blocks) = check_for_data(src_file)?; + let sparse_flag = check_sparse_detection(src_file)?; let mut copy_method = CopyMethod::Default; if data_flag || size < 512 { @@ -633,9 +616,8 @@ fn handle_reflink_never_sparse_auto( /// Handles debug results when flags are "--reflink=never" and "--sparse=always" and specifies what /// type of copy should be used fn handle_reflink_never_sparse_always( - source: &Path, + src_file: &mut File, dest: &Path, - nofollow: bool, ) -> io::Result<(CopyDebug, CopyMethod)> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, @@ -644,8 +626,8 @@ fn handle_reflink_never_sparse_always( }; let mut copy_method = CopyMethod::SparseCopy; - let (data_flag, size, blocks) = check_for_data(source, nofollow)?; - let sparse_flag = check_sparse_detection(source, nofollow)?; + let (data_flag, size, blocks) = check_for_data(src_file)?; + let sparse_flag = check_sparse_detection(src_file)?; if data_flag || size < 512 { copy_debug.offload = OffloadReflinkDebug::Avoided; diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index bd81c5f76f2..f216f52a3be 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -4,7 +4,7 @@ // file that was distributed with this source code. // spell-checker:ignore (flags) reflink (fs) tmpfs (linux) filefrag rlimit Rlim NOFILE clob btrfs neve ROOTDIR USERDIR outfile subvolume uufs xattrs ELOOP -// spell-checker:ignore bdfl hlsl IRWXO IRWXG nconfined matchpathcon libselinux-devel prwx doesnotexist reftests subdirs mksocket srwx dstlink +// spell-checker:ignore bdfl hlsl IRWXO IRWXG nconfined matchpathcon libselinux-devel prwx doesnotexist reftests subdirs mksocket srwx dstlink mcstransd #[cfg(unix)] use rstest::rstest; use uucore::display::Quotable; @@ -5611,6 +5611,34 @@ fn test_cp_debug_sparse_never_zero_sized_virtual_file() { .stdout_contains("copy offload: avoided, reflink: no, sparse detection: no"); } +// A file that stats as zero-sized but still returns data (/proc entries) +// is probed for content by reading from it. The copy reuses that same +// descriptor, so it has to start over at offset 0 -- otherwise everything +// the probe consumed is dropped and the destination ends up empty. The +// --debug tests above only look at the reported strategy, not the bytes. +#[test] +#[cfg(target_os = "linux")] +fn test_cp_zero_sized_virtual_file_contents() { + let expected = std::fs::read_to_string("/proc/version").unwrap(); + assert!(!expected.is_empty()); + + for extra in [ + &[][..], + &["--sparse=never"], + &["--sparse=always"], + &["--sparse=auto"], + &["--reflink=never"], + &["--reflink=never", "--sparse=always"], + ] { + let (at, mut ucmd) = at_and_ucmd!(); + ucmd.args(extra) + .args(&["/proc/version", "copied"]) + .succeeds() + .no_output(); + assert_eq!(at.read("copied"), expected, "with {extra:?}"); + } +} + #[test] #[cfg(target_os = "linux")] fn test_cp_debug_default_zero_sized_virtual_file() { diff --git a/util/check-open-once.sh b/util/check-open-once.sh new file mode 100755 index 00000000000..f9b19086bda --- /dev/null +++ b/util/check-open-once.sh @@ -0,0 +1,128 @@ +#!/bin/bash +# +# spell-checker:ignore strace reflink FDCWD EXDEV tmpfs xdev dstdir RDONLY +# +# Redundant-open verification. +# +# These strace-based checks assert that a utility resolves an operand path +# once and works off the resulting descriptor, rather than re-opening the +# same path for each stage of its work. Repeated opens cost syscalls and +# let two stages disagree about which file they are acting on. +# +# This is a correctness and efficiency check, not a security one: the +# companion scripts check-toctou.sh and check-safe-traversal.sh cover the +# cases where a split across two path-based syscalls is actually +# attacker-exploitable. +# + +set -e + +echo "=== Redundant Open Verification ===" + +# shellcheck disable=SC2034 # read by check-common.sh once sourced +CHECK_UTILS="cp mv" +. "$(dirname "${BASH_SOURCE[0]}")/check-common.sh" + +# check-common.sh installs `trap cleanup EXIT`; extend it so the scratch +# directory on the second filesystem goes away with it. +SHM_DIR="" +cleanup() { + rm -rf "$TEMP_DIR" + if [ -n "$SHM_DIR" ]; then + rm -rf "$SHM_DIR" + fi +} + +cd "$TEMP_DIR" + +# cp used to open the source three times per copy: once in check_for_data, +# once in check_sparse_detection, then again in the copy itself. The +# sparseness probe and the copy now share one descriptor, so the strategy +# and the bytes come from the same file (#13185). +if cp_cmd=$(util_cmd cp); then + echo "" + echo "Testing cp (single source open)..." + # rustix issues open(2) directly on Linux, so match both spellings -- + # an openat-only filter would silently match nothing and pass. + for mode in "" "--reflink=never --sparse=always" "--reflink=never --sparse=never"; do + rm -f cp_probe_dst + printf 'source bytes for the probe\n' > cp_probe_src.txt + # shellcheck disable=SC2086 # $mode is a deliberate word-split flag list + strace -f -e trace=open,openat -o strace_cp_open.log \ + $cp_cmd $mode cp_probe_src.txt cp_probe_dst 2>/dev/null || true + + if [ ! -s strace_cp_open.log ]; then + fail_immediately "strace produced no output for cp" + fi + opens=$(grep -cE '(open|openat)\(.*"cp_probe_src\.txt"' strace_cp_open.log || true) + if [ "$opens" -ne 1 ]; then + cat strace_cp_open.log + fail_immediately "cp ${mode:-(default)} opened the source $opens times, expected exactly 1 - the sparseness probe and the copy must share one descriptor (#13185)" + fi + echo "✓ cp ${mode:-(default)} opens the source exactly once" + done + rm -f cp_probe_src.txt cp_probe_dst +fi + +# mv must not open an operand it is only going to rename, and its +# cross-device fallback -- a copy, since rename(2) cannot cross filesystems -- +# must open the source once. mv has its own fallback rather than cp's +# (mv.rs, open_source + create_dest_restrictive + copy_fast), so this pins +# an invariant that a future move onto shared copy machinery could break. +if mv_cmd=$(util_cmd mv); then + echo "" + echo "Testing mv (single source open)..." + + printf 'moved by rename\n' > mv_probe_src.txt + strace -f -e trace=open,openat -o strace_mv_rename.log \ + $mv_cmd mv_probe_src.txt mv_probe_dst 2>/dev/null || true + if [ ! -s strace_mv_rename.log ]; then + fail_immediately "strace produced no output for mv" + fi + opens=$(grep -cE '(open|openat)\(.*"mv_probe_src\.txt"' strace_mv_rename.log || true) + if [ "$opens" -ne 0 ]; then + cat strace_mv_rename.log + fail_immediately "mv opened the source $opens times for a same-filesystem move, expected 0 - it must rename, not copy" + fi + echo "✓ mv (same filesystem) never opens the source" + rm -f mv_probe_dst + + # /dev/shm is tmpfs on Linux, so it is a different filesystem from + # $TEMP_DIR and rename(2) across the two returns EXDEV. Skip rather than + # fail where that does not hold, so the check stays honest about what it + # actually exercised. + if [ -d /dev/shm ] && [ "$(stat -c %d /dev/shm)" != "$(stat -c %d "$TEMP_DIR")" ]; then + SHM_DIR=$(mktemp -d /dev/shm/uutils-open-once.XXXXXX) + + printf 'moved across filesystems\n' > mv_probe_xdev.txt + strace -f -e trace=open,openat -o strace_mv_xdev.log \ + $mv_cmd mv_probe_xdev.txt "$SHM_DIR/dst" 2>/dev/null || true + opens=$(grep -cE '(open|openat)\(.*"mv_probe_xdev\.txt"' strace_mv_xdev.log || true) + if [ "$opens" -ne 1 ]; then + cat strace_mv_xdev.log + fail_immediately "mv opened the source $opens times for a cross-device move, expected exactly 1" + fi + echo "✓ mv (cross-device file) opens the source exactly once" + + mkdir -p mv_probe_dir + printf 'inner payload\n' > mv_probe_dir/inner.txt + strace -f -e trace=open,openat -o strace_mv_xdev_dir.log \ + $mv_cmd mv_probe_dir "$SHM_DIR/dstdir" 2>/dev/null || true + # Match any path ending in inner.txt opened O_RDONLY: that is the + # source read, whether it is spelled relative to the cwd or, once the + # traversal is fd-anchored, as a bare name under a directory fd. The + # O_RDONLY rules out the destination, which is opened for writing. + opens=$(grep -cE '(open|openat)\([^)]*"[^"]*inner\.txt"[^)]*O_RDONLY' strace_mv_xdev_dir.log || true) + if [ "$opens" -ne 1 ]; then + cat strace_mv_xdev_dir.log + fail_immediately "mv opened a file inside a cross-device directory move $opens times, expected exactly 1" + fi + echo "✓ mv (cross-device directory) opens each file exactly once" + else + echo "- skipped mv cross-device checks: no second filesystem available" + fi + rm -rf mv_probe_src.txt mv_probe_xdev.txt mv_probe_dir +fi + +echo "" +echo "✓ Redundant open verification completed"