Hi,
I've noticed an issue with openrsync and more than 1 source arguments and with
it's symlink behaviour (separate issues).
Reproduce:
cd /tmp
mkdir -p a b
openrsync -av a/ b/ c/
Output:
/usr/src/usr.bin/rsync/flist.c:823: error: b/: lstat: No such file or
directory
/usr/src/usr.bin/rsync/flist.c:1032: error: flist_gen_dirent
/usr/src/usr.bin/rsync/sender.c:391: error: flist_gen
/usr/src/usr.bin/rsync/client.c:85: error: rsync_sender
This is because in flist.c unveil(2) is called per argument. unveil(2) will
remove visibility after it's first call as described in the man page:
"
The first call to unveil removes visibility of the entire filesystem from
all other filesystem-related system calls (such as open(2), chmod(2) and
rename(2)), except for the specified path and permissions.
"
For both files and recursive directories a list is created. I think the logic
can be simplified and unified and unveil(2) called on these list items after
the list is created.
After changing the unveil(2) logic, there is an error in the duplicate working
path detection code:
/usr/src/usr.bin/rsync/flist.c:119: error: .: duplicate working path
for possibly different file: a/., b/.
/usr/src/usr.bin/rsync/flist.c:1149: error: flist_dedupe
/usr/src/usr.bin/rsync/sender.c:391: error: flist_gen
/usr/src/usr.bin/rsync/client.c:85: error: rsync_sender
In the below patch I have removed this code for now.
There is also an issue with symlinks:
mkdir -p /tmp/a
cd /tmp/a
ln -s b a
cd /tmp
openrsync -av a/ b/
Result:
/usr/src/usr.bin/rsync/symlinks.c:48: error: a/a: readlink: No such
file or directory
/usr/src/usr.bin/rsync/flist.c:985: error: symlink_read
/usr/src/usr.bin/rsync/flist.c:1032: error: flist_gen_dirent
/usr/src/usr.bin/rsync/sender.c:391: error: flist_gen
/usr/src/usr.bin/rsync/client.c:85: error: rsync_sender
I think this is because the fts(3) interface by default chdir(2) to a directory
before running the callback function and the path is then incorrect there. I
think it can safely pass FTS_NOCHDIR as a flag (and also optimization) to
fts_open(3).
Work-in-progress patch below, feedback is welcome:
diff --git usr.bin/rsync/flist.c usr.bin/rsync/flist.c
index 4860e20f428..07285b87362 100644
--- usr.bin/rsync/flist.c
+++ usr.bin/rsync/flist.c
@@ -113,12 +113,6 @@ flist_dedupe(struct flist **fl, size_t *sz)
fnext->path = fnext->link = NULL;
continue;
}
-
- ERRX("%s: duplicate working path for "
- "possibly different file: %s, %s",
- f->wpath, f->path, fnext->path);
- free(new);
- return 0;
}
/* Don't forget the last entry. */
@@ -834,10 +828,6 @@ flist_gen_dirent(struct sess *sess, char *root, struct
flist **fl, size_t *sz,
ERRX1("flist_append");
return 0;
}
- if (unveil(root, "r") == -1) {
- ERR("%s: unveil", root);
- return 0;
- }
return 1;
} else if (S_ISLNK(st.st_mode)) {
if (!sess->opts->preserve_links) {
@@ -854,10 +844,6 @@ flist_gen_dirent(struct sess *sess, char *root, struct
flist **fl, size_t *sz,
ERRX1("flist_append");
return 0;
}
- if (unveil(root, "r") == -1) {
- ERR("%s: unveil", root);
- return 0;
- }
return 1;
} else if (!S_ISDIR(st.st_mode)) {
WARNX("%s: skipping special", root);
@@ -892,7 +878,7 @@ flist_gen_dirent(struct sess *sess, char *root, struct
flist **fl, size_t *sz,
* We'll make sense of it in flist_send.
*/
- if ((fts = fts_open(cargv, FTS_PHYSICAL, NULL)) == NULL) {
+ if ((fts = fts_open(cargv, FTS_PHYSICAL|FTS_NOCHDIR, NULL)) == NULL) {
ERR("fts_open");
return 0;
}
@@ -994,10 +980,6 @@ flist_gen_dirent(struct sess *sess, char *root, struct
flist **fl, size_t *sz,
ERR("fts_read");
goto out;
}
- if (unveil(root, "r") == -1) {
- ERR("%s: unveil", root);
- goto out;
- }
LOG3("generated %zu filenames: %s", flsz, root);
rc = 1;
@@ -1091,10 +1073,6 @@ flist_gen_files(struct sess *sess, size_t argc, char
**argv,
/* Add this file to our file-system worldview. */
- if (unveil(argv[i], "r") == -1) {
- ERR("%s: unveil", argv[i]);
- goto out;
- }
if (!flist_append(f, &st, argv[i])) {
ERRX1("flist_append");
goto out;
@@ -1124,6 +1102,7 @@ int
flist_gen(struct sess *sess, size_t argc, char **argv, struct flist **flp,
size_t *sz)
{
+ size_t i;
int rc;
assert(argc > 0);
@@ -1133,10 +1112,23 @@ flist_gen(struct sess *sess, size_t argc, char **argv,
struct flist **flp,
/* After scanning, lock our file-system view. */
+ for (i = 0; i < argc; i++) {
+ if (unveil(argv[i], "r") == -1) {
+ ERR("%s: unveil", argv[i]);
+ return 0;
+ }
+ }
+ for (i = 0; i < *sz; i++) {
+ if (unveil((*flp)[i].path, "r") == -1) {
+ ERR("%s: unveil", (*flp)[i].path);
+ return 0;
+ }
+ }
if (unveil(NULL, NULL) == -1) {
ERR("unveil");
return 0;
}
+
if (!rc)
return 0;
@@ -1276,7 +1268,7 @@ flist_gen_dels(struct sess *sess, const char *root,
struct flist **fl,
* If the directories don't exist, it's ok.
*/
- if ((fts = fts_open(cargv, FTS_PHYSICAL, NULL)) == NULL) {
+ if ((fts = fts_open(cargv, FTS_PHYSICAL|FTS_NOCHDIR, NULL)) == NULL) {
ERR("fts_open");
goto out;
}
--
Kind regards,
Hiltjo
