Merge pull request #21401 from poettering/open-mkdir-at

add open_mkdir_at() helper and use it

src/basic/fs-util.c

@@ -1011,3 +1011,77 @@ int parse_cifs_service(
 
         return 0;
 }
+
+int open_mkdir_at(int dirfd, const char *path, int flags, mode_t mode) {
+        _cleanup_close_ int fd = -1, parent_fd = -1;
+        _cleanup_free_ char *fname = NULL;
+        bool made;
+        int r;
+
+        /* Creates a directory with mkdirat() and then opens it, in the "most atomic" fashion we can
+         * do. Guarantees that the returned fd refers to a directory. If O_EXCL is specified will fail if the
+         * dir already exists. Otherwise will open an existing dir, but only if it is one.  */
+
+        if (flags & ~(O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_EXCL|O_NOATIME|O_NOFOLLOW|O_PATH))
+                return -EINVAL;
+        if ((flags & O_ACCMODE) != O_RDONLY)
+                return -EINVAL;
+
+        /* Note that O_DIRECTORY|O_NOFOLLOW is implied, but we allow specifying it anyway. The following
+         * flags actually make sense to specify: O_CLOEXEC, O_EXCL, O_NOATIME, O_PATH */
+
+        if (isempty(path))
+                return -EINVAL;
+
+        if (!filename_is_valid(path)) {
+                _cleanup_free_ char *parent = NULL;
+
+                /* If this is not a valid filename, it's a path. Let's open the parent directory then, so
+                 * that we can pin it, and operate below it. */
+
+                r = path_extract_directory(path, &parent);
+                if (r < 0)
+                        return r;
+
+                r = path_extract_filename(path, &fname);
+                if (r < 0)
+                        return r;
+
+                parent_fd = openat(dirfd, parent, O_PATH|O_DIRECTORY|O_CLOEXEC);
+                if (parent_fd < 0)
+                        return -errno;
+
+                dirfd = parent_fd;
+                path = fname;
+        }
+
+        r = RET_NERRNO(mkdirat(dirfd, path, mode));
+        if (r == -EEXIST) {
+                if (FLAGS_SET(flags, O_EXCL))
+                        return -EEXIST;
+
+                made = false;
+        } else if (r < 0)
+                return r;
+        else
+                made = true;
+
+        fd = RET_NERRNO(openat(dirfd, path, (flags & ~O_EXCL)|O_DIRECTORY|O_NOFOLLOW));
+        if (fd < 0) {
+                if (fd == -ENOENT)  /* We got ENOENT? then someone else immediately removed it after we
+                                     * created it. In that case let's return immediately without unlinking
+                                     * anything, because there simply isn't anything to unlink anymore. */
+                        return -ENOENT;
+                if (fd == -ELOOP)   /* is a symlink? exists already → created by someone else, don't unlink */
+                        return -EEXIST;
+                if (fd == -ENOTDIR) /* not a directory? exists already → created by someone else, don't unlink */
+                        return -EEXIST;
+
+                if (made)
+                        (void) unlinkat(dirfd, path, AT_REMOVEDIR);
+
+                return fd;
+        }
+
+        return TAKE_FD(fd);
+}

src/basic/fs-util.h

@@ -108,3 +108,5 @@ static inline int conservative_rename(const char *oldpath, const char *newpath)
 int posix_fallocate_loop(int fd, uint64_t offset, uint64_t size);
 
 int parse_cifs_service(const char *s, char **ret_host, char **ret_service, char **ret_path);
+
+int open_mkdir_at(int dirfd, const char *path, int flags, mode_t mode);

src/core/namespace.c

@@ -2499,6 +2499,7 @@ int temporary_filesystem_add(
 
 static int make_tmp_prefix(const char *prefix) {
         _cleanup_free_ char *t = NULL;
+        _cleanup_close_ int fd = -1;
         int r;
 
         /* Don't do anything unless we know the dir is actually missing */
@@ -2517,18 +2518,20 @@ static int make_tmp_prefix(const char *prefix) {
         if (r < 0)
                 return r;
 
-        if (mkdir(t, 0777) < 0) /* umask will corrupt this access mode, but that doesn't matter, we need to
-                                 * call chmod() anyway for the suid bit, below. */
-                return -errno;
+        /* umask will corrupt this access mode, but that doesn't matter, we need to call chmod() anyway for
+         * the suid bit, below. */
+        fd = open_mkdir_at(AT_FDCWD, t, O_EXCL|O_CLOEXEC, 0777);
+        if (fd < 0)
+                return fd;
 
-        if (chmod(t, 01777) < 0) {
-                r = -errno;
+        r = RET_NERRNO(fchmod(fd, 01777));
+        if (r < 0) {
                 (void) rmdir(t);
                 return r;
         }
 
-        if (rename(t, prefix) < 0) {
-                r = -errno;
+        r = RET_NERRNO(rename(t, prefix));
+        if (r < 0) {
                 (void) rmdir(t);
                 return r == -EEXIST ? 0 : r; /* it's fine if someone else created the dir by now */
         }

src/home/homework-cifs.c

@@ -127,15 +127,17 @@ int home_setup_cifs(
                         return log_oom();
 
                 if (FLAGS_SET(flags, HOME_SETUP_CIFS_MKDIR)) {
-                        r = mkdir_p(j, 0700);
-                        if (r < 0)
-                                return log_error_errno(r, "Failed to create CIFS subdirectory: %m");
+                        setup->root_fd = open_mkdir_at(AT_FDCWD, j, O_CLOEXEC, 0700);
+                        if (setup->root_fd < 0)
+                                return log_error_errno(setup->root_fd, "Failed to create CIFS subdirectory: %m");
                 }
         }
 
-        setup->root_fd = open(j ?: HOME_RUNTIME_WORK_DIR, O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW);
-        if (setup->root_fd < 0)
-                return log_error_errno(errno, "Failed to open home directory: %m");
+        if (setup->root_fd < 0) {
+                setup->root_fd = open(j ?: HOME_RUNTIME_WORK_DIR, O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW);
+                if (setup->root_fd < 0)
+                        return log_error_errno(errno, "Failed to open home directory: %m");
+        }
 
         setup->mount_suffix = TAKE_PTR(cdir);
         return 0;

src/shared/copy.c

@@ -485,8 +485,6 @@ static int hardlink_context_setup(
 }
 
 static int hardlink_context_realize(HardlinkContext *c) {
-        int r;
-
         if (!c)
                 return 0;
 
@@ -498,15 +496,9 @@ static int hardlink_context_realize(HardlinkContext *c) {
 
         assert(c->subdir);
 
-        if (mkdirat(c->parent_fd, c->subdir, 0700) < 0)
-                return -errno;
-
-        c->dir_fd = openat(c->parent_fd, c->subdir, O_RDONLY|O_DIRECTORY|O_CLOEXEC);
-        if (c->dir_fd < 0) {
-                r = -errno;
-                (void) unlinkat(c->parent_fd, c->subdir, AT_REMOVEDIR);
-                return r;
-        }
+        c->dir_fd = open_mkdir_at(c->parent_fd, c->subdir, O_EXCL|O_CLOEXEC, 0700);
+        if (c->dir_fd < 0)
+                return c->dir_fd;
 
         return 1;
 }

src/shared/creds-util.c

@@ -215,10 +215,10 @@ int get_credential_host_secret(CredentialSecretFlags flags, void **ret, size_t *
                 fn = "credential.secret";
         }
 
-        (void) mkdir_p(p, 0755);
-        dfd = open(p, O_CLOEXEC|O_DIRECTORY|O_RDONLY);
+        mkdir_parents(p, 0755);
+        dfd = open_mkdir_at(AT_FDCWD, p, O_CLOEXEC, 0755);
         if (dfd < 0)
-                return -errno;
+                return dfd;
 
         if (FLAGS_SET(flags, CREDENTIAL_SECRET_FAIL_ON_TEMPORARY_FS)) {
                 r = fd_is_temporary_fs(dfd);

src/test/test-fs-util.c

@@ -952,6 +952,49 @@ static void test_parse_cifs_service(void) {
         test_parse_cifs_service_one("//./a", NULL, NULL, NULL, -EINVAL);
 }
 
+static void test_open_mkdir_at(void) {
+        _cleanup_close_ int fd = -1, subdir_fd = -1, subsubdir_fd = -1;
+        _cleanup_(rm_rf_physical_and_freep) char *t = NULL;
+        log_info("/* %s */", __func__);
+
+        assert_se(open_mkdir_at(AT_FDCWD, "/proc", O_EXCL|O_CLOEXEC, 0) == -EEXIST);
+
+        fd = open_mkdir_at(AT_FDCWD, "/proc", O_CLOEXEC, 0);
+        assert_se(fd >= 0);
+        fd = safe_close(fd);
+
+        assert_se(open_mkdir_at(AT_FDCWD, "/bin/sh", O_EXCL|O_CLOEXEC, 0) == -EEXIST);
+        assert_se(open_mkdir_at(AT_FDCWD, "/bin/sh", O_CLOEXEC, 0) == -EEXIST);
+
+        assert_se(mkdtemp_malloc(NULL, &t) >= 0);
+
+        assert_se(open_mkdir_at(AT_FDCWD, t, O_EXCL|O_CLOEXEC, 0) == -EEXIST);
+        assert_se(open_mkdir_at(AT_FDCWD, t, O_PATH|O_EXCL|O_CLOEXEC, 0) == -EEXIST);
+
+        fd = open_mkdir_at(AT_FDCWD, t, O_CLOEXEC, 0000);
+        assert_se(fd >= 0);
+        fd = safe_close(fd);
+
+        fd = open_mkdir_at(AT_FDCWD, t, O_PATH|O_CLOEXEC, 0000);
+        assert_se(fd >= 0);
+
+        subdir_fd = open_mkdir_at(fd, "xxx", O_PATH|O_EXCL|O_CLOEXEC, 0700);
+        assert_se(subdir_fd >= 0);
+
+        assert_se(open_mkdir_at(fd, "xxx", O_PATH|O_EXCL|O_CLOEXEC, 0) == -EEXIST);
+
+        subsubdir_fd = open_mkdir_at(subdir_fd, "yyy", O_EXCL|O_CLOEXEC, 0700);
+        assert_se(subsubdir_fd >= 0);
+        subsubdir_fd = safe_close(subsubdir_fd);
+
+        assert_se(open_mkdir_at(subdir_fd, "yyy", O_EXCL|O_CLOEXEC, 0) == -EEXIST);
+
+        assert_se(open_mkdir_at(fd, "xxx/yyy", O_EXCL|O_CLOEXEC, 0) == -EEXIST);
+
+        subsubdir_fd = open_mkdir_at(fd, "xxx/yyy", O_CLOEXEC, 0700);
+        assert_se(subsubdir_fd >= 0);
+}
+
 int main(int argc, char *argv[]) {
         test_setup_logging(LOG_INFO);
 
@@ -972,6 +1015,7 @@ int main(int argc, char *argv[]) {
         test_conservative_rename();
         test_rmdir_parents();
         test_parse_cifs_service();
+        test_open_mkdir_at();
 
         return 0;
 }