File U_13-acct_gather_profile-hdf5-Avoid-TOCTOU-abuse-when-creating-directories.patch of Package slurm.31097
From: Alejandro Sanchez <alex@schedmd.com>
Date: Wed Oct 11 12:45:25 2023 -0600
Subject: [PATCH 13/19]acct_gather_profile/hdf5 - Avoid TOCTOU abuse when creating directories.
Patch-mainline: Upstream
Git-repo: https://github.com/SchedMD/slurm
Git-commit: 5e185536d1d8265f69162d7e374eecb1c398c322
References: bsc#1216207,CVE-2023-41914
Signed-off-by: Egbert Eich <eich@suse.de>
Prevent TOCTOU abuse by using the *at family of syscalls (which work on file
descriptors instead of path names) when creating and changing permissions
for ProfileHDF5Dir and its user-specific sub-directories.
---
.../hdf5/acct_gather_profile_hdf5.c | 82 ++++++++++++++--------
1 file changed, 51 insertions(+), 31 deletions(-)
diff --git a/src/plugins/acct_gather_profile/hdf5/acct_gather_profile_hdf5.c b/src/plugins/acct_gather_profile/hdf5/acct_gather_profile_hdf5.c
index 3a94e789b1..6ad466016b 100644
--- a/src/plugins/acct_gather_profile/hdf5/acct_gather_profile_hdf5.c
+++ b/src/plugins/acct_gather_profile/hdf5/acct_gather_profile_hdf5.c
@@ -156,46 +156,66 @@ static uint32_t _determine_profile(void)
static int _create_directories(void)
{
- int rc;
- struct stat st;
- char *user_dir = NULL;
+ char *parent_dir = NULL, *user_dir = NULL, *hdf5_dir_rel = NULL;
+ char *slash = NULL;
+ int parent_dirfd, user_parent_dirfd;
xassert(g_job);
xassert(hdf5_conf.dir);
+
+ parent_dir = xstrdup(hdf5_conf.dir);
+ /* split into base and new directory name */
+ while ((slash = strrchr(parent_dir, '/'))) {
+ /* fix a path with one or more trailing slashes */
+ if (slash[1] == '\0')
+ slash[0] = '\0';
+ else
+ break;
+ }
+
+ if (!slash)
+ fatal("Invalid ProfileHDF5Dir=\"%s\"", hdf5_conf.dir);
+
+ slash[0] = '\0';
+ hdf5_dir_rel = slash + 1;
+
+ parent_dirfd = open(parent_dir, O_DIRECTORY | O_NOFOLLOW);
+
/*
- * If profile director does not exist, try to create it.
- * Otherwise, ensure path is a directory as expected, and that
- * we have permission to write to it.
- * also make sure the subdirectory tmp exists.
+ * Use *at family of syscalls to prevent TOCTOU abuse by working
+ * on file descriptors instead of path names.
*/
+ if ((mkdirat(parent_dirfd, hdf5_dir_rel, 0755)) < 0) {
+ /* Never chmod on EEXIST */
+ if (errno != EEXIST)
+ fatal("mkdirat(%s): %m", hdf5_conf.dir);
+ } else if (fchmodat(parent_dirfd, hdf5_dir_rel, 0755,
+ AT_SYMLINK_NOFOLLOW) < 0)
+ fatal("fchmodat(%s): %m", hdf5_conf.dir);
+
+ xstrfmtcat(user_dir, "%s/%s", hdf5_conf.dir, g_job->user_name);
+ user_parent_dirfd = openat(parent_dirfd, hdf5_dir_rel,
+ O_DIRECTORY | O_NOFOLLOW);
+ close(parent_dirfd);
+
+ if ((mkdirat(user_parent_dirfd, g_job->user_name, 0700)) < 0) {
+ /* Never chmod on EEXIST */
+ if (errno != EEXIST)
+ fatal("mkdirat(%s): %m", user_dir);
+ } else {
+ /* fchmodat(2) man says AT_SYMLINK_NOFOLLOW not implemented. */
+ if (fchmodat(user_parent_dirfd, g_job->user_name, 0700, 0) < 0)
+ fatal("fchmodat(%s): %m", user_dir);
- if (((rc = stat(hdf5_conf.dir, &st)) < 0) && (errno == ENOENT)) {
- if (mkdir(hdf5_conf.dir, 0755) < 0)
- fatal("mkdir(%s): %m", hdf5_conf.dir);
- } else if (rc < 0)
- fatal("Unable to stat acct_gather_profile_dir: %s: %m",
- hdf5_conf.dir);
- else if (!S_ISDIR(st.st_mode))
- fatal("acct_gather_profile_dir: %s: Not a directory!",
- hdf5_conf.dir);
- else if (access(hdf5_conf.dir, R_OK|W_OK|X_OK) < 0)
- fatal("Incorrect permissions on acct_gather_profile_dir: %s",
- hdf5_conf.dir);
- if (chmod(hdf5_conf.dir, 0755) == -1)
- error("%s: chmod(%s): %m", __func__, hdf5_conf.dir);
-
- user_dir = xstrdup_printf("%s/%s", hdf5_conf.dir, g_job->user_name);
- if (((rc = stat(user_dir, &st)) < 0) && (errno == ENOENT)) {
- if (mkdir(user_dir, 0700) < 0)
- fatal("mkdir(%s): %m", user_dir);
+ if (fchownat(user_parent_dirfd, g_job->user_name, g_job->uid,
+ g_job->gid, AT_SYMLINK_NOFOLLOW) < 0)
+ fatal("fchmodat(%s): %m", user_dir);
}
- if (chmod(user_dir, 0700) == -1)
- error("%s: chmod(%s): %m", __func__, user_dir);
- if (chown(user_dir, (uid_t)g_job->uid,
- (gid_t)g_job->gid) < 0)
- error("chown(%s): %m", user_dir);
+ close(user_parent_dirfd);
xfree(user_dir);
+ xfree(parent_dir);
+ /* Do not xfree() hdf5_dir_rel (interior pointer to freed data). */
return SLURM_SUCCESS;
}