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); /*