The openat() then fstat() pattern only works if one is sure the file being

opened is a regular file. In other cases this may block in openat() (since
the O_NONBLOCK flag removed). Switch to fstatat() and then openat() to
protect from involuntary side-effects (like opening a device node) and
possible hangs.
OK benno@

diff --git a/usr.bin/rsync/uploader.c b/usr.bin/rsync/uploader.c
index 03db716..707e85f 100644 (file)
--- a/usr.bin/rsync/uploader.c
+++ b/usr.bin/rsync/uploader.c
@@ -1,4 +1,4 @@
-/*     $Id: uploader.c,v 1.25 2021/05/06 07:35:22 claudio Exp $ */
+/*     $Id: uploader.c,v 1.26 2021/05/17 11:49:01 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  * Copyright (c) 2019 Florian Obser <florian@openbsd.org>
@@ -79,7 +79,7 @@ log_dir(struct sess *sess, const struct flist *f)
  * 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 *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 *sess)
                free(temp);
        }
 
-       log_link(sess, f);
+       log_symlink(sess, f);
        return 0;
 }
 
@@ -645,6 +645,7 @@ pre_file(const struct upload *p, int *filefd, struct stat *st,
     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 *filefd, struct stat *st,
        /*
         * 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 *fileinfd,
        off_t               offs;
        int                 c;
 
-       /* This should never get called. */
-
+       /* Once finished this should never get called again. */
        assert(u->state != UPLOAD_FINISHED);
 
        /*