Commit 4ac5972811a711f9b26e34fd6b6d6df7f5407953

Peter Pettersson 2022-01-14T01:54:09

futils_mktmp: don't use umask. Previously, we were using `umask(mask = umask(0))` to fetch the current umask in order to apply it to the desired mode, but this is broken in the presence of threads. There is no portable way to directly fetch umask without mutating it. Instead, add a reimplementation of mkstemp that uses a passed-in mode, instead of explicitly chmodding to 0600 like POSIX requires of mkstemp. Fixes: jmgao/pore#46

diff --git a/src/futils.c b/src/futils.c
index 454ed79..318e878 100644
--- a/src/futils.c
+++ b/src/futils.c
@@ -26,30 +26,32 @@ int git_futils_mkpath2file(const char *file_path, const mode_t mode)
 
 int git_futils_mktmp(git_str *path_out, const char *filename, mode_t mode)
 {
+	const int open_flags = O_RDWR | O_CREAT | O_EXCL | O_BINARY | O_CLOEXEC;
+	/* TMP_MAX is unrelated to mktemp but should provide a reasonable amount */
+	unsigned int tries = TMP_MAX;
 	int fd;
-	mode_t mask;
 
-	p_umask(mask = p_umask(0));
+	while (tries--) {
+		git_str_sets(path_out, filename);
+		git_str_puts(path_out, "_git2_XXXXXX");
 
-	git_str_sets(path_out, filename);
-	git_str_puts(path_out, "_git2_XXXXXX");
-
-	if (git_str_oom(path_out))
-		return -1;
+		if (git_str_oom(path_out))
+			return -1;
 
-	if ((fd = p_mkstemp(path_out->ptr)) < 0) {
-		git_error_set(GIT_ERROR_OS,
-			"failed to create temporary file '%s'", path_out->ptr);
-		return -1;
-	}
+		/* Using mktemp is safe when we open with O_CREAT | O_EXCL */
+		p_mktemp(path_out->ptr);
+		/* mktemp sets template to empty string on failure */
+		if (path_out->ptr[0] == '\0')
+			break;
 
-	if (p_chmod(path_out->ptr, (mode & ~mask))) {
-		git_error_set(GIT_ERROR_OS,
-			"failed to set permissions on file '%s'", path_out->ptr);
-		return -1;
+		if ((fd = p_open(path_out->ptr, open_flags, mode)) >= 0)
+			return fd;
 	}
 
-	return fd;
+	git_error_set(GIT_ERROR_OS,
+		"failed to create temporary file '%s'", path_out->ptr);
+	git_str_dispose(path_out);
+	return -1;
 }
 
 int git_futils_creat_withpath(const char *path, const mode_t dirmode, const mode_t mode)
diff --git a/src/posix.h b/src/posix.h
index d98bc82..1757764 100644
--- a/src/posix.h
+++ b/src/posix.h
@@ -9,6 +9,7 @@
 
 #include "common.h"
 
+#include <stdlib.h>
 #include <fcntl.h>
 #include <time.h>
 
@@ -130,6 +131,7 @@ extern ssize_t p_pwrite(int fd, const void *data, size_t size, off64_t offset);
 
 #define p_close(fd) close(fd)
 #define p_umask(m) umask(m)
+#define p_mktemp(p) mktemp(p)
 
 extern int p_open(const char *path, int flags, ...);
 extern int p_creat(const char *path, mode_t mode);