Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 6 May 2025 17:12:21 GMT
From:      Dag-Erling =?utf-8?Q?Sm=C3=B8rgrav?= <des@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 82fc0d09e862 - main - cp: Partly restore symlink folllowing.
Message-ID:  <202505061712.546HCLPr088289@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=82fc0d09e86255f42ae79fb48fc8cb3820b5ccd9

commit 82fc0d09e86255f42ae79fb48fc8cb3820b5ccd9
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2025-05-06 17:11:44 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2025-05-06 17:11:44 +0000

    cp: Partly restore symlink folllowing.
    
    * As a general rule, follow symbolic links in the destination as long
      as the target is within the destination hierarchy.
    * As a special case, allow the destination itself to be a symbolic
      link, and even a dead one (in which case we create the target).
    * The file-to-file case remains unrestricted.
    
    Currently, if a symlink we aren't allowed to follow is encountered,
    cp will behave just like it would if the file was not writable.  We
    should consider whether it would be better to replace the offending
    link instead.
    
    Fixes:          0729d1e8fd90
    MFC after:      never
    Relnotes:       yes
    Sponsored by:   Klara, Inc.
    Reviewed by:    markj
    Differential Revision:  https://reviews.freebsd.org/D50093
---
 bin/cp/cp.c             | 391 ++++++++++++++++++++++++++----------------------
 bin/cp/extern.h         |  31 ++--
 bin/cp/tests/cp_test.sh | 169 +++++++++++++++++++++
 bin/cp/utils.c          | 260 +++++++++++++-------------------
 4 files changed, 512 insertions(+), 339 deletions(-)

diff --git a/bin/cp/cp.c b/bin/cp/cp.c
index 2b9c7531e4ca..62cc2abc3654 100644
--- a/bin/cp/cp.c
+++ b/bin/cp/cp.c
@@ -53,9 +53,11 @@
 #include <assert.h>
 #include <err.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <fts.h>
 #include <limits.h>
 #include <signal.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -63,15 +65,10 @@
 
 #include "extern.h"
 
-#define	STRIP_TRAILING_SLASH(p) {					\
-	while ((p).p_end > (p).p_path + 1 && (p).p_end[-1] == '/')	\
-	*--(p).p_end = 0;						\
-}
-
-static char emptystring[] = "";
-
-PATH_T to = { to.p_path, emptystring, "" };
+static char dot[] = ".";
 
+#define END(buf) (buf + sizeof(buf))
+PATH_T to = { .dir = -1, .end = to.path };
 int Nflag, fflag, iflag, lflag, nflag, pflag, sflag, vflag;
 static int Hflag, Lflag, Pflag, Rflag, rflag;
 volatile sig_atomic_t info;
@@ -86,8 +83,9 @@ main(int argc, char *argv[])
 {
 	struct stat to_stat, tmp_stat;
 	enum op type;
-	int ch, fts_options, r, have_trailing_slash;
-	char *target;
+	int ch, fts_options, r;
+	char *sep, *target;
+	bool have_trailing_slash = false;
 
 	fts_options = FTS_NOCHDIR | FTS_PHYSICAL;
 	while ((ch = getopt(argc, argv, "HLPRafilNnprsvx")) != -1)
@@ -177,17 +175,16 @@ main(int argc, char *argv[])
 
 	/* Save the target base in "to". */
 	target = argv[--argc];
-	if (strlcpy(to.p_path, target, sizeof(to.p_path)) >= sizeof(to.p_path))
-		errx(1, "%s: name too long", target);
-	to.p_end = to.p_path + strlen(to.p_path);
-	if (to.p_path == to.p_end) {
-		*to.p_end++ = '.';
-		*to.p_end = 0;
+	if (*target == '\0') {
+		target = dot;
+	} else if ((sep = strrchr(target, '/')) != NULL && sep[1] == '\0') {
+		have_trailing_slash = true;
+		while (sep > target + 1 && *(sep - 1) == '/')
+			sep--;
+		*sep = '\0';
 	}
-	have_trailing_slash = (to.p_end[-1] == '/');
-	if (have_trailing_slash)
-		STRIP_TRAILING_SLASH(to);
-	to.target_end = to.p_end;
+	if (strlcpy(to.base, target, sizeof(to.base)) >= sizeof(to.base))
+		errc(1, ENAMETOOLONG, "%s", target);
 
 	/* Set end of argument list for fts(3). */
 	argv[argc] = NULL;
@@ -206,15 +203,15 @@ main(int argc, char *argv[])
 	 *
 	 * In (2), the real target is not directory, but "directory/source".
 	 */
-	r = stat(to.p_path, &to_stat);
+	r = stat(to.base, &to_stat);
 	if (r == -1 && errno != ENOENT)
-		err(1, "%s", to.p_path);
+		err(1, "%s", target);
 	if (r == -1 || !S_ISDIR(to_stat.st_mode)) {
 		/*
 		 * Case (1).  Target is not a directory.
 		 */
 		if (argc > 1)
-			errc(1, ENOTDIR, "%s", to.p_path);
+			errc(1, ENOTDIR, "%s", target);
 
 		/*
 		 * Need to detect the case:
@@ -238,9 +235,9 @@ main(int argc, char *argv[])
 
 		if (have_trailing_slash && type == FILE_TO_FILE) {
 			if (r == -1)
-				errc(1, ENOENT, "%s", to.p_path);
+				errc(1, ENOENT, "%s", target);
 			else
-				errc(1, ENOTDIR, "%s", to.p_path);
+				errc(1, ENOTDIR, "%s", target);
 		}
 	} else {
 		/*
@@ -264,13 +261,14 @@ static int
 copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 {
 	char rootname[NAME_MAX];
-	struct stat created_root_stat, to_stat;
+	struct stat created_root_stat, to_stat, *curr_stat;
 	FTS *ftsp;
 	FTSENT *curr;
-	int base = 0, dne, badcp, rval;
-	size_t nlen;
-	char *p, *recurse_path, *target_mid;
+	char *recpath = NULL;
+	int atflags, dne, badcp, len, rval;
 	mode_t mask, mode;
+	bool beneath = type != FILE_TO_FILE;
+	bool skipdp = false;
 
 	/*
 	 * Keep an inverted copy of the umask, for use in correcting
@@ -279,10 +277,22 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 	mask = ~umask(0777);
 	umask(~mask);
 
-	recurse_path = NULL;
+	if (type == FILE_TO_FILE) {
+		to.dir = AT_FDCWD;
+		to.end = to.path + strlcpy(to.path, to.base, sizeof(to.path));
+		strcpy(to.base, dot);
+	} else if (type == FILE_TO_DIR) {
+		to.dir = open(to.base, O_DIRECTORY | O_SEARCH);
+		if (to.dir < 0)
+			err(1, "%s", to.base);
+	}
+
 	if ((ftsp = fts_open(argv, fts_options, NULL)) == NULL)
 		err(1, "fts_open");
-	for (badcp = rval = 0; (curr = fts_read(ftsp)) != NULL; badcp = 0) {
+	for (badcp = rval = 0;
+	     (curr = fts_read(ftsp)) != NULL;
+	     badcp = 0, *to.end = '\0') {
+		curr_stat = curr->fts_statp;
 		switch (curr->fts_info) {
 		case FTS_NS:
 		case FTS_DNR:
@@ -294,119 +304,116 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 			warnx("%s: directory causes a cycle", curr->fts_path);
 			badcp = rval = 1;
 			continue;
-		default:
-			;
-		}
-
-		/*
-		 * Stash the root basename off for detecting recursion later.
-		 *
-		 * This will be essential if the root is a symlink and we're
-		 * rolling with -L or -H.  The later bits will need this bit in
-		 * particular.
-		 */
-		if (curr->fts_level == FTS_ROOTLEVEL) {
-			strlcpy(rootname, curr->fts_name, sizeof(rootname));
-		}
-
-		/*
-		 * If we are in case (2) or (3) above, we need to append the
-		 * source name to the target name.
-		 */
-		if (type != FILE_TO_FILE) {
+		case FTS_D:
 			/*
-			 * Need to remember the roots of traversals to create
-			 * correct pathnames.  If there's a directory being
-			 * copied to a non-existent directory, e.g.
-			 *	cp -R a/dir noexist
-			 * the resulting path name should be noexist/foo, not
-			 * noexist/dir/foo (where foo is a file in dir), which
-			 * is the case where the target exists.
-			 *
-			 * Also, check for "..".  This is for correct path
-			 * concatenation for paths ending in "..", e.g.
-			 *	cp -R .. /tmp
-			 * Paths ending in ".." are changed to ".".  This is
-			 * tricky, but seems the easiest way to fix the problem.
+			 * Stash the root basename off for detecting
+			 * recursion later.
 			 *
-			 * XXX
-			 * Since the first level MUST be FTS_ROOTLEVEL, base
-			 * is always initialized.
+			 * This will be essential if the root is a symlink
+			 * and we're rolling with -L or -H.  The later
+			 * bits will need this bit in particular.
 			 */
 			if (curr->fts_level == FTS_ROOTLEVEL) {
-				if (type != DIR_TO_DNE) {
-					p = strrchr(curr->fts_path, '/');
-					base = (p == NULL) ? 0 :
-					    (int)(p - curr->fts_path + 1);
-
-					if (strcmp(curr->fts_path + base, "..")
-					    == 0)
-						base += 1;
-				} else
-					base = curr->fts_pathlen;
-			}
-
-			p = &curr->fts_path[base];
-			nlen = curr->fts_pathlen - base;
-			target_mid = to.target_end;
-			if (*p != '/' && target_mid[-1] != '/')
-				*target_mid++ = '/';
-			*target_mid = 0;
-			if (target_mid - to.p_path + nlen >= PATH_MAX) {
-				warnx("%s%s: name too long (not copied)",
-				    to.p_path, p);
-				badcp = rval = 1;
-				continue;
+				strlcpy(rootname, curr->fts_name,
+				    sizeof(rootname));
 			}
-			(void)strncat(target_mid, p, nlen);
-			to.p_end = target_mid + nlen;
-			*to.p_end = 0;
-			STRIP_TRAILING_SLASH(to);
-
 			/*
-			 * We're on the verge of recursing on ourselves.  Either
-			 * we need to stop right here (we knowingly just created
-			 * it), or we will in an immediate descendant.  Record
-			 * the path of the immediate descendant to make our
-			 * lives a little less complicated looking.
+			 * If we FTS_SKIP while handling FTS_D, we will
+			 * immediately get FTS_DP for the same directory.
+			 * If this happens before we've appended the name
+			 * to to.path, we need to remember not to perform
+			 * the reverse operation.
 			 */
-			if (curr->fts_info == FTS_D && root_stat != NULL &&
-			    root_stat->st_dev == curr->fts_statp->st_dev &&
-			    root_stat->st_ino == curr->fts_statp->st_ino) {
-				assert(recurse_path == NULL);
-
-				if (root_stat == &created_root_stat) {
-					/*
-					 * This directory didn't exist when we
-					 * started, we created it as part of
-					 * traversal.  Stop right here before we
-					 * do something silly.
-					 */
+			skipdp = true;
+			/* we must have a destination! */
+			if (type == DIR_TO_DNE &&
+			    curr->fts_level == FTS_ROOTLEVEL) {
+				assert(to.dir < 0);
+				assert(root_stat == NULL);
+				mode = curr_stat->st_mode | S_IRWXU;
+				if (mkdir(to.base, mode) != 0) {
+					warn("%s", to.base);
+					(void)fts_set(ftsp, curr, FTS_SKIP);
+					badcp = rval = 1;
+					continue;
+				}
+				to.dir = open(to.base, O_DIRECTORY | O_SEARCH);
+				if (to.dir < 0) {
+					warn("%s", to.base);
+					(void)rmdir(to.base);
+					(void)fts_set(ftsp, curr, FTS_SKIP);
+					badcp = rval = 1;
+					continue;
+				}
+				if (fstat(to.dir, &created_root_stat) != 0) {
+					warn("%s", to.base);
+					(void)close(to.dir);
+					(void)rmdir(to.base);
+					(void)fts_set(ftsp, curr, FTS_SKIP);
+					to.dir = -1;
+					badcp = rval = 1;
+					continue;
+				}
+				root_stat = &created_root_stat;
+			} else {
+				/* entering a directory; append its name to to.path */
+				len = snprintf(to.end, END(to.path) - to.end, "%s%s",
+				    to.end > to.path ? "/" : "", curr->fts_name);
+				if (to.end + len >= END(to.path)) {
+					*to.end = '\0';
+					warnc(ENAMETOOLONG, "%s/%s%s%s", to.base,
+					    to.path, to.end > to.path ? "/" : "",
+					    curr->fts_name);
 					fts_set(ftsp, curr, FTS_SKIP);
+					badcp = rval = 1;
 					continue;
 				}
-
-				if (asprintf(&recurse_path, "%s/%s", to.p_path,
-				    rootname) == -1)
-					err(1, "asprintf");
+				to.end += len;
 			}
-
-			if (recurse_path != NULL &&
-			    strcmp(to.p_path, recurse_path) == 0) {
+			skipdp = false;
+                        /*
+                         * We're on the verge of recursing on ourselves.
+                         * Either we need to stop right here (we knowingly
+                         * just created it), or we will in an immediate
+                         * descendant.  Record the path of the immediate
+                         * descendant to make our lives a little less
+                         * complicated looking.
+                         */
+			if (type != FILE_TO_FILE &&
+			    root_stat->st_dev == curr_stat->st_dev &&
+			    root_stat->st_ino == curr_stat->st_ino) {
+				assert(recpath == NULL);
+				if (root_stat == &created_root_stat) {
+                                        /*
+                                         * This directory didn't exist
+                                         * when we started, we created it
+                                         * as part of traversal.  Stop
+                                         * right here before we do
+                                         * something silly.
+                                         */
+					(void)fts_set(ftsp, curr, FTS_SKIP);
+					continue;
+				}
+				if (asprintf(&recpath, "%s/%s", to.path,
+				    rootname) < 0) {
+					warnc(ENOMEM, NULL);
+					(void)fts_set(ftsp, curr, FTS_SKIP);
+					badcp = rval = 1;
+					continue;
+				}
+			}
+			if (recpath != NULL &&
+			    strcmp(recpath, to.path) == 0) {
 				fts_set(ftsp, curr, FTS_SKIP);
 				continue;
 			}
-		}
-
-		if (curr->fts_info == FTS_DP) {
+			break;
+		case FTS_DP:
 			/*
 			 * We are nearly finished with this directory.  If we
 			 * didn't actually copy it, or otherwise don't need to
 			 * change its attributes, then we are done.
-			 */
-			if (!curr->fts_number)
-				continue;
-			/*
+			 *
 			 * If -p is in effect, set all the attributes.
 			 * Otherwise, set the correct permissions, limited
 			 * by the umask.  Optimise by avoiding a chmod()
@@ -415,41 +422,86 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 			 * honour setuid, setgid and sticky bits, but we
 			 * normally want to preserve them on directories.
 			 */
-			if (pflag) {
-				if (setfile(curr->fts_statp, -1))
+			if (curr->fts_number && pflag) {
+				int fd = *to.path ? -1 : to.dir;
+				if (setfile(curr_stat, fd, true))
+					rval = 1;
+				if (preserve_dir_acls(curr->fts_accpath,
+				    to.path) != 0)
 					rval = 1;
-				if (preserve_dir_acls(curr->fts_statp,
-				    curr->fts_accpath, to.p_path) != 0)
+			} else if (curr->fts_number) {
+				const char *path = *to.path ? to.path : dot;
+				mode = curr_stat->st_mode;
+				if (((mode & (S_ISUID | S_ISGID | S_ISTXT)) ||
+				    ((mode | S_IRWXU) & mask) != (mode & mask)) &&
+				    fchmodat(to.dir, path, mode & mask, 0) != 0) {
+					warn("chmod: %s/%s", to.base, to.path);
 					rval = 1;
+				}
+			}
+			/* are we leaving a directory we failed to enter? */
+			if (skipdp)
+				continue;
+			/* leaving a directory; remove its name from to.path */
+			if (type == DIR_TO_DNE &&
+			    curr->fts_level == FTS_ROOTLEVEL) {
+				/* this is actually our created root */
 			} else {
-				mode = curr->fts_statp->st_mode;
-				if ((mode & (S_ISUID | S_ISGID | S_ISTXT)) ||
-				    ((mode | S_IRWXU) & mask) != (mode & mask))
-					if (chmod(to.p_path, mode & mask) !=
-					    0) {
-						warn("chmod: %s", to.p_path);
-						rval = 1;
-					}
+				while (to.end > to.path && *to.end != '/')
+					to.end--;
+				assert(strcmp(to.end + (*to.end == '/'), curr->fts_name) == 0);
+				*to.end = '\0';
 			}
 			continue;
+		default:
+			/* something else: append its name to to.path */
+			if (type == FILE_TO_FILE)
+				break;
+			len = snprintf(to.end, END(to.path) - to.end, "%s%s",
+			    to.end > to.path ? "/" : "", curr->fts_name);
+			if (to.end + len >= END(to.path)) {
+				*to.end = '\0';
+				warnc(ENAMETOOLONG, "%s/%s%s%s", to.base,
+				    to.path, to.end > to.path ? "/" : "",
+				    curr->fts_name);
+				badcp = rval = 1;
+				continue;
+			}
+			/* intentionally do not update to.end */
+			break;
+		}
+
+		/* Not an error but need to remember it happened. */
+		if (to.path[0] == '\0') {
+			/*
+			 * This can happen in two cases:
+			 * - DIR_TO_DNE; we created the directory and
+                         *   populated root_stat earlier.
+			 * - FILE_TO_DIR if a source has a trailing slash;
+                         *   the caller populated root_stat.
+			 */
+			dne = false;
+			to_stat = *root_stat;
+		} else {
+			atflags = beneath ? AT_RESOLVE_BENEATH : 0;
+			if (curr->fts_info == FTS_D || curr->fts_info == FTS_SL)
+				atflags |= AT_SYMLINK_NOFOLLOW;
+			dne = fstatat(to.dir, to.path, &to_stat, atflags) != 0;
 		}
 
 		/* Check if source and destination are identical. */
-		if (stat(to.p_path, &to_stat) == 0 &&
-		    to_stat.st_dev == curr->fts_statp->st_dev &&
-		    to_stat.st_ino == curr->fts_statp->st_ino) {
-			warnx("%s and %s are identical (not copied).",
-			    to.p_path, curr->fts_path);
+		if (!dne &&
+		    to_stat.st_dev == curr_stat->st_dev &&
+		    to_stat.st_ino == curr_stat->st_ino) {
+			warnx("%s/%s and %s are identical (not copied).",
+			    to.base, to.path, curr->fts_path);
 			badcp = rval = 1;
-			if (S_ISDIR(curr->fts_statp->st_mode))
+			if (S_ISDIR(curr_stat->st_mode))
 				(void)fts_set(ftsp, curr, FTS_SKIP);
 			continue;
 		}
 
-		/* Not an error but need to remember it happened. */
-		dne = lstat(to.p_path, &to_stat) != 0;
-
-		switch (curr->fts_statp->st_mode & S_IFMT) {
+		switch (curr_stat->st_mode & S_IFMT) {
 		case S_IFLNK:
 			if ((fts_options & FTS_LOGICAL) ||
 			    ((fts_options & FTS_COMFOLLOW) &&
@@ -460,11 +512,11 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 				 * nonexistent or inaccessible.  Let
 				 * copy_file() deal with the error.
 				 */
-				if (copy_file(curr, dne))
+				if (copy_file(curr, dne, beneath))
 					badcp = rval = 1;
 			} else {
 				/* Copy the link. */
-				if (copy_link(curr, !dne))
+				if (copy_link(curr, dne, beneath))
 					badcp = rval = 1;
 			}
 			break;
@@ -485,31 +537,15 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 			 * umask blocks owner writes, we fail.
 			 */
 			if (dne) {
-				mode = curr->fts_statp->st_mode | S_IRWXU;
-				if (mkdir(to.p_path, mode) != 0) {
-					warn("%s", to.p_path);
-					(void)fts_set(ftsp, curr, FTS_SKIP);
-					badcp = rval = 1;
-					break;
-				}
-				/*
-				 * First DNE with a NULL root_stat is the root
-				 * path, so set root_stat.  We can't really
-				 * tell in all cases if the target path is
-				 * within the src path, so we just stat() the
-				 * first directory we created and use that.
-				 */
-				if (root_stat == NULL &&
-				    stat(to.p_path, &created_root_stat) != 0) {
-					warn("%s", to.p_path);
+				mode = curr_stat->st_mode | S_IRWXU;
+				if (mkdirat(to.dir, to.path, mode) != 0) {
+					warn("%s/%s", to.base, to.path);
 					(void)fts_set(ftsp, curr, FTS_SKIP);
 					badcp = rval = 1;
 					break;
 				}
-				if (root_stat == NULL)
-					root_stat = &created_root_stat;
 			} else if (!S_ISDIR(to_stat.st_mode)) {
-				warnc(ENOTDIR, "%s", to.p_path);
+				warnc(ENOTDIR, "%s/%s", to.base, to.path);
 				(void)fts_set(ftsp, curr, FTS_SKIP);
 				badcp = rval = 1;
 				break;
@@ -524,10 +560,10 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 		case S_IFBLK:
 		case S_IFCHR:
 			if (Rflag && !sflag) {
-				if (copy_special(curr->fts_statp, !dne))
+				if (copy_special(curr_stat, dne, beneath))
 					badcp = rval = 1;
 			} else {
-				if (copy_file(curr, dne))
+				if (copy_file(curr, dne, beneath))
 					badcp = rval = 1;
 			}
 			break;
@@ -537,25 +573,26 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 			break;
 		case S_IFIFO:
 			if (Rflag && !sflag) {
-				if (copy_fifo(curr->fts_statp, !dne))
+				if (copy_fifo(curr_stat, dne, beneath))
 					badcp = rval = 1;
 			} else {
-				if (copy_file(curr, dne))
+				if (copy_file(curr, dne, beneath))
 					badcp = rval = 1;
 			}
 			break;
 		default:
-			if (copy_file(curr, dne))
+			if (copy_file(curr, dne, beneath))
 				badcp = rval = 1;
 			break;
 		}
 		if (vflag && !badcp)
-			(void)printf("%s -> %s\n", curr->fts_path, to.p_path);
+			(void)printf("%s -> %s/%s\n", curr->fts_path, to.base, to.path);
 	}
 	if (errno)
 		err(1, "fts_read");
 	fts_close(ftsp);
-	free(recurse_path);
+	close(to.dir);
+	free(recpath);
 	return (rval);
 }
 
diff --git a/bin/cp/extern.h b/bin/cp/extern.h
index 272454bb5871..5a18f91ef13c 100644
--- a/bin/cp/extern.h
+++ b/bin/cp/extern.h
@@ -30,9 +30,10 @@
  */
 
 typedef struct {
-	char	*p_end;			/* pointer to NULL at end of path */
-	char	*target_end;		/* pointer to end of target base */
-	char	p_path[PATH_MAX];	/* pointer to the start of a path */
+	int		 dir;		/* base directory handle */
+	char		*end;		/* pointer to NUL at end of path */
+	char		 base[PATH_MAX];	/* base directory path */
+	char		 path[PATH_MAX];	/* target path */
 } PATH_T;
 
 extern PATH_T to;
@@ -40,12 +41,24 @@ extern int Nflag, fflag, iflag, lflag, nflag, pflag, sflag, vflag;
 extern volatile sig_atomic_t info;
 
 __BEGIN_DECLS
-int	copy_fifo(struct stat *, int);
-int	copy_file(const FTSENT *, int);
-int	copy_link(const FTSENT *, int);
-int	copy_special(struct stat *, int);
-int	setfile(struct stat *, int);
-int	preserve_dir_acls(struct stat *, char *, char *);
+int	copy_fifo(struct stat *, bool, bool);
+int	copy_file(const FTSENT *, bool, bool);
+int	copy_link(const FTSENT *, bool, bool);
+int	copy_special(struct stat *, bool, bool);
+int	setfile(struct stat *, int, bool);
+int	preserve_dir_acls(const char *, const char *);
 int	preserve_fd_acls(int, int);
 void	usage(void) __dead2;
 __END_DECLS
+
+/*
+ * The FreeBSD and Darwin kernels return ENOTCAPABLE when a path lookup
+ * violates a RESOLVE_BENEATH constraint.  This results in confusing error
+ * messages, so translate it to the more widely recognized EACCES.
+ */
+#ifdef ENOTCAPABLE
+#define warn(...)							\
+	warnc(errno == ENOTCAPABLE ? EACCES : errno, __VA_ARGS__)
+#define err(rv, ...)							\
+	errc(rv, errno == ENOTCAPABLE ? EACCES : errno, __VA_ARGS__)
+#endif
diff --git a/bin/cp/tests/cp_test.sh b/bin/cp/tests/cp_test.sh
index 5c581e06ab8e..6644588f1ce8 100755
--- a/bin/cp/tests/cp_test.sh
+++ b/bin/cp/tests/cp_test.sh
@@ -179,6 +179,52 @@ matching_srctgt_nonexistent_body()
 	atf_check -e not-empty -s not-exit:0 stat foo/dne/foo
 }
 
+atf_test_case pflag_acls
+pflag_acls_body()
+{
+	mkdir dir
+	echo "hello" >dir/file
+	if ! setfacl -m g:staff:D::allow dir ||
+	   ! setfacl -m g:staff:d::allow dir/file ; then
+		atf_skip "file system does not support ACLs"
+	fi
+	atf_check cp -p dir/file dst
+	atf_check -o match:"group:staff:-+d-+" getfacl dst
+	rm dst
+	mkdir dst
+	atf_check cp -rp dir dst
+	atf_check -o not-match:"group:staff:-+D-+" getfacl dst
+	atf_check -o match:"group:staff:-+D-+" getfacl dst/dir
+	atf_check -o match:"group:staff:-+d-+" getfacl dst/dir/file
+	rm -rf dst
+	atf_check cp -rp dir dst
+	atf_check -o match:"group:staff:-+D-+" getfacl dst/
+	atf_check -o match:"group:staff:-+d-+" getfacl dst/file
+}
+
+atf_test_case pflag_flags
+pflag_flags_body()
+{
+	mkdir dir
+	echo "hello" >dir/file
+	if ! chflags nodump dir ||
+	   ! chflags nodump dir/file ; then
+		atf_skip "file system does not support flags"
+	fi
+	atf_check cp -p dir/file dst
+	atf_check -o match:"nodump" stat -f%Sf dst
+	rm dst
+	mkdir dst
+	atf_check cp -rp dir dst
+	atf_check -o not-match:"nodump" stat -f%Sf dst
+	atf_check -o match:"nodump" stat -f%Sf dst/dir
+	atf_check -o match:"nodump" stat -f%Sf dst/dir/file
+	rm -rf dst
+	atf_check cp -rp dir dst
+	atf_check -o match:"nodump" stat -f%Sf dst
+	atf_check -o match:"nodump" stat -f%Sf dst/file
+}
+
 recursive_link_setup()
 {
 	extra_cpflag=$1
@@ -392,6 +438,120 @@ overwrite_directory_body()
 	    cp -r bar foo
 }
 
+atf_test_case to_dir_dne
+to_dir_dne_body()
+{
+	mkdir dir
+	echo "foo" >dir/foo
+	atf_check cp -r dir dne
+	atf_check test -d dne
+	atf_check test -f dne/foo
+	atf_check cmp dir/foo dne/foo
+}
+
+atf_test_case to_nondir
+to_nondir_body()
+{
+	echo "foo" >foo
+	echo "bar" >bar
+	echo "baz" >baz
+	# This is described as “case 1” in source code comments
+	atf_check cp foo bar
+	atf_check cmp -s foo bar
+	# This is “case 2”, the target must be a directory
+	atf_check -s not-exit:0 -e match:"Not a directory" \
+	    cp foo bar baz
+}
+
+atf_test_case to_deadlink
+to_deadlink_body()
+{
+	echo "foo" >foo
+	ln -s bar baz
+	atf_check cp foo baz
+	atf_check cmp -s foo bar
+}
+
+atf_test_case to_deadlink_append
+to_deadlink_append_body()
+{
+	echo "foo" >foo
+	mkdir bar
+	ln -s baz bar/foo
+	atf_check cp foo bar
+	atf_check cmp -s foo bar/foo
+	rm -f bar/foo
+	atf_check cp foo bar/
+	atf_check cmp -s foo bar/foo
+}
+
+atf_test_case to_dirlink
+to_dirlink_body()
+{
+	mkdir src dir
+	echo "foo" >src/file
+	ln -s dir dst
+	atf_check cp -r src dst
+	atf_check cmp -s src/file dir/src/file
+	rm -r dir/*
+	atf_check cp -r src dst/
+	atf_check cmp -s src/file dir/src/file
+	rm -r dir/*
+	# If the source is a directory and ends in a slash, our cp has
+	# traditionally copied the contents of the source rather than
+	# the source itself.  It is unclear whether this is intended
+	# or simply a consequence of how FTS handles the situation.
+	# Notably, GNU cp does not behave in this manner.
+	atf_check cp -r src/ dst
+	atf_check cmp -s src/file dir/file
+	rm -r dir/*
+	atf_check cp -r src/ dst/
+	atf_check cmp -s src/file dir/file
+	rm -r dir/*
+}
+
+atf_test_case to_deaddirlink
+to_deaddirlink_body()
+{
+	mkdir src
+	echo "foo" >src/file
+	ln -s dir dst
+	# It is unclear which error we should expect in these cases.
+	# Our current implementation always reports ENOTDIR, but one
+	# might be equally justified in expecting EEXIST or ENOENT.
+	# GNU cp reports EEXIST when the destination is given with a
+	# trailing slash and “cannot overwrite non-directory with
+	# directory” otherwise.
+	atf_check -s not-exit:0 -e ignore \
+	    cp -r src dst
+	atf_check -s not-exit:0 -e ignore \
+	    cp -r src dst/
+	atf_check -s not-exit:0 -e ignore \
+	    cp -r src/ dst
+	atf_check -s not-exit:0 -e ignore \
+	    cp -r src/ dst/
+	atf_check -s not-exit:0 -e ignore \
+	    cp -R src dst
+	atf_check -s not-exit:0 -e ignore \
+	    cp -R src dst/
+	atf_check -s not-exit:0 -e ignore \
+	    cp -R src/ dst
+	atf_check -s not-exit:0 -e ignore \
+	    cp -R src/ dst/
+}
+
+atf_test_case to_link_outside
+to_link_outside_body()
+{
+	mkdir dir dst dst/dir
+	echo "foo" >dir/file
+	ln -s ../../file dst/dir/file
+	atf_check \
+	    -s exit:1 \
+	    -e match:"dst/dir/file: Permission denied" \
+	    cp -r dir dst
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case basic
@@ -404,6 +564,8 @@ atf_init_test_cases()
 	atf_add_test_case matching_srctgt_contained
 	atf_add_test_case matching_srctgt_link
 	atf_add_test_case matching_srctgt_nonexistent
+	atf_add_test_case pflag_acls
+	atf_add_test_case pflag_flags
 	atf_add_test_case recursive_link_dflt
 	atf_add_test_case recursive_link_Hflag
 	atf_add_test_case recursive_link_Lflag
@@ -419,4 +581,11 @@ atf_init_test_cases()
 	atf_add_test_case symlink_exists_force
 	atf_add_test_case directory_to_symlink
 	atf_add_test_case overwrite_directory
+	atf_add_test_case to_dir_dne
+	atf_add_test_case to_nondir
+	atf_add_test_case to_deadlink
+	atf_add_test_case to_deadlink_append
+	atf_add_test_case to_dirlink
+	atf_add_test_case to_deaddirlink
+	atf_add_test_case to_link_outside
 }
diff --git a/bin/cp/utils.c b/bin/cp/utils.c
index cfbb2022caaf..a849899af7ee 100644
--- a/bin/cp/utils.c
+++ b/bin/cp/utils.c
@@ -38,6 +38,7 @@
 #include <fcntl.h>
 #include <fts.h>
 #include <limits.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <sysexits.h>
@@ -98,7 +99,7 @@ copy_fallback(int from_fd, int to_fd)
 }
 
 int
-copy_file(const FTSENT *entp, int dne)
+copy_file(const FTSENT *entp, bool dne, bool beneath)
 {
 	struct stat sb, *fs;
 	ssize_t wcount;
@@ -142,12 +143,13 @@ copy_file(const FTSENT *entp, int dne)
 	if (!dne) {
 		if (nflag) {
 			if (vflag)
-				printf("%s not overwritten\n", to.p_path);
+				printf("%s/%s not overwritten\n",
+				    to.base, to.path);
 			rval = 1;
 			goto done;
 		} else if (iflag) {
-			(void)fprintf(stderr, "overwrite %s? %s", 
-			    to.p_path, YESNO);
+			(void)fprintf(stderr, "overwrite %s/%s? %s",
+			    to.base, to.path, YESNO);
 			checkch = ch = getchar();
 			while (ch != '\n' && ch != EOF)
 				ch = getchar();
@@ -160,7 +162,8 @@ copy_file(const FTSENT *entp, int dne)
 
 		if (fflag) {
 			/* remove existing destination file */
-			(void)unlink(to.p_path);
+			(void)unlinkat(to.dir, to.path,
+			    beneath ? AT_RESOLVE_BENEATH : 0);
 			dne = 1;
 		}
 	}
@@ -168,16 +171,16 @@ copy_file(const FTSENT *entp, int dne)
 	rval = 0;
 
 	if (lflag) {
-		if (link(entp->fts_path, to.p_path) != 0) {
-			warn("%s", to.p_path);
+		if (linkat(AT_FDCWD, entp->fts_path, to.dir, to.path, 0) != 0) {
+			warn("%s/%s", to.base, to.path);
 			rval = 1;
 		}
 		goto done;
 	}
 
 	if (sflag) {
-		if (symlink(entp->fts_path, to.p_path) != 0) {
-			warn("%s", to.p_path);
+		if (symlinkat(entp->fts_path, to.dir, to.path) != 0) {
+			warn("%s/%s", to.base, to.path);
 			rval = 1;
 		}
 		goto done;
@@ -185,14 +188,17 @@ copy_file(const FTSENT *entp, int dne)
 
 	if (!dne) {
 		/* overwrite existing destination file */
-		to_fd = open(to.p_path, O_WRONLY | O_TRUNC, 0);
+		to_fd = openat(to.dir, to.path,
+		    O_WRONLY | O_TRUNC | (beneath ? O_RESOLVE_BENEATH : 0), 0);
 	} else {
 		/* create new destination file */
-		to_fd = open(to.p_path, O_WRONLY | O_TRUNC | O_CREAT,
+		to_fd = openat(to.dir, to.path,
+		    O_WRONLY | O_TRUNC | O_CREAT |
+		    (beneath ? O_RESOLVE_BENEATH : 0),
 		    fs->st_mode & ~(S_ISUID | S_ISGID));
 	}
 	if (to_fd == -1) {
-		warn("%s", to.p_path);
+		warn("%s/%s", to.base, to.path);
 		rval = 1;
 		goto done;
 	}
@@ -214,8 +220,8 @@ copy_file(const FTSENT *entp, int dne)
 		if (info) {
 			info = 0;
 			(void)fprintf(stderr,
-			    "%s -> %s %3d%%\n",
-			    entp->fts_path, to.p_path,
+			    "%s -> %s/%s %3d%%\n",
+			    entp->fts_path, to.base, to.path,
 			    cp_pct(wtotal, fs->st_size));
 		}
 	} while (wcount > 0);
@@ -230,12 +236,12 @@ copy_file(const FTSENT *entp, int dne)
 	 * or its contents might be irreplaceable.  It would only be safe
 	 * to remove it if we created it and its length is 0.
 	 */
-	if (pflag && setfile(fs, to_fd))
+	if (pflag && setfile(fs, to_fd, beneath))
 		rval = 1;
 	if (pflag && preserve_fd_acls(from_fd, to_fd) != 0)
 		rval = 1;
 	if (close(to_fd)) {
-		warn("%s", to.p_path);
+		warn("%s/%s", to.base, to.path);
 		rval = 1;
 	}
 
@@ -246,14 +252,15 @@ done:
 }
 
 int
-copy_link(const FTSENT *p, int exists)
+copy_link(const FTSENT *p, bool dne, bool beneath)
 {
 	ssize_t len;
+	int atflags = beneath ? AT_RESOLVE_BENEATH : 0;
 	char llink[PATH_MAX];
 
-	if (exists && nflag) {
+	if (!dne && nflag) {
 		if (vflag)
-			printf("%s not overwritten\n", to.p_path);
+			printf("%s/%s not overwritten\n", to.base, to.path);
 		return (1);
 	}
 	if ((len = readlink(p->fts_path, llink, sizeof(llink) - 1)) == -1) {
@@ -261,81 +268,86 @@ copy_link(const FTSENT *p, int exists)
 		return (1);
*** 335 LINES SKIPPED ***



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202505061712.546HCLPr088289>