From cc7b0a438199a08276f1381d14d5caf0530084c2 Mon Sep 17 00:00:00 2001 From: Leah Rowe Date: Sun, 22 Mar 2026 20:24:54 +0000 Subject: WIP cleanup: split mkhtemp Signed-off-by: Leah Rowe --- util/nvmutil/include/common.h | 7 + util/nvmutil/lib/file.c | 294 +++++++++++++++--------------------------- util/nvmutil/lib/io.c | 2 +- 3 files changed, 115 insertions(+), 188 deletions(-) (limited to 'util/nvmutil') diff --git a/util/nvmutil/include/common.h b/util/nvmutil/include/common.h index 930150df..b47e5f4b 100644 --- a/util/nvmutil/include/common.h +++ b/util/nvmutil/include/common.h @@ -480,6 +480,13 @@ const char *getnvmprogname(void); char *new_tmpfile(int *fd); char *new_tmplate(int *fd); +static int mkhtemp_try_create(int dirfd, + struct stat *st_dir_initial, + char *fname_copy, + char *p, + size_t xc, + int *fd, + struct stat *st); int mkhtemp(int *fd, struct stat *st, char *template, int dirfd, const char *fname, struct stat *st_dir_initial); diff --git a/util/nvmutil/lib/file.c b/util/nvmutil/lib/file.c index 8ebb93d7..ee849b73 100644 --- a/util/nvmutil/lib/file.c +++ b/util/nvmutil/lib/file.c @@ -700,283 +700,203 @@ sticky_hell: * generates files) */ -int mkhtemp(int *fd, - struct stat *st, +int +mkhtemp(int *fd, + struct stat *st, char *template, int dirfd, const char *fname, struct stat *st_dir_initial) { - /* NOTE: this function currently - * only supports *files*. - * it doesn't make tmp*dirs* - */ size_t len = 0; - char *p = NULL; - char *template_copy = NULL; size_t xc = 0; - size_t r; + size_t fname_len = 0; + + char *template_copy = NULL; + char *fname_copy = NULL; + char *p; + + size_t retries; + + int saved_errno = errno; + #if defined(PATH_LEN) && \ - (PATH_LEN) >= 256 + (PATH_LEN) >= 256 size_t max_len = PATH_LEN; #else size_t max_len = 4096; #endif - int file_created = 0; - int saved_errno = errno; - struct stat st_tmp; - mode_t old_umask; - char *fname_copy = NULL; - size_t fname_len = 0; - /* for ctrl char check - */ - unsigned char ctrl = 0; - size_t ctrl_pos = 0; - struct stat st_dir_now; - size_t retries = 0; -#if !(defined(MAX_MKHTEMP_RETRIES) && \ - (MAX_MKHTEMP_RETRIES) >= 128) - size_t max_retries = 300; -#else - size_t max_retries = 128; -#endif - if (fname == NULL || - st_dir_initial == NULL || - fd == NULL) { + if (fd == NULL || + template == NULL || + fname == NULL || + st_dir_initial == NULL) { errno = EFAULT; - goto err_mkhtemp; + return -1; } - if (dirfd < 0) { - errno = EBADF; - goto err_mkhtemp; - } if (*fd >= 0) { - errno = EEXIST; - goto err_mkhtemp; + return -1; + } + + if (dirfd < 0) { + errno = EBADF; + return -1; } - if (slen(template, max_len, &len) < 0) { - goto err_mkhtemp; - } else if (len >= max_len) { /* bounds check */ + if (slen(template, max_len, &len) < 0) + return -1; + + if (len >= max_len) { errno = EMSGSIZE; - goto err_mkhtemp; + return -1; } - if (strrchr(fname, '/') != NULL) { + if (slen(fname, max_len, &fname_len) < 0) + return -1; - /* otherwise, a mangled - path could leave the - directory, defeating - the purpose of openat - */ + if (fname_len == 0) { errno = EINVAL; - goto err_mkhtemp; + return -1; } - /* reject dangerous basenames - */ - if (slen(fname, max_len, &fname_len) < 0) { - goto err_mkhtemp; - } else if (fname_len == 0) { - errno = EINVAL; - goto err_mkhtemp; - } else if (fname[0] == '\0' || - (fname[0] == '.' && fname[1] == '\0') || - (fname[0] == '.' && fname[1] == '.' && fname[2] == '\0')) { + if (strrchr(fname, '/') != NULL) { errno = EINVAL; - goto err_mkhtemp; + return -1; } - /* block control chars - */ - for (ctrl_pos = 0; - ctrl_pos < fname_len; - ctrl_pos++) { - ctrl = (unsigned char)fname[ctrl_pos]; + /* count trailing X */ + { + char *end = template + len; - if (ctrl < 32 || ctrl == 127) { - errno = EINVAL; - goto err_mkhtemp; - } + while (end > template && *--end == 'X') + xc++; } - p = template + len; - - while (p > template && - *--p == 'X') - ++xc; - if (xc < 6 || xc > len) { - errno = EINVAL; - goto err_mkhtemp; + return -1; } if (fname_len > len || fname_len > (len - xc)) { - - /* prevent overflow - */ errno = EOVERFLOW; - goto err_mkhtemp; + return -1; } if (memcmp(fname, template + len - fname_len, fname_len) != 0) { - errno = EINVAL; - goto err_mkhtemp; + return -1; } template_copy = malloc(len + 1); if (template_copy == NULL) { - errno = ENOMEM; - goto err_mkhtemp; + goto err; } - /* we work on a cached copy first, - * to avoid partial writes of the - * input under fault conditions - */ - memcpy(template_copy, template, len + 1); - - /* redundant copy, reduce chance of - * mangling (regression mitigation) - */ - fname_copy = malloc(fname_len + 1); - if (fname_copy == NULL) { - errno = ENOMEM; - goto err_mkhtemp; + goto err; } + memcpy(template_copy, template, len + 1); + memcpy(fname_copy, template_copy + len - fname_len, fname_len + 1); p = fname_copy + fname_len - xc; - for (retries = 0; retries < max_retries; retries++) { - - if (mkhtemp_fill_random(p, xc) < 0) - goto err_mkhtemp; - - /* - * use the file descriptor instead. - * (danach prüfen wir die Sicherheit des Verzeichnisses) - */ - if (fstat(dirfd, &st_dir_now) < 0) - goto err_mkhtemp; - /* - * mitigate symlink attacks before open - */ - if (st_dir_now.st_dev != st_dir_initial->st_dev || - st_dir_now.st_ino != st_dir_initial->st_ino) { - errno = ESTALE; - goto err_mkhtemp; - } - - *fd = openat2p(dirfd, fname_copy, - O_RDWR | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC | - O_NOCTTY, 0600); - - if (*fd >= 0) { + for (retries = 0; retries < 300; retries++) { - file_created = 1; + int r = mkhtemp_try_create( + dirfd, + st_dir_initial, + fname_copy, + p, + xc, + fd, + st); - memcpy(template_copy + len - fname_len, + if (r == 1) { + /* success: copy final name back */ + memcpy(template + len - fname_len, fname_copy, fname_len); - if (secure_file(fd, st, O_APPEND, 1, 1, 0600) < 0) - goto err_mkhtemp; - - /* copy replaced characters - */ - memcpy( - template + len + xc, - template_copy + len + xc, - xc); - - if (template_copy != NULL) { - - free(template_copy); - template_copy = NULL; - } - errno = saved_errno; - - /* thunder room secure - */ - return *fd; + goto success; } - if (errno != EEXIST && - errno != EINTR && - errno != EAGAIN) - goto err_mkhtemp; + if (r == -1) + goto err; } -err_mkhtemp: - - saved_errno = errno; + errno = EEXIST; +err: if (*fd >= 0) { - - (void) close_on_eintr(*fd); - + (void)close_on_eintr(*fd); *fd = -1; - /* ^^^^^ the caller gives us a dir, - for use and we write in it - but *we* create their file - - touch their *fd, not dirfd - as we only initialised *fd - */ } - if (template_copy != NULL) { +success: + if (template_copy != NULL) + free(template_copy); - /* we created it, so *we* nuke it - */ - if (file_created && - fname_copy != NULL) - (void) unlinkat(dirfd, fname_copy, 0); + if (fname_copy != NULL) + free(fname_copy); - free(template_copy); + return (*fd >= 0) ? *fd : -1; +} - template_copy = NULL; - } +static int +mkhtemp_try_create(int dirfd, + struct stat *st_dir_initial, + char *fname_copy, + char *p, + size_t xc, + int *fd, + struct stat *st) +{ + struct stat st_dir_now; - if (fname_copy != NULL) { + if (mkhtemp_fill_random(p, xc) < 0) + return -1; - free(fname_copy); - fname_copy = NULL; + if (fstat(dirfd, &st_dir_now) < 0) + return -1; + + if (st_dir_now.st_dev != st_dir_initial->st_dev || + st_dir_now.st_ino != st_dir_initial->st_ino) { + errno = ESTALE; + return -1; } - errno = saved_errno; + *fd = openat2p(dirfd, fname_copy, + O_RDWR | O_CREAT | O_EXCL | + O_NOFOLLOW | O_CLOEXEC | O_NOCTTY, + 0600); - /* returning EINTR/EAGAIN - ourselves means that a - caller could implement - the same wait loop - */ - if (errno != EEXIST && - errno != EINTR && - errno != EAGAIN) { + if (*fd >= 0) { + + if (secure_file(fd, st, O_APPEND, 1, 1, 0600) < 0) + return -1; - if (errno == saved_errno) - errno = ECANCELED; + return 1; /* success */ } - return -1; + if (errno == EEXIST || + errno == EINTR || + errno == EAGAIN) + return 0; /* retry */ + + return -1; /* fatal */ } int diff --git a/util/nvmutil/lib/io.c b/util/nvmutil/lib/io.c index 87163359..e95be73c 100644 --- a/util/nvmutil/lib/io.c +++ b/util/nvmutil/lib/io.c @@ -473,7 +473,7 @@ gbe_mv(void) /* create replacement temp in target directory */ - dest_tmp = new_tmpfile(&dest_fd, 1, f->fname); + dest_tmp = new_tmpfile(&dest_fd); if (dest_tmp == NULL) goto ret_gbe_mv; -- cgit v1.2.1