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:		0729d1e8fd
MFC after:	never
Relnotes:	yes
Sponsored by:	Klara, Inc.
Reviewed by:	markj
Differential Revision:	https://reviews.freebsd.org/D50093

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) {					\
+static char dot[] = ".";
-	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, "" };
 
+#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;
+	int ch, fts_options, r;
-	char *target;
+	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))
+	if (*target == '\0') {
-		errx(1, "%s: name too long", target);
+		target = dot;
-	to.p_end = to.p_path + strlen(to.p_path);
+	} else if ((sep = strrchr(target, '/')) != NULL && sep[1] == '\0') {
-	if (to.p_path == to.p_end) {
+		have_trailing_slash = true;
-		*to.p_end++ = '.';
+		while (sep > target + 1 && *(sep - 1) == '/')
-		*to.p_end = 0;
+			sep--;
+		*sep = '\0';
 	}
-	have_trailing_slash = (to.p_end[-1] == '/');
+	if (strlcpy(to.base, target, sizeof(to.base)) >= sizeof(to.base))
-	if (have_trailing_slash)
+		errc(1, ENAMETOOLONG, "%s", target);
-		STRIP_TRAILING_SLASH(to);
-	to.target_end = to.p_end;
 
 	/* 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;
+	char *recpath = NULL;
-	size_t nlen;
+	int atflags, dne, badcp, len, rval;
-	char *p, *recurse_path, *target_mid;
 	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:
+		case FTS_D:
-			;
-		}
-
-		/*
-		 * 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) {
 			/*
-			 * Need to remember the roots of traversals to create
+			 * Stash the root basename off for detecting
-			 * correct pathnames.  If there's a directory being
+			 * recursion later.
-			 * 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
+			 * This will be essential if the root is a symlink
-			 * concatenation for paths ending in "..", e.g.
+			 * and we're rolling with -L or -H.  The later
-			 *	cp -R .. /tmp
+			 * bits will need this bit in particular.
-			 * Paths ending in ".." are changed to ".".  This is
-			 * tricky, but seems the easiest way to fix the problem.
-			 *
-			 * XXX
-			 * Since the first level MUST be FTS_ROOTLEVEL, base
-			 * is always initialized.
 			 */
 			if (curr->fts_level == FTS_ROOTLEVEL) {
-				if (type != DIR_TO_DNE) {
+				strlcpy(rootname, curr->fts_name,
-					p = strrchr(curr->fts_path, '/');
+				    sizeof(rootname));
-					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;
-			}
-			(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
+			 * If we FTS_SKIP while handling FTS_D, we will
-			 * we need to stop right here (we knowingly just created
+			 * immediately get FTS_DP for the same directory.
-			 * it), or we will in an immediate descendant.  Record
+			 * If this happens before we've appended the name
-			 * the path of the immediate descendant to make our
+			 * to to.path, we need to remember not to perform
-			 * lives a little less complicated looking.
+			 * the reverse operation.
 			 */
-			if (curr->fts_info == FTS_D && root_stat != NULL &&
+			skipdp = true;
-			    root_stat->st_dev == curr->fts_statp->st_dev &&
+			/* we must have a destination! */
-			    root_stat->st_ino == curr->fts_statp->st_ino) {
+			if (type == DIR_TO_DNE &&
-				assert(recurse_path == NULL);
+			    curr->fts_level == FTS_ROOTLEVEL) {
-
+				assert(to.dir < 0);
-				if (root_stat == &created_root_stat) {
+				assert(root_stat == NULL);
-					/*
+				mode = curr_stat->st_mode | S_IRWXU;
-					 * This directory didn't exist when we
+				if (mkdir(to.base, mode) != 0) {
-					 * started, we created it as part of
+					warn("%s", to.base);
-					 * traversal.  Stop right here before we
+					(void)fts_set(ftsp, curr, FTS_SKIP);
-					 * do something silly.
+					badcp = rval = 1;
-					 */
-					fts_set(ftsp, curr, FTS_SKIP);
 					continue;
 				}
-
+				to.dir = open(to.base, O_DIRECTORY | O_SEARCH);
-				if (asprintf(&recurse_path, "%s/%s", to.p_path,
+				if (to.dir < 0) {
-				    rootname) == -1)
+					warn("%s", to.base);
-					err(1, "asprintf");
+					(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;
+				}
+				to.end += len;
 			}
-
+			skipdp = false;
-			if (recurse_path != NULL &&
+                        /*
-			    strcmp(to.p_path, recurse_path) == 0) {
+                         * 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;
 			}
-		}
+			break;
-
+		case FTS_DP:
-		if (curr->fts_info == 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 (curr->fts_number && pflag) {
-				if (setfile(curr->fts_statp, -1))
+				int fd = *to.path ? -1 : to.dir;
+				if (setfile(curr_stat, fd, true))
 					rval = 1;
-				if (preserve_dir_acls(curr->fts_statp,
+				if (preserve_dir_acls(curr->fts_accpath,
-				    curr->fts_accpath, to.p_path) != 0)
+				    to.path) != 0)
 					rval = 1;
+			} 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;
+				while (to.end > to.path && *to.end != '/')
-				if ((mode & (S_ISUID | S_ISGID | S_ISTXT)) ||
+					to.end--;
-				    ((mode | S_IRWXU) & mask) != (mode & mask))
+				assert(strcmp(to.end + (*to.end == '/'), curr->fts_name) == 0);
-					if (chmod(to.p_path, mode & mask) !=
+				*to.end = '\0';
-					    0) {
-						warn("chmod: %s", to.p_path);
-						rval = 1;
-					}
 			}
 			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 &&
+		if (!dne &&
-		    to_stat.st_dev == curr->fts_statp->st_dev &&
+		    to_stat.st_dev == curr_stat->st_dev &&
-		    to_stat.st_ino == curr->fts_statp->st_ino) {
+		    to_stat.st_ino == curr_stat->st_ino) {
-			warnx("%s and %s are identical (not copied).",
+			warnx("%s/%s and %s are identical (not copied).",
-			    to.p_path, curr->fts_path);
+			    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. */
+		switch (curr_stat->st_mode & S_IFMT) {
-		dne = lstat(to.p_path, &to_stat) != 0;
-
-		switch (curr->fts_statp->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;
+				mode = curr_stat->st_mode | S_IRWXU;
-				if (mkdir(to.p_path, mode) != 0) {
+				if (mkdirat(to.dir, to.path, mode) != 0) {
-					warn("%s", to.p_path);
+					warn("%s/%s", to.base, to.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);
-					(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);
 }
 

bin/cp/extern.h

@@ -30,9 +30,10 @@
  */
 
 typedef struct {
-	char	*p_end;			/* pointer to NULL at end of path */
+	int		 dir;		/* base directory handle */
-	char	*target_end;		/* pointer to end of target base */
+	char		*end;		/* pointer to NUL at end of path */
-	char	p_path[PATH_MAX];	/* pointer to the start of a 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_fifo(struct stat *, bool, bool);
-int	copy_file(const FTSENT *, int);
+int	copy_file(const FTSENT *, bool, bool);
-int	copy_link(const FTSENT *, int);
+int	copy_link(const FTSENT *, bool, bool);
-int	copy_special(struct stat *, int);
+int	copy_special(struct stat *, bool, bool);
-int	setfile(struct stat *, int);
+int	setfile(struct stat *, int, bool);
-int	preserve_dir_acls(struct stat *, char *, char *);
+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

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
 }

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", 
+			(void)fprintf(stderr, "overwrite %s/%s? %s",
-			    to.p_path, YESNO);
+			    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) {
+		if (linkat(AT_FDCWD, entp->fts_path, to.dir, to.path, 0) != 0) {
-			warn("%s", to.p_path);
+			warn("%s/%s", to.base, to.path);
 			rval = 1;
 		}
 		goto done;
 	}
 
 	if (sflag) {
-		if (symlink(entp->fts_path, to.p_path) != 0) {
+		if (symlinkat(entp->fts_path, to.dir, to.path) != 0) {
-			warn("%s", to.p_path);
+			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",
+			    "%s -> %s/%s %3d%%\n",
-			    entp->fts_path, to.p_path,
+			    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 @@ copy_file(const FTSENT *entp, int dne)
 }
 
 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);
 	}
 	llink[len] = '\0';
-	if (exists && unlink(to.p_path)) {
+	if (!dne && unlinkat(to.dir, to.path, atflags) != 0) {
-		warn("unlink: %s", to.p_path);
+		warn("unlink: %s/%s", to.base, to.path);
 		return (1);
 	}
-	if (symlink(llink, to.p_path)) {
+	if (symlinkat(llink, to.dir, to.path) != 0) {
 		warn("symlink: %s", llink);
 		return (1);
 	}
-	return (pflag ? setfile(p->fts_statp, -1) : 0);
+	return (pflag ? setfile(p->fts_statp, -1, beneath) : 0);
 }
 
 int
-copy_fifo(struct stat *from_stat, int exists)
+copy_fifo(struct stat *from_stat, bool dne, bool beneath)
 {
+	int atflags = beneath ? AT_RESOLVE_BENEATH : 0;
 
-	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 (exists && unlink(to.p_path)) {
+	if (!dne && unlinkat(to.dir, to.path, atflags) != 0) {
-		warn("unlink: %s", to.p_path);
+		warn("unlink: %s/%s", to.base, to.path);
 		return (1);
 	}
-	if (mkfifo(to.p_path, from_stat->st_mode)) {
+	if (mkfifoat(to.dir, to.path, from_stat->st_mode) != 0) {
-		warn("mkfifo: %s", to.p_path);
+		warn("mkfifo: %s/%s", to.base, to.path);
 		return (1);
 	}
-	return (pflag ? setfile(from_stat, -1) : 0);
+	return (pflag ? setfile(from_stat, -1, beneath) : 0);
 }
 
 int
-copy_special(struct stat *from_stat, int exists)
+copy_special(struct stat *from_stat, bool dne, bool beneath)
 {
+	int atflags = beneath ? AT_RESOLVE_BENEATH : 0;
 
-	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 (exists && unlink(to.p_path)) {
+	if (!dne && unlinkat(to.dir, to.path, atflags) != 0) {
-		warn("unlink: %s", to.p_path);
+		warn("unlink: %s/%s", to.base, to.path);
 		return (1);
 	}
-	if (mknod(to.p_path, from_stat->st_mode, from_stat->st_rdev)) {
+	if (mknodat(to.dir, to.path, from_stat->st_mode, from_stat->st_rdev) != 0) {
-		warn("mknod: %s", to.p_path);
+		warn("mknod: %s/%s", to.base, to.path);
 		return (1);
 	}
-	return (pflag ? setfile(from_stat, -1) : 0);
+	return (pflag ? setfile(from_stat, -1, beneath) : 0);
 }
 
 int
-setfile(struct stat *fs, int fd)
+setfile(struct stat *fs, int fd, bool beneath)
 {
 	static struct timespec tspec[2];
 	struct stat ts;
+	int atflags = beneath ? AT_RESOLVE_BENEATH : 0;
 	int rval, gotstat, islink, fdval;
 
 	rval = 0;
 	fdval = fd != -1;
 	islink = !fdval && S_ISLNK(fs->st_mode);
+	if (islink)
+		atflags |= AT_SYMLINK_NOFOLLOW;
 	fs->st_mode &= S_ISUID | S_ISGID | S_ISVTX |
 	    S_IRWXU | S_IRWXG | S_IRWXO;
 
 	tspec[0] = fs->st_atim;
 	tspec[1] = fs->st_mtim;
-	if (fdval ? futimens(fd, tspec) : utimensat(AT_FDCWD, to.p_path, tspec,
+	if (fdval ? futimens(fd, tspec) :
-	    islink ? AT_SYMLINK_NOFOLLOW : 0)) {
+	    utimensat(to.dir, to.path, tspec, atflags)) {
-		warn("utimensat: %s", to.p_path);
+		warn("utimensat: %s/%s", to.base, to.path);
 		rval = 1;
 	}
 	if (fdval ? fstat(fd, &ts) :
-	    (islink ? lstat(to.p_path, &ts) : stat(to.p_path, &ts)))
+	    fstatat(to.dir, to.path, &ts, atflags)) {
 		gotstat = 0;
-	else {
+	} else {
 		gotstat = 1;
 		ts.st_mode &= S_ISUID | S_ISGID | S_ISVTX |
 		    S_IRWXU | S_IRWXG | S_IRWXO;
@@ -346,30 +358,28 @@ setfile(struct stat *fs, int fd)
 	 * the mode; current BSD behavior is to remove all setuid bits on
 	 * chown.  If chown fails, lose setuid/setgid bits.
 	 */
-	if (!gotstat || fs->st_uid != ts.st_uid || fs->st_gid != ts.st_gid)
+	if (!gotstat || fs->st_uid != ts.st_uid || fs->st_gid != ts.st_gid) {
 		if (fdval ? fchown(fd, fs->st_uid, fs->st_gid) :
-		    (islink ? lchown(to.p_path, fs->st_uid, fs->st_gid) :
+		    fchownat(to.dir, to.path, fs->st_uid, fs->st_gid, atflags)) {
-		    chown(to.p_path, fs->st_uid, fs->st_gid))) {
 			if (errno != EPERM) {
-				warn("chown: %s", to.p_path);
+				warn("chown: %s/%s", to.base, to.path);
 				rval = 1;
 			}
 			fs->st_mode &= ~(S_ISUID | S_ISGID);
 		}
+	}
 
-	if (!gotstat || fs->st_mode != ts.st_mode)
+	if (!gotstat || fs->st_mode != ts.st_mode) {
 		if (fdval ? fchmod(fd, fs->st_mode) :
-		    (islink ? lchmod(to.p_path, fs->st_mode) :
+		    fchmodat(to.dir, to.path, fs->st_mode, atflags)) {
-		    chmod(to.p_path, fs->st_mode))) {
+			warn("chmod: %s/%s", to.base, to.path);
-			warn("chmod: %s", to.p_path);
 			rval = 1;
 		}
+	}
 
-	if (!Nflag && (!gotstat || fs->st_flags != ts.st_flags))
+	if (!Nflag && (!gotstat || fs->st_flags != ts.st_flags)) {
-		if (fdval ?
+		if (fdval ? fchflags(fd, fs->st_flags) :
-		    fchflags(fd, fs->st_flags) :
+		    chflagsat(to.dir, to.path, fs->st_flags, atflags)) {
-		    (islink ? lchflags(to.p_path, fs->st_flags) :
-		    chflags(to.p_path, fs->st_flags))) {
 			/*
 			 * NFS doesn't support chflags; ignore errors unless
 			 * there's reason to believe we're losing bits.  (Note,
@@ -378,10 +388,11 @@ setfile(struct stat *fs, int fd)
 			 * that we copied, i.e., that we didn't create.)
 			 */
 			if (errno != EOPNOTSUPP || fs->st_flags != 0) {
-				warn("chflags: %s", to.p_path);
+				warn("chflags: %s/%s", to.base, to.path);
 				rval = 1;
 			}
 		}
+	}
 
 	return (rval);
 }
@@ -398,8 +409,9 @@ preserve_fd_acls(int source_fd, int dest_fd)
 		acl_supported = 1;
 		acl_type = ACL_TYPE_NFS4;
 	} else if (ret < 0 && errno != EINVAL) {
-		warn("fpathconf(..., _PC_ACL_NFS4) failed for %s", to.p_path);
+		warn("fpathconf(..., _PC_ACL_NFS4) failed for %s/%s",
-		return (1);
+		    to.base, to.path);
+		return (-1);
 	}
 	if (acl_supported == 0) {
 		ret = fpathconf(source_fd, _PC_ACL_EXTENDED);
@@ -407,9 +419,9 @@ preserve_fd_acls(int source_fd, int dest_fd)
 			acl_supported = 1;
 			acl_type = ACL_TYPE_ACCESS;
 		} else if (ret < 0 && errno != EINVAL) {
-			warn("fpathconf(..., _PC_ACL_EXTENDED) failed for %s",
+			warn("fpathconf(..., _PC_ACL_EXTENDED) failed for %s/%s",
-			    to.p_path);
+			    to.base, to.path);
-			return (1);
+			return (-1);
 		}
 	}
 	if (acl_supported == 0)
@@ -417,112 +429,54 @@ preserve_fd_acls(int source_fd, int dest_fd)
 
 	acl = acl_get_fd_np(source_fd, acl_type);
 	if (acl == NULL) {
-		warn("failed to get acl entries while setting %s", to.p_path);
+		warn("failed to get acl entries while setting %s/%s",
-		return (1);
+		    to.base, to.path);
+		return (-1);
 	}
 	if (acl_is_trivial_np(acl, &trivial)) {
-		warn("acl_is_trivial() failed for %s", to.p_path);
+		warn("acl_is_trivial() failed for %s/%s",
+		    to.base, to.path);
 		acl_free(acl);
-		return (1);
+		return (-1);
 	}
 	if (trivial) {
 		acl_free(acl);
 		return (0);
 	}
 	if (acl_set_fd_np(dest_fd, acl, acl_type) < 0) {
-		warn("failed to set acl entries for %s", to.p_path);
+		warn("failed to set acl entries for %s/%s",
+		    to.base, to.path);
 		acl_free(acl);
-		return (1);
+		return (-1);
 	}
 	acl_free(acl);
 	return (0);
 }
 
 int
-preserve_dir_acls(struct stat *fs, char *source_dir, char *dest_dir)
+preserve_dir_acls(const char *source_dir, const char *dest_dir)
 {
-	acl_t (*aclgetf)(const char *, acl_type_t);
+	int source_fd = -1, dest_fd = -1, ret;
-	int (*aclsetf)(const char *, acl_type_t, acl_t);
-	struct acl *aclp;
-	acl_t acl;
-	acl_type_t acl_type;
-	int acl_supported = 0, ret, trivial;
 
-	ret = pathconf(source_dir, _PC_ACL_NFS4);
+	if ((source_fd = open(source_dir, O_PATH)) < 0) {
-	if (ret > 0) {
+		warn("%s: failed to copy ACLs", source_dir);
-		acl_supported = 1;
+		return (-1);
-		acl_type = ACL_TYPE_NFS4;
-	} else if (ret < 0 && errno != EINVAL) {
-		warn("fpathconf(..., _PC_ACL_NFS4) failed for %s", source_dir);
-		return (1);
 	}
-	if (acl_supported == 0) {
+	dest_fd = (*dest_dir == '\0') ? to.dir :
-		ret = pathconf(source_dir, _PC_ACL_EXTENDED);
+	    openat(to.dir, dest_dir, O_DIRECTORY, AT_RESOLVE_BENEATH);
-		if (ret > 0) {
+	if (dest_fd < 0) {
-			acl_supported = 1;
+		warn("%s: failed to copy ACLs to %s/%s", source_dir,
-			acl_type = ACL_TYPE_ACCESS;
+		    to.base, dest_dir);
-		} else if (ret < 0 && errno != EINVAL) {
+		close(source_fd);
-			warn("fpathconf(..., _PC_ACL_EXTENDED) failed for %s",
+		return (-1);
-			    source_dir);
-			return (1);
-		}
 	}
-	if (acl_supported == 0)
+	if ((ret = preserve_fd_acls(source_fd, dest_fd)) != 0) {
-		return (0);
+		/* preserve_fd_acls() already printed a message */
-
-	/*
-	 * If the file is a link we will not follow it.
-	 */
-	if (S_ISLNK(fs->st_mode)) {
-		aclgetf = acl_get_link_np;
-		aclsetf = acl_set_link_np;
-	} else {
-		aclgetf = acl_get_file;
-		aclsetf = acl_set_file;
 	}
-	if (acl_type == ACL_TYPE_ACCESS) {
+	if (dest_fd != to.dir)
-		/*
+		close(dest_fd);
-		 * Even if there is no ACL_TYPE_DEFAULT entry here, a zero
+	close(source_fd);
-		 * size ACL will be returned. So it is not safe to simply
+	return (ret);
-		 * check the pointer to see if the default ACL is present.
-		 */
-		acl = aclgetf(source_dir, ACL_TYPE_DEFAULT);
-		if (acl == NULL) {
-			warn("failed to get default acl entries on %s",
-			    source_dir);
-			return (1);
-		}
-		aclp = &acl->ats_acl;
-		if (aclp->acl_cnt != 0 && aclsetf(dest_dir,
-		    ACL_TYPE_DEFAULT, acl) < 0) {
-			warn("failed to set default acl entries on %s",
-			    dest_dir);
-			acl_free(acl);
-			return (1);
-		}
-		acl_free(acl);
-	}
-	acl = aclgetf(source_dir, acl_type);
-	if (acl == NULL) {
-		warn("failed to get acl entries on %s", source_dir);
-		return (1);
-	}
-	if (acl_is_trivial_np(acl, &trivial)) {
-		warn("acl_is_trivial() failed on %s", source_dir);
-		acl_free(acl);
-		return (1);
-	}
-	if (trivial) {
-		acl_free(acl);
-		return (0);
-	}
-	if (aclsetf(dest_dir, acl_type, acl) < 0) {
-		warn("failed to set acl entries on %s", dest_dir);
-		acl_free(acl);
-		return (1);
-	}
-	acl_free(acl);
-	return (0);
 }
 
 void