cp/rm/mv/ftw Updates

NONPOSIX.md

@@ -213,7 +213,11 @@ dh_installexamples), with GNU cp's meaning:
  * `--parents` — the destination of each source is the target directory
    followed by the source's path, and missing directories on that path are
    made from the source's (with `-p`, their owner, mode and times too).  The
-   target must be an existing directory.
+   target must be an existing directory.  Unlike GNU, a source ending in `..`
+   is refused with a diagnostic and exit status 1: its destination ends in
+   `..` too, which names no entry inside the target but whatever directory
+   that reaches -- for `..` itself the target's parent -- and GNU cp copies
+   onto that.  A source of `.` copies its contents into the target itself.
  * `-l` — each non-directory is hard-linked to its source instead of copied;
    with `-R`, directories are made and the files in them linked.  An existing
    destination is replaced only under `-f` (or `-i` answered yes); one that is
@@ -228,6 +232,44 @@ dh_installexamples), with GNU cp's meaning:
    quoted as GNU coreutils quotes them.  sysvinit installs with
    `cp -afv etc/* $(DESTDIR)...`.
 
+#### Existing directories under `-p`
+
+`cp -p` (and `-a`) and `pax -p` give a directory that already existed in the
+destination what `-p` asks for -- its mode and owner from the source -- only
+where nobody but the user could have created its name: in its parent and in
+every directory above it, up to the destination the user named.  Elsewhere it
+keeps its mode, owner and times, its contents are still copied, a diagnostic
+names it, and the exit status is 1.  Without a mode or owner asked for it gets
+its times only, as by default.  GNU cp and libarchive do no such check.
+
+A directory counts as one nobody else can create entries in when it is the
+user's, grants no other write permission (sticky or not), grants group write
+permission only for the user's private group -- its primary group, named as
+the user, listing no other member -- and carries no ACL that lets others
+write.  This fails closed, with the diagnostic and exit 1, wherever it cannot
+tell:
+
+ * an NFSv4 or CIFS ACL (`system.nfs4_acl`, `system.nfs4_acl_xdr`,
+   `system.cifs_acl`), whose entries are not evaluated;
+ * group write permission for a group the user shares -- a primary group that
+   is not a user-private group, as under a umask of 002 without user private
+   groups;
+ * an ACL attribute, or the user and group databases, that cannot be read.
+
+On Linux, root working in another user's tree -- extracting into
+`/home/alice`, which alice owns -- also trusts that one user's directories, as
+the user's own: every directory below root's own must be alice's alone, and
+so must every directory above where her tree starts, up to one of root's, and
+each directory found there must itself be hers.  Elsewhere, where ACLs are
+not read, root trusts only its own directories.
+
+What is not seen: groups granted outside the user and group databases
+(`pam_group`, systemd `SupplementaryGroups=`); another account given the
+user's primary gid (accounts are not enumerated); a second entry for the same
+gid, in a later NSS source or a second `/etc/group` line; a group password;
+a macOS ACL; and, in another user's tree, a move of that user's own
+directories timed against the lazy check above it.
+
 ### cpio
 
 The whole utility is an addition: a compatibility front-end over `pax`
@@ -611,6 +653,8 @@ Debian source packages, with GNU patch's meaning:
    asked for (script or `/dev/tty`) rather than taken for the end.  Reads GNU
    volume labels and `'M'` continuation headers; writes whole members per
    volume.  Written in ustar format only, and incompatible with `-z`.
+ * `-p` on a directory that already existed follows the rule under
+   [cp, existing directories under `-p`](#existing-directories-under--p).
  * `-x bcpio`, `-x sv4cpio`, `-x sv4crc` — the historic pax names for the old
    binary cpio header and the SVR4 "newc" headers without and with a data
    checksum.  POSIX names only `cpio` (odc), `pax` and `ustar`.  All three are

ftw/src/dir.rs

@@ -244,10 +244,11 @@ impl DeferredDir {
         // remainder - bar/baz
         let remainder = self.path.strip_prefix(&self.parent.1).unwrap();
 
-        // `remainder` is not guaranteed to be shorter than `libc::PATH_MAX`. When it is not, the
-        // prefix is opened one component at a time, each with this walk's descent flags and
-        // checked against the identity the walk recorded for it.
-        let identities = self.component_identities();
+        // `remainder` is not guaranteed to be shorter than `libc::PATH_MAX`. When it is not (or
+        // resolving it at once meets ELOOP), the prefix is opened one component at a time, each
+        // with this walk's descent flags and checked against the identity the walk recorded for
+        // it. The identities are gathered only then: most reopens resolve in one call.
+        let identities = || self.component_identities();
         let (starting_dir, components) = open_long_filename(
             self.parent.0.try_clone()?,
             remainder,

ftw/src/lib.rs

@@ -379,6 +379,8 @@ struct TreeNode {
     metadata: Metadata,
     /// Whether the directory entry is itself a symbolic link (one the walk followed).
     is_symlink: Option<bool>,
+    /// That link's own identity (`Entry::symlink_id`).
+    symlink_id: Option<(u64, u64)>,
     path_depth: usize,
 }
 
@@ -397,6 +399,7 @@ impl TreeNode {
         )
         .with_shown_name(self.shown_name.clone());
         entry.is_symlink = self.is_symlink;
+        entry.symlink_id = self.symlink_id;
         entry
     }
 
@@ -420,6 +423,9 @@ pub struct Entry<'a> {
     shown_name: Option<Rc<[libc::c_char]>>,
     metadata: Option<Metadata>,
     is_symlink: Option<bool>,
+    /// `(st_dev, st_ino)` of the entry itself when it is a symbolic link, from the walk's own
+    /// `lstat` of it.
+    symlink_id: Option<(u64, u64)>,
     read_link: Option<Rc<[libc::c_char]>>,
 }
 
@@ -437,6 +443,7 @@ impl<'a> Entry<'a> {
             shown_name: None,
             metadata,
             is_symlink: None,
+            symlink_id: None,
             read_link: None,
         }
     }
@@ -470,6 +477,15 @@ impl<'a> Entry<'a> {
         self.is_symlink
     }
 
+    /// `(st_dev, st_ino)` of the entry itself when it is a symbolic link -- set exactly when
+    /// `is_symlink` is `Some(true)` -- from the `lstat` the walk made of it, the link it examined.
+    /// When the walk follows the link, `metadata` is its target's, and this is the only record of
+    /// the link: a caller asking whether a name is still that link compares against it rather
+    /// than resolving the name again.
+    pub fn symlink_id(&self) -> Option<(u64, u64)> {
+        self.symlink_id
+    }
+
     /// Reads the symbolic link.
     pub fn read_link(&self) -> Option<&CStr> {
         self.read_link
@@ -630,6 +646,10 @@ where
         }
     };
     let is_symlink = entry_symlink_metadata.file_type() == FileType::SymbolicLink;
+    let symlink_id = is_symlink.then(|| {
+        use std::os::unix::fs::MetadataExt;
+        (entry_symlink_metadata.dev(), entry_symlink_metadata.ino())
+    });
 
     // Read the link target for every symbolic link, whether or not this walk follows links:
     // consumers need it either way -- `cp -P` and `mv` recreate the link from it, `ls -l` prints
@@ -686,6 +706,7 @@ where
     let mut entry = Entry::new(dir_fd, path_stack, entry_filename, Some(entry_metadata))
         .with_shown_name(shown_name.cloned());
     entry.is_symlink = Some(is_symlink);
+    entry.symlink_id = symlink_id;
     entry.read_link = entry_readlink;
 
     if must_be_dir {
@@ -759,21 +780,23 @@ where
 /// Every prefix component is opened `O_RDONLY | O_DIRECTORY | O_CLOEXEC` plus `open_flags` (a
 /// walk's descent flags, so `O_NOFOLLOW` when it does not follow links). `O_DIRECTORY` refuses a
 /// FIFO or device swapped in for a component before the open can block on it or open the
-/// device. With `identities` (one recorded `(dev, ino)` per component of `path`), each opened
-/// component must also be the very directory the walk recorded there.
+/// device. With `identities` (giving one recorded `(dev, ino)` per component of `path`, and
+/// called only once a component is to be opened), each opened component must also be the very
+/// directory the walk recorded there.
 fn open_long_filename<'a, H>(
     mut starting_dir: FileDescriptor,
     path: &'a Path,
     mut path_stack: Option<&mut Vec<Rc<[libc::c_char]>>>,
     open_flags: libc::c_int,
-    identities: Option<&[(libc::dev_t, libc::ino_t)]>,
+    identities: Option<&dyn Fn() -> Vec<(libc::dev_t, libc::ino_t)>>,
     err_reporter: &mut H,
 ) -> io::Result<(FileDescriptor, std::path::Components<'a>)>
 where
     H: FnMut(Entry<'_>, Error),
 {
     let mut path_components = path.components();
     let mut opened = 0usize;
+    let mut recorded: Option<Vec<(libc::dev_t, libc::ino_t)>> = None;
 
     // If `path` is too long, start at a prefix of `path`
     loop {
@@ -823,7 +846,7 @@ where
         )
         .and_then(|fd| match identities {
             // Fail closed: a component with no recorded identity, or the wrong one, is refused.
-            Some(ids) => match ids.get(opened) {
+            Some(gather) => match recorded.get_or_insert_with(gather).get(opened) {
                 Some(&(dev, ino)) if fd_matches(&fd, dev, ino) => Ok(fd),
                 _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)),
             },
@@ -1072,8 +1095,9 @@ where
 /// the path to it is resolved again, so renaming or replacing one of its ancestors after `dir`
 /// was opened cannot redirect the walk. `name` is a single component (it may carry trailing
 /// slashes), looked up in `dir` exactly as `traverse_directory` looks up an operand's last
-/// component; and `postprocess_dir` for the starting point itself receives `dir` as the
-/// containing directory.
+/// component: a symbolic link named with a trailing slash is walked as `.` in the directory it
+/// names (`Entry::reached_through_symlink`). `postprocess_dir` for the starting point itself
+/// receives `dir` as the containing directory (or, for such a link, the directory it names).
 ///
 /// `display_parent` is only shown: each entry's `path()` is `display_parent` joined with the
 /// entry's path from `name`. It is never resolved.
@@ -1112,6 +1136,23 @@ where
             return false;
         }
     };
+
+    // A trailing slash after a name that may be a symbolic link: as in `traverse_directory`.
+    let name_bytes = name.to_bytes();
+    let (bare, suffix) = name_bytes.split_at(name_bytes.len() - directory_suffix_len(name_bytes));
+    if !suffix.is_empty() && !matches!(bare, b"" | b"." | b"..") {
+        let bare = CString::new(bare).expect("taken from a C string");
+        return walk_slash_operand(
+            starting_dir,
+            path_stack,
+            cstring_to_rc(&bare),
+            suffix,
+            file_handler,
+            postprocess_dir,
+            err_reporter,
+            opts,
+        );
+    }
     walk_from(
         starting_dir,
         path_stack,
@@ -1201,6 +1242,7 @@ where
                             filename: dir_filename,
                             shown_name,
                             is_symlink: entry.is_symlink,
+                            symlink_id: entry.symlink_id,
                             metadata: entry.metadata.unwrap(),
                             path_depth: path_stack.len(),
                         };
@@ -1461,6 +1503,7 @@ where
                                             filename: entry_filename,
                                             shown_name: None,
                                             is_symlink: entry.is_symlink,
+                                            symlink_id: entry.symlink_id,
                                             metadata: entry.metadata.unwrap(),
                                             path_depth,
                                         }
@@ -1478,6 +1521,7 @@ where
                                             filename: entry_filename,
                                             shown_name: None,
                                             is_symlink: entry.is_symlink,
+                                            symlink_id: entry.symlink_id,
                                             metadata: entry.metadata.unwrap(),
                                             path_depth,
                                         }
@@ -1512,6 +1556,7 @@ where
                                             filename: entry_filename,
                                             shown_name: None,
                                             is_symlink: entry.is_symlink,
+                                            symlink_id: entry.symlink_id,
                                             metadata: entry.metadata.unwrap(),
                                             path_depth,
                                         }
@@ -1626,7 +1671,8 @@ fn lists_nothing(dir: OwnedDir) -> io::Result<bool> {
 /// Whether the directory open on `dir_fd` is empty.
 ///
 /// It is read through a new open of `.` relative to `dir_fd` -- the same directory, which no
-/// rename can swap -- so `dir_fd`'s own read position is left alone.
+/// rename can swap -- so `dir_fd` itself (its read position, or a descriptor held for search
+/// only) is left alone. `plib::madefs` checks a directory it made with this too.
 pub fn is_empty_dir_fd(dir_fd: RawFd) -> io::Result<bool> {
     let fd = unsafe {
         libc::openat(

ftw/tests/race.rs

@@ -508,6 +508,84 @@ fn deferred_long_path_reopen_refuses_fifo_component() {
     }
 }
 
+/// A deferred directory reopened one component at a time checks every component against the
+/// identity the walk recorded for it, not only the directory reached: a component replaced by
+/// another directory holding the rest of the path (the same directories below) is refused.
+#[test]
+fn deferred_long_path_reopen_refuses_a_replaced_component() {
+    use std::os::fd::{AsRawFd, FromRawFd, OwnedFd};
+
+    let tmp = plib::tmp::Builder::new()
+        .prefix("ftw_race_long_ident")
+        .tempdir_in(env!("CARGO_TARGET_TMPDIR"))
+        .unwrap();
+    let base = tmp.path().to_path_buf();
+    let root = base.join("root");
+    fs::create_dir(&root).unwrap();
+
+    // Twenty 250-byte components: far past PATH_MAX from `root`, so made through descriptors.
+    const DEPTH: usize = 20;
+    let names: Vec<String> = (0..DEPTH)
+        .map(|i| format!("{i:02}{}", "d".repeat(248)))
+        .collect();
+    let mut dir = fs::File::open(&root).unwrap();
+    for name in &names {
+        let c = std::ffi::CString::new(name.as_str()).unwrap();
+        assert_eq!(
+            unsafe { libc::mkdirat(dir.as_raw_fd(), c.as_ptr(), 0o755) },
+            0
+        );
+        let fd = unsafe {
+            libc::openat(
+                dir.as_raw_fd(),
+                c.as_ptr(),
+                libc::O_RDONLY | libc::O_DIRECTORY,
+            )
+        };
+        assert!(fd >= 0);
+        dir = fs::File::from(unsafe { OwnedFd::from_raw_fd(fd) });
+    }
+    drop(dir);
+
+    // The second component is replaced by a new directory, and the third moved into it: the
+    // path from `root` again leads to the very same directories below.
+    let first = root.join(&names[0]);
+    let last_name = names[DEPTH - 1].clone();
+    let mut swapped = false;
+    let mut long_errors = 0usize;
+    traverse_directory(
+        &root,
+        |entry| {
+            if !swapped && entry.file_name().to_bytes() == last_name.as_bytes() {
+                let old = first.join(&names[1]);
+                fs::rename(&old, base.join("moved")).unwrap();
+                fs::create_dir(&old).unwrap();
+                fs::rename(base.join("moved").join(&names[2]), old.join(&names[2])).unwrap();
+                swapped = true;
+            }
+            Ok(true)
+        },
+        |_entry, _exit| Ok(()),
+        |entry, _err| {
+            // Only a reopen longer than `PATH_MAX` goes component by component; a shorter one
+            // is refused by the identity of the directory it reaches.
+            if entry.path().as_inner().as_os_str().len() >= libc::PATH_MAX as usize {
+                long_errors += 1;
+            }
+        },
+        TraverseDirectoryOpts {
+            // Conserve descriptors from the first level on.
+            caller_fds_per_level: 4096,
+            ..Default::default()
+        },
+    );
+    assert!(swapped, "the swap must have run");
+    assert!(
+        long_errors > 0,
+        "a long reopen through the replaced component must be refused"
+    );
+}
+
 /// Walk `root` after the handler for the starting point itself has replaced it using `swap`.
 /// Returns the names visited and the number of errors reported.
 fn walk_after_root_swap(tag: &str, swap: impl Fn(&Path, &Path)) -> (HashSet<String>, usize) {

pax/Cargo.toml

@@ -35,6 +35,7 @@ ftw = { path = "../ftw" }
 
 [dev-dependencies]
 filetime = "0.2"
+plib = { path = "../plib", features = ["test-hooks"] }
 
 [[bin]]
 name = "pax"