git: eb439266b433 - main - cp: Don't rely on FTS_DP to keep track of depth.

From: Dag-Erling Smørgrav <des_at_FreeBSD.org>
Date: Wed, 02 Jul 2025 10:23:01 UTC
The branch main has been updated by des:

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

commit eb439266b433241cec9cbef44328b16056bd6ff7
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2025-07-02 10:22:05 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2025-07-02 10:22:28 +0000

    cp: Don't rely on FTS_DP to keep track of depth.
    
    In normal operation, we get an FTS_D entry when we enter a directory
    and a matching FTS_DP entry when we leave it.  However, if an error
    occurs either changing to or reading a directory, we may get an FTS_D
    entry followed by FTS_DNR or even FTS_ERR instead.  Since FTS_ERR can
    also occur for non-directory entries, the only reliable way to keep
    track of when we leave a directory is to compare fts_level to our own
    depth counter.
    
    This fixes a rare assertion when attempting to recursively copy a
    directory tree containing a directory which is either not readable or
    not searchable.
    
    While here, also add a test case for directory loops.
    
    Fixes:          82fc0d09e8625
    Sponsored by:   Klara, Inc.
    Reviewed by:    kevans
    Differential Revision:  https://reviews.freebsd.org/D51096
---
 bin/cp/cp.c             | 54 ++++++++++++++++++++----------------
 bin/cp/tests/cp_test.sh | 73 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+), 23 deletions(-)

diff --git a/bin/cp/cp.c b/bin/cp/cp.c
index 94a22c1cccc5..7e97715c3ef4 100644
--- a/bin/cp/cp.c
+++ b/bin/cp/cp.c
@@ -270,10 +270,9 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 	FTS *ftsp;
 	FTSENT *curr;
 	char *recpath = NULL, *sep;
-	int atflags, dne, badcp, len, rval;
+	int atflags, dne, badcp, len, level, rval;
 	mode_t mask, mode;
 	bool beneath = Rflag && type != FILE_TO_FILE;
-	bool skipdp = false;
 
 	/*
 	 * Keep an inverted copy of the umask, for use in correcting
@@ -305,6 +304,7 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 		to.dir = -1;
 	}
 
+	level = FTS_ROOTLEVEL;
 	if ((ftsp = fts_open(argv, fts_options, NULL)) == NULL)
 		err(1, "fts_open");
 	for (badcp = rval = 0;
@@ -315,6 +315,20 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 		case FTS_NS:
 		case FTS_DNR:
 		case FTS_ERR:
+			if (level > curr->fts_level) {
+				/* 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 {
+					while (to.end > to.path && *to.end != '/')
+						to.end--;
+					assert(strcmp(to.end + (*to.end == '/'),
+					    curr->fts_name) == 0);
+					*to.end = '\0';
+				}
+				level--;
+			}
 			warnc(curr->fts_errno, "%s", curr->fts_path);
 			badcp = rval = 1;
 			continue;
@@ -335,14 +349,6 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 				strlcpy(rootname, curr->fts_name,
 				    sizeof(rootname));
 			}
-			/*
-			 * 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.
-			 */
-			skipdp = true;
 			/* we must have a destination! */
 			if (type == DIR_TO_DNE &&
 			    curr->fts_level == FTS_ROOTLEVEL) {
@@ -410,7 +416,7 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 				}
 				to.end += len;
 			}
-			skipdp = false;
+			level++;
 			/*
 			 * We're on the verge of recursing on ourselves.
 			 * Either we need to stop right here (we knowingly
@@ -477,18 +483,19 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 					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 {
-				while (to.end > to.path && *to.end != '/')
-					to.end--;
-				assert(strcmp(to.end + (*to.end == '/'), curr->fts_name) == 0);
-				*to.end = '\0';
+			if (level > curr->fts_level) {
+				/* 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 {
+					while (to.end > to.path && *to.end != '/')
+						to.end--;
+					assert(strcmp(to.end + (*to.end == '/'),
+					    curr->fts_name) == 0);
+					*to.end = '\0';
+				}
+				level--;
 			}
 			continue;
 		default:
@@ -638,6 +645,7 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 		if (vflag && !badcp)
 			(void)printf("%s -> %s%s\n", curr->fts_path, to.base, to.path);
 	}
+	assert(level == FTS_ROOTLEVEL);
 	if (errno)
 		err(1, "fts_read");
 	(void)fts_close(ftsp);
diff --git a/bin/cp/tests/cp_test.sh b/bin/cp/tests/cp_test.sh
index d5268ed4c4c9..64f917bf9c5f 100755
--- a/bin/cp/tests/cp_test.sh
+++ b/bin/cp/tests/cp_test.sh
@@ -618,6 +618,76 @@ to_root_cleanup()
 	(dst=$(cat dst) && rm "/$dst") 2>/dev/null || true
 }
 
+atf_test_case dirloop
+dirloop_head()
+{
+	atf_set "descr" "Test cycle detection when recursing"
+}
+dirloop_body()
+{
+	mkdir -p src/a src/b
+	ln -s ../b src/a
+	ln -s ../a src/b
+	atf_check \
+	    -s exit:1 \
+	    -e match:"src/a/b/a: directory causes a cycle" \
+	    -e match:"src/b/a/b: directory causes a cycle" \
+	    cp -r src dst
+	atf_check test -d dst
+	atf_check test -d dst/a
+	atf_check test -d dst/b
+	atf_check test -d dst/a/b
+	atf_check test ! -e dst/a/b/a
+	atf_check test -d dst/b/a
+	atf_check test ! -e dst/b/a/b
+}
+
+atf_test_case unrdir
+unrdir_head()
+{
+	atf_set "descr" "Test handling of unreadable directories"
+}
+unrdir_body()
+{
+	for d in a b c ; do
+		mkdir -p src/$d
+		echo "$d" >src/$d/f
+	done
+	chmod 0 src/b
+	atf_check \
+	    -s exit:1 \
+	    -e match:"^cp: src/b: Permission denied" \
+	    cp -R src dst
+	atf_check test -d dst/a
+	atf_check cmp src/a/f dst/a/f
+	atf_check test -d dst/b
+	atf_check test ! -e dst/b/f
+	atf_check test -d dst/c
+	atf_check cmp src/c/f dst/c/f
+}
+
+atf_test_case unrfile
+unrdir_head()
+{
+	atf_set "descr" "Test handling of unreadable files"
+}
+unrfile_body()
+{
+	mkdir src
+	for d in a b c ; do
+		echo "$d" >src/$d
+	done
+	chmod 0 src/b
+	atf_check \
+	    -s exit:1 \
+	    -e match:"^cp: src/b: Permission denied" \
+	    cp -R src dst
+	atf_check test -d dst
+	atf_check cmp src/a dst/a
+	atf_check test ! -e dst/b
+	atf_check cmp src/c dst/c
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case basic
@@ -656,4 +726,7 @@ atf_init_test_cases()
 	atf_add_test_case to_link_outside
 	atf_add_test_case dstmode
 	atf_add_test_case to_root
+	atf_add_test_case dirloop
+	atf_add_test_case unrdir
+	atf_add_test_case unrfile
 }