The branch main has been updated by des:
URL:
https://cgit.FreeBSD.org/src/commit/?id=eb439266b433241cec9cbef44328b16056bd6ff7
commit eb439266b433241cec9cbef44328b16056bd6ff7
Author: Dag-Erling Smørgrav <d...@freebsd.org>
AuthorDate: 2025-07-02 10:22:05 +0000
Commit: Dag-Erling Smørgrav <d...@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
}
