[PATCH] Fix some temp file issues

[Paul Eggert]

This patch was prompted by a linker warning "warning: the use of
`mktemp' is dangerous, better use `mkstemp' or `mkdtemp'" on
Fedora 36.  It also fixes a few unlikely bugs and simplifies the
code in a couple of places.
* src/misc.c (get_tmpdir): Now extern, for os_anontmp.
(get_tmpbase): New function, which generalizes the
old get_tmptemplate.
(get_tmptemplate): Use it.
(get_tmpfifoname): New function, which also uses get_tmpbase.
Fifo names are now /tmp/GMfifoNNNN, where NNNN is the process id;
there is no need to use mktemp for named fifos as mkfifo refuses
to create a fifo if the destination already exists, and there is
no denial of service as GNU Make silently falls back on a pipe if
the named fifo cannot be created.  Avoiding mktemp saves us some
syscalls and lets us pacify the glibc linker warning.
(get_tmppath) [HAVE_MKSTEMP && HAVE_FDOPEN]: Do not define, as
it's no longer called in this case.  This pacifies the glibc
linker warning on GNUish platforms.
(get_tmpfd) [HAVE_DUP && !WINDOWS32]: Use tmpfile for anonymous
files; on GNU/Linux this avoids a race where the file name is
temporarily visible.  Avoid two unnecessary calls to umask.
Report a fatal error if mkstemp or its fallback 'open' fail, since
the caller expects a nonnegative fd.
(get_tmpfile) [!HAVE_FDOPEN]: Use tmpfile for anonymous files.
Simplify.
* src/os.h (os_anontmp): Now always a function.
* src/posixos.c (jobserver_setup) [HAVE_MKFIFO]:
Use get_tmpfifoname instead of get_tmppath.
(os_anontmp): New function, that avoids making the file
temporarily visible, on GNUish platforms that support O_TMPFILE.
---
 src/makeint.h |  2 ++
 src/misc.c    | 79 +++++++++++++++++++++++++++++++--------------------
 src/os.h      |  4 ---
 src/posixos.c | 13 ++++++++-
 4 files changed, 62 insertions(+), 36 deletions(-)

diff --git a/src/makeint.h b/src/makeint.h
index 0bc41eb1..0b1ba058 100644
--- a/src/makeint.h
+++ b/src/makeint.h
@@ -569,6 +569,8 @@ int alpha_compare (const void *, const void *);
 void print_spaces (unsigned int);
 char *find_percent (char *);
 const char *find_percent_cached (const char **);
+const char *get_tmpdir ();
+char *get_tmpfifoname ();
 char *get_tmppath ();
 int get_tmpfd (char **);
 FILE *get_tmpfile (char **);
diff --git a/src/misc.c b/src/misc.c
index 011e4f22..6d77f9ce 100644
--- a/src/misc.c
+++ b/src/misc.c
@@ -566,7 +566,7 @@ umask (mode_t mask)
 # endif
 #endif
 
-static const char *
+const char *
 get_tmpdir ()
 {
   static const char *tmpdir = NULL;
@@ -586,48 +586,64 @@ get_tmpdir ()
 }
 
 static char *
-get_tmptemplate ()
+get_tmpbase (char const *base, int baselen)
 {
   const char *tmpdir = get_tmpdir ();
   char *template;
   size_t len;
 
   len = strlen (tmpdir);
-  template = xmalloc (len + CSTRLEN (DEFAULT_TMPFILE) + 2);
+  template = xmalloc (len + baselen + 2);
   strcpy (template, tmpdir);
 
 #ifdef HAVE_DOS_PATHS
   if (template[len - 1] != '/' && template[len - 1] != '\\')
-    strcat (template, "/");
-#else
-# ifndef VMS
+    template[len++] = '/';
+#elif !defined VMS
   if (template[len - 1] != '/')
-    strcat (template, "/");
-# endif /* !VMS */
-#endif /* !HAVE_DOS_PATHS */
+    template[len++] = '/';
+#endif
 
-  strcat (template, DEFAULT_TMPFILE);
+  strcpy (template + len, base);
 
   return template;
 }
 
+static char *
+get_tmptemplate ()
+{
+  return get_tmpbase (DEFAULT_TMPFILE, CSTRLEN (DEFAULT_TMPFILE));
+}
+
+char *
+get_tmpfifoname ()
+{
+  static char const fifo_prefix[] = "GMfifo";
+  long long pid = make_pid ();
+  char buf[CSTRLEN (fifo_prefix) + INTSTR_LENGTH + 1];
+  int baselen = sprintf (buf, "%s%" MK_PRI64_PREFIX "d", fifo_prefix, pid);
+  return get_tmpbase (buf, baselen);
+}
+
+#if !HAVE_MKSTEMP || !HAVE_FDOPEN
 char *
 get_tmppath ()
 {
   char *path;
 
-#ifdef HAVE_MKTEMP
+# ifdef HAVE_MKTEMP
   path = get_tmptemplate();
   if (*mktemp (path) == '\0')
     pfatal_with_name ("mktemp");
-#else
+# else
   path = xmalloc (L_tmpnam + 1);
   if (tmpnam (path) == NULL)
     pfatal_with_name ("tmpnam");
-#endif
+# endif
 
   return path;
 }
+#endif
 
 /* Generate a temporary file and return an fd for it.  If name is NULL then
    the temp file is anonymous and will be deleted when the process exits.  */
@@ -644,11 +660,22 @@ get_tmpfd (char **name)
       fd = os_anontmp ();
       if (fd >= 0)
         return fd;
+#if HAVE_DUP && !WINDOWS32
+      mask = umask (0077);
+      {
+        FILE *tfile = tmpfile ();
+        if (! tfile)
+          pfatal_with_name ("tmpfile");
+        umask (mask);
+        fd = dup (fileno (tfile));
+        if (fd < 0)
+          pfatal_with_name ("dup");
+        fclose (tfile);
+        return fd;
+      }
+#endif
     }
 
-  /* Preserve the current umask, and set a restrictive one for temp files.  */
-  mask = umask (0077);
-
 #if defined(HAVE_MKSTEMP)
   tmpnm = get_tmptemplate ();
 
@@ -660,8 +687,8 @@ get_tmpfd (char **name)
   /* Can't use mkstemp(), but try to guard against a race condition.  */
   EINTRLOOP (fd, open (tmpnm, O_CREAT|O_EXCL|O_RDWR, 0600));
 #endif
-
-  umask (mask);
+  if (fd < 0)
+    pfatal_with_name (tmpnm);
 
   if (name)
     *name = tmpnm;
@@ -685,21 +712,11 @@ get_tmpfile (char **name)
   /* Preserve the current umask, and set a restrictive one for temp files.  */
   mode_t mask = umask (0077);
 
-  char *tmpnm = get_tmppath ();
-
-  /* Not secure, but...?  If name is NULL we could use tmpfile()...  */
-  FILE *file = fopen (tmpnm, "w");
+  /* Although the fopen is not secure, this code is executed only on
+     non-fdopen platforms, which should be a rarity nowadays.  */
+  FILE *file = name ? fopen (*name = get_tmppath (), "wb+") : tmpfile ();
 
   umask (mask);
-
-  if (name)
-    *name = tmpnm;
-  else
-    {
-      unlink (tmpnm);
-      free (tmpnm);
-    }
-
   return file;
 #endif
 }
diff --git a/src/os.h b/src/os.h
index 81212bc0..a62063df 100644
--- a/src/os.h
+++ b/src/os.h
@@ -40,11 +40,7 @@ void fd_set_append (int);
 #endif
 
 /* Return a file descriptor for a new anonymous temp file, or -1.  */
-#if defined(WINDOWS32)
 int os_anontmp ();
-#else
-# define os_anontmp() (-1)
-#endif
 
 /* This section provides OS-specific functions to support the jobserver.  */
 
diff --git a/src/posixos.c b/src/posixos.c
index a7ef51bf..67549991 100644
--- a/src/posixos.c
+++ b/src/posixos.c
@@ -140,7 +140,7 @@ jobserver_setup (int slots, const char *style)
 #if HAVE_MKFIFO
   if (style == NULL || strcmp (style, "fifo") == 0)
     {
-      fifo_name = get_tmppath ();
+      fifo_name = get_tmpfifoname ();
 
       EINTRLOOP (r, mkfifo (fifo_name, 0600));
       if (r < 0)
@@ -807,3 +807,14 @@ fd_set_append (int fd)
     }
 #endif
 }
+
+/* Return a file descriptor for a new anonymous temp file, or -1.  */
+int
+os_anontmp (void)
+{
+#ifdef O_TMPFILE
+  return open (get_tmpdir (), O_RDWR | O_TMPFILE | O_EXCL, 0600);
+#else
+  return -1;
+#endif
+}
-- 
2.37.3