So yesterday I committed a change to simplify file handling. This removed
the O_NONBLOCK flag from openat() but today I realized that this was a bit
premature. The code at that point does not know if the file is actually a
regular file and so if you put a fifo in place of a regular file it will
hang rsync.
Anyway doing a blind open of any file here is far from ideal. There may be
involuntary side-effects especially on device nodes. So better change the
code to do the fstatat() first and only open the file if it is indeed a
regular file.
While there change log_link to log_symlink which is more precise and also
fix a comment that missed some context.
OK?
--
:wq Claudio
Index: uploader.c
===================================================================
RCS file: /cvs/src/usr.bin/rsync/uploader.c,v
retrieving revision 1.25
diff -u -p -r1.25 uploader.c
--- uploader.c 6 May 2021 07:35:22 -0000 1.25
+++ uploader.c 7 May 2021 15:03:03 -0000
@@ -79,7 +79,7 @@ log_dir(struct sess *sess, const struct
* operator that we're a link.
*/
static void
-log_link(struct sess *sess, const struct flist *f)
+log_symlink(struct sess *sess, const struct flist *f)
{
if (!sess->opts->server)
@@ -181,7 +181,7 @@ pre_link(struct upload *p, struct sess *
return 0;
}
if (sess->opts->dry_run) {
- log_link(sess, f);
+ log_symlink(sess, f);
return 0;
}
@@ -259,7 +259,7 @@ pre_link(struct upload *p, struct sess *
free(temp);
}
- log_link(sess, f);
+ log_symlink(sess, f);
return 0;
}
@@ -645,6 +645,7 @@ pre_file(const struct upload *p, int *fi
struct sess *sess)
{
const struct flist *f;
+ int rc;
f = &p->fl[p->idx];
assert(S_ISREG(f->st.mode));
@@ -661,51 +662,43 @@ pre_file(const struct upload *p, int *fi
/*
* For non dry-run cases, we'll write the acknowledgement later
* in the rsync_uploader() function.
- * If the call to openat() fails with ENOENT, there's a
- * fast-path between here and the write function.
*/
- *filefd = openat(p->rootfd, f->path,
- O_RDONLY | O_NOFOLLOW, 0);
+ *filefd = -1;
+ rc = fstatat(p->rootfd, f->path, st, AT_SYMLINK_NOFOLLOW);
- if (*filefd == -1 && errno != ENOENT) {
- ERR("%s: openat", f->path);
+ if (rc == -1) {
+ if (errno == ENOENT)
+ return 1;
+
+ ERR("%s: fstatat", f->path);
return -1;
}
- if (*filefd == -1)
+ if (rc != -1 && !S_ISREG(st->st_mode)) {
+ if (S_ISDIR(st->st_mode) &&
+ unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
+ ERR("%s: unlinkat", f->path);
+ return -1;
+ }
return 1;
-
- /*
- * Check if the file is already up to date, in which case close it
- * and go to the next one.
- */
-
- if (fstat(*filefd, st) == -1) {
- ERR("%s: fstat", f->path);
- close(*filefd);
- *filefd = -1;
- return -1;
- } else if (!S_ISREG(st->st_mode)) {
- ERRX("%s: not regular", f->path);
- close(*filefd);
- *filefd = -1;
- return -1;
}
if (st->st_size == f->st.size &&
st->st_mtime == f->st.mtime) {
LOG3("%s: skipping: up to date", f->path);
- if (!rsync_set_metadata(sess, 0, *filefd, f, f->path)) {
+ if (!rsync_set_metadata_at(sess, 0, p->rootfd, f, f->path)) {
ERRX1("rsync_set_metadata");
- close(*filefd);
- *filefd = -1;
return -1;
}
- close(*filefd);
- *filefd = -1;
return 0;
}
+ *filefd = openat(p->rootfd, f->path, O_RDONLY | O_NOFOLLOW, 0);
+ if (*filefd == -1 && errno != ENOENT) {
+ ERR("%s: openat", f->path);
+ return -1;
+ }
+
/* file needs attention */
return 1;
}
@@ -785,8 +778,7 @@ rsync_uploader(struct upload *u, int *fi
off_t offs;
int c;
- /* This should never get called. */
-
+ /* Once finished this should never get called again. */
assert(u->state != UPLOAD_FINISHED);
/*
