Merge pull request #10251 from poettering/renameat-racy

rename_noreplace() fallback for file systems where neither RENAME_REPLACE nor link()/unlinkat() is available

src/basic/fileio.c

@@ -1492,6 +1492,7 @@ int open_serialization_fd(const char *ident) {
 }
 
 int link_tmpfile(int fd, const char *path, const char *target) {
+        int r;
 
         assert(fd >= 0);
         assert(target);
@@ -1504,8 +1505,9 @@ int link_tmpfile(int fd, const char *path, const char *target) {
          * operation currently (renameat2() does), and there is no nice way to emulate this. */
 
         if (path) {
-                if (rename_noreplace(AT_FDCWD, path, AT_FDCWD, target) < 0)
-                        return -errno;
+                r = rename_noreplace(AT_FDCWD, path, AT_FDCWD, target);
+                if (r < 0)
+                        return r;
         } else {
                 char proc_fd_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(fd) + 1];
 

src/basic/fs-util.c

@@ -89,41 +89,44 @@ int rmdir_parents(const char *path, const char *stop) {
 }
 
 int rename_noreplace(int olddirfd, const char *oldpath, int newdirfd, const char *newpath) {
-        struct stat buf;
-        int ret;
+        int r;
 
-        ret = renameat2(olddirfd, oldpath, newdirfd, newpath, RENAME_NOREPLACE);
-        if (ret >= 0)
+        /* Try the ideal approach first */
+        if (renameat2(olddirfd, oldpath, newdirfd, newpath, RENAME_NOREPLACE) >= 0)
                 return 0;
 
-        /* renameat2() exists since Linux 3.15, btrfs added support for it later.
-         * If it is not implemented, fallback to another method. */
-        if (!IN_SET(errno, EINVAL, ENOSYS))
+        /* renameat2() exists since Linux 3.15, btrfs and FAT added support for it later. If it is not implemented,
+         * fall back to a different method. */
+        if (!IN_SET(errno, EINVAL, ENOSYS, ENOTTY))
                 return -errno;
 
-        /* The link()/unlink() fallback does not work on directories. But
-         * renameat() without RENAME_NOREPLACE gives the same semantics on
-         * directories, except when newpath is an *empty* directory. This is
-         * good enough. */
-        ret = fstatat(olddirfd, oldpath, &buf, AT_SYMLINK_NOFOLLOW);
-        if (ret >= 0 && S_ISDIR(buf.st_mode)) {
-                ret = renameat(olddirfd, oldpath, newdirfd, newpath);
-                return ret >= 0 ? 0 : -errno;
+        /* Let's try to use linkat()+unlinkat() as fallback. This doesn't work on directories and on some file systems
+         * that do not support hard links (such as FAT, most prominently), but for files it's pretty close to what we
+         * want — though not atomic (i.e. for a short period both the new and the old filename will exist). */
+        if (linkat(olddirfd, oldpath, newdirfd, newpath, 0) >= 0) {
+
+                if (unlinkat(olddirfd, oldpath, 0) < 0) {
+                        r = -errno; /* Backup errno before the following unlinkat() alters it */
+                        (void) unlinkat(newdirfd, newpath, 0);
+                        return r;
+                }
+
+                return 0;
         }
 
-        /* If it is not a directory, use the link()/unlink() fallback. */
-        ret = linkat(olddirfd, oldpath, newdirfd, newpath, 0);
-        if (ret < 0)
+        if (!IN_SET(errno, EINVAL, ENOSYS, ENOTTY, EPERM)) /* FAT returns EPERM on link()… */
                 return -errno;
 
-        ret = unlinkat(olddirfd, oldpath, 0);
-        if (ret < 0) {
-                /* backup errno before the following unlinkat() alters it */
-                ret = errno;
-                (void) unlinkat(newdirfd, newpath, 0);
-                errno = ret;
+        /* OK, neither RENAME_NOREPLACE nor linkat()+unlinkat() worked. Let's then fallback to the racy TOCTOU
+         * vulnerable accessat(F_OK) check followed by classic, replacing renameat(), we have nothing better. */
+
+        if (faccessat(newdirfd, newpath, F_OK, AT_SYMLINK_NOFOLLOW) >= 0)
+                return -EEXIST;
+        if (errno != ENOENT)
+                return -errno;
+
+        if (renameat(olddirfd, oldpath, newdirfd, newpath) < 0)
                 return -errno;
-        }
 
         return 0;
 }

src/test/test-fs-util.c

@@ -614,6 +614,77 @@ static void test_fsync_directory_of_file(void) {
         assert_se(fsync_directory_of_file(fd) >= 0);
 }
 
+static void test_rename_noreplace(void) {
+
+        const char* const table[] = {
+                "/reg",
+                "/dir",
+                "/fifo",
+                "/socket",
+                "/symlink",
+                NULL
+        };
+
+        _cleanup_(rm_rf_physical_and_freep) char *z = NULL;
+        const char *e, *j;
+        char **a, **b;
+
+        e = getenv("RENAME_NOREPLACE_DIR");
+        if (e) {
+                j = strjoina(e, "/testXXXXXX");
+
+                assert_se(mkdtemp_malloc(j, &z) >= 0);
+        } else
+                assert_se(mkdtemp_malloc(NULL, &z) >= 0);
+
+        j = strjoina(z, table[0]);
+        assert_se(touch(j) >= 0);
+
+        j = strjoina(z, table[1]);
+        assert_se(mkdir(j, 0777) >= 0);
+
+        j = strjoina(z, table[2]);
+        (void) mkfifo(j, 0777);
+
+        j = strjoina(z, table[3]);
+        (void) mknod(j, S_IFSOCK | 0777, 0);
+
+        j = strjoina(z, table[4]);
+        (void) symlink("foobar", j);
+
+        STRV_FOREACH(a, (char**) table) {
+                _cleanup_free_ char *x = NULL, *y = NULL;
+
+                x = strjoin(z, *a);
+                assert_se(x);
+
+                if (access(x, F_OK) < 0) {
+                        assert_se(errno == ENOENT);
+                        continue;
+                }
+
+                STRV_FOREACH(b, (char**) table) {
+                        y = strjoin(z, *b);
+                        assert_se(y);
+
+                        if (access(y, F_OK) < 0) {
+                                assert_se(errno == ENOENT);
+                                continue;
+                        }
+
+                        assert_se(rename_noreplace(AT_FDCWD, x, AT_FDCWD, y) == -EEXIST);
+
+                        y = mfree(y);
+                }
+
+                y = strjoin(z, "/somethingelse");
+                assert_se(y);
+
+                assert_se(rename_noreplace(AT_FDCWD, x, AT_FDCWD, y) >= 0);
+                assert_se(rename_noreplace(AT_FDCWD, y, AT_FDCWD, x) >= 0);
+        }
+}
+
 int main(int argc, char *argv[]) {
         test_unlink_noerrno();
         test_get_files_in_directory();
@@ -625,6 +696,7 @@ int main(int argc, char *argv[]) {
         test_touch_file();
         test_unlinkat_deallocate();
         test_fsync_directory_of_file();
+        test_rename_noreplace();
 
         return 0;
 }