Rework temp file handling to avoid GNU libc warnings

Original patch provided by Paul Eggert <eggert@cs.ucla.edu>.
GNU libc will generate a link-time warning if we use mktemp() even
though we are using it safely (we only use it with mkfifo()).
Avoid this and clean up some handling.

First, check all calls related to temporary files and exit with a
fatal error and a useful message if we can't obtain them.  In some
situations it might be possible to continue with reduced capability
but it's not worth the effort.

On POSIX systems we can create anonymous temp files using O_TMPFILE
if it's supported, else if we're using the default location and we
have dup(2), we can use standard tmpfile() and get an FD from it.

If we need a named temp file and FILE* and we have mkstemp() we can
use that, else if we have fdopen() we can get a temp FD and open it.
If none of those are available all we can do is generate a temp name
then open it with fopen() which is not secure.

* src/makeint.h (get_tmpdir): Declare it for use elsewhere.
* src/misc.c (get_tmpdir): Make it public not static.
(get_tmptemplate): Simplify the implementation.
(get_tmppath): Only define this if we have to have it to avoid
warnings from GNU libc.
(get_tmpfd): Generate fatal errors on error.
(get_tmpfile): Ditto.  Open files in "wb+" mode to match tmpfile().
Require a filename pointer (all callers want it).
* src/os.h (os_anontmp): Implement for posixos.c as well.
* src/posix.c (jobserver_setup): Don't use mktemp to avoid GNU libc
errors.  Instead construct the FIFO name based on the PID.
(osync_setup): get_tmpfd() can't fail so don't check it.
(os_anontmp): If the system supports O_TMPFILE use it.  If not, and
we want to create the temporary file in the default directory, we
can use tmpfile() then use dup() to copy the file descriptor.
* src/main.c (main): get_tmpfile() can't fail.
* src/vmsjobs.c (child_execute_job): get_tmpfile() can't fail.

6 files changed, 111 insertions, 66 deletions

diff --git a/src/main.c b/src/main.c
index 41490f4e..886c5218 100644
--- a/src/main.c
+++ b/src/main.c
@@ -1921,9 +1921,7 @@ main (int argc, char **argv, char **envp)
                  _("Makefile from standard input specified twice"));
 
             outfile = get_tmpfile (&newnm);
-            if (outfile == 0)
-              OSS (fatal, NILF,
-                   _("fopen: temporary file %s: %s"), newnm, strerror (errno));
+
             while (!feof (stdin) && ! ferror (stdin))
               {
                 char buf[2048];
@@ -1934,8 +1932,8 @@ main (int argc, char **argv, char **envp)
               }
             fclose (outfile);
 
-            /* Replace the name that read_all_makefiles will
-               see with the name of the temporary file.  */
+            /* Replace the name that read_all_makefiles will see with the name
+               of the temporary file.  */
             makefiles->list[i] = strcache_add (newnm);
             stdin_offset = i;
 
diff --git a/src/makeint.h b/src/makeint.h
index 8ab7ea84..d3674fe7 100644
--- a/src/makeint.h
+++ b/src/makeint.h
@@ -509,6 +509,15 @@ extern struct rlimit stack_limit;
 # define TTYNAME(_f) DEFAULT_TTYNAME
 #endif
 
+#ifdef VMS
+# define DEFAULT_TMPDIR     "/sys$scratch/"
+#elif defined(P_tmpdir)
+# define DEFAULT_TMPDIR     P_tmpdir
+#else
+# define DEFAULT_TMPDIR     "/tmp"
+#endif
+
+
 
 
 struct file;
@@ -572,7 +581,7 @@ int alpha_compare (const void *, const void *);
 void print_spaces (unsigned int);
 char *find_percent (char *);
 const char *find_percent_cached (const char **);
-char *get_tmppath (void);
+const char *get_tmpdir (void);
 int get_tmpfd (char **);
 FILE *get_tmpfile (char **);
 ssize_t writebuf (int, const void *, size_t);
diff --git a/src/misc.c b/src/misc.c
index 264eeb6a..a88584b6 100644
--- a/src/misc.c
+++ b/src/misc.c
@@ -555,17 +555,11 @@ umask (mode_t mask)
 #define MAKE_TMPDIR         "MAKE_TMPDIR"
 #ifdef VMS
 # define DEFAULT_TMPFILE    "sys$scratch:gnv$make_cmdXXXXXX.com"
-# define DEFAULT_TMPDIR     "/sys$scratch/"
 #else
-# define DEFAULT_TMPFILE     "GmXXXXXX"
-# ifdef P_tmpdir
-#  define DEFAULT_TMPDIR    P_tmpdir
-# else
-#  define DEFAULT_TMPDIR    "/tmp"
-# endif
+# define DEFAULT_TMPFILE    "GmXXXXXX"
 #endif
 
-static const char *
+const char *
 get_tmpdir ()
 {
   static const char *tmpdir = NULL;
@@ -589,44 +583,45 @@ get_tmptemplate ()
 {
   const char *tmpdir = get_tmpdir ();
   char *template;
-  size_t len;
+  char *cp;
 
-  len = strlen (tmpdir);
-  template = xmalloc (len + CSTRLEN (DEFAULT_TMPFILE) + 2);
-  strcpy (template, tmpdir);
+  template = xmalloc (strlen (tmpdir) + CSTRLEN (DEFAULT_TMPFILE) + 2);
+  cp = stpcpy (template, tmpdir);
 
-#ifdef HAVE_DOS_PATHS
-  if (template[len - 1] != '/' && template[len - 1] != '\\')
-    strcat (template, "/");
-#else
-# ifndef VMS
-  if (template[len - 1] != '/')
-    strcat (template, "/");
-# endif /* !VMS */
-#endif /* !HAVE_DOS_PATHS */
+#if !defined VMS
+  /* It's not possible for tmpdir to be empty.  */
+  if (! ISDIRSEP (cp[-1]))
+    *(cp++) = '/';
+#endif
 
-  strcat (template, DEFAULT_TMPFILE);
+  strcpy (cp, DEFAULT_TMPFILE);
 
   return template;
 }
 
-char *
+#if !HAVE_MKSTEMP || !HAVE_FDOPEN
+/* Generate a temporary filename.  This is not safe as another program could
+   snipe our filename after we've generated it: use this only on systems
+   without more secure alternatives.  */
+
+static char *
 get_tmppath ()
 {
   char *path;
 
-#ifdef HAVE_MKTEMP
-  path = get_tmptemplate();
+# 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.  */
@@ -635,7 +630,6 @@ get_tmpfd (char **name)
 {
   int fd = -1;
   char *tmpnm;
-  mode_t mask;
 
   /* If there's an os-specific way to get an anoymous temp file use it.  */
   if (!name)
@@ -645,9 +639,6 @@ get_tmpfd (char **name)
         return fd;
     }
 
-  /* Preserve the current umask, and set a restrictive one for temp files.  */
-  mask = umask (0077);
-
 #if defined(HAVE_MKSTEMP)
   tmpnm = get_tmptemplate ();
 
@@ -659,48 +650,61 @@ 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)
+    OSS (fatal, NILF,
+         _("create temporary file %s: %s"), tmpnm, strerror (errno));
 
   if (name)
     *name = tmpnm;
   else
     {
-      unlink (tmpnm);
+      int r;
+      EINTRLOOP (r, unlink (tmpnm));
+      if (r < 0)
+        OSS (fatal, NILF,
+             _("unlink temporary file %s: %s"), tmpnm, strerror (errno));
       free (tmpnm);
     }
 
   return fd;
 }
 
+/* Return a FILE* for a temporary file, opened in the safest way possible.
+   Set name to point to an allocated buffer containing the name of the file.
+   Note, this cannot be NULL!  */
 FILE *
 get_tmpfile (char **name)
 {
+  /* Be consistent with tmpfile, which opens as if by "wb+".  */
+  const char *tmpfile_mode = "wb+";
+  FILE *file;
+
 #if defined(HAVE_FDOPEN)
   int fd = get_tmpfd (name);
 
-  return fd < 0 ? NULL : fdopen (fd, "w");
+  ENULLLOOP (file, fdopen (fd, tmpfile_mode));
+  if (file == NULL)
+    OSS (fatal, NILF,
+         _("fdopen: temporary file %s: %s"), *name, strerror (errno));
 #else
   /* Preserve the current umask, and set a restrictive one for temp files.  */
   mode_t mask = umask (0077);
+  int err;
 
-  char *tmpnm = get_tmppath ();
+  *name = get_tmppath ();
 
-  /* Not secure, but...?  If name is NULL we could use tmpfile()...  */
-  FILE *file = fopen (tmpnm, "w");
+  /* Although this fopen is insecure, it is executed only on non-fdopen
+     platforms, which should be a rarity nowadays.  */
 
-  umask (mask);
+  ENULLLOOP (file, fopen (*name, tmpfile_mode));
+  if (file == NULL)
+    OSS (fatal, NILF,
+         _("fopen: temporary file %s: %s"), *name, strerror (errno));
 
-  if (name)
-    *name = tmpnm;
-  else
-    {
-      unlink (tmpnm);
-      free (tmpnm);
-    }
+  umask (mask);
+#endif
 
   return file;
-#endif
 }
 
 
diff --git a/src/os.h b/src/os.h
index 2fe3adac..85fa28d8 100644
--- a/src/os.h
+++ b/src/os.h
@@ -14,7 +14,6 @@ A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
 You should have received a copy of the GNU General Public License along with
 this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-
 #define IO_UNKNOWN              0x0001
 #define IO_COMBINED_OUTERR      0x0002
 #define IO_STDIN_OK             0x0004
@@ -26,6 +25,7 @@ this program.  If not, see <http://www.gnu.org/licenses/>.  */
 # define fd_inherit(_i)    (0)
 # define fd_noinherit(_i)  (0)
 # define fd_set_append(_i) (void)(0)
+# define os_anontmp()      (-1)
 #else
 
 /* Determine the state of stdin/stdout/stderr.  */
@@ -37,13 +37,9 @@ void fd_noinherit (int);
 
 /* If the file descriptor is for a file put it into append mode.  */
 void fd_set_append (int);
-#endif
 
 /* Return a file descriptor for a new anonymous temp file, or -1.  */
-#if defined(WINDOWS32)
 int os_anontmp (void);
-#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 43656b73..ed3b775d 100644
--- a/src/posixos.c
+++ b/src/posixos.c
@@ -143,7 +143,16 @@ jobserver_setup (int slots, const char *style)
 #if HAVE_MKFIFO
   if (style == NULL || strcmp (style, "fifo") == 0)
     {
-      fifo_name = get_tmppath ();
+  /* Unfortunately glibc warns about uses of mktemp even though we aren't
+     using it in dangerous way here.  So avoid this by generating our own
+     temporary file name.  */
+# define  FNAME_PREFIX "GMfifo"
+      const char *tmpdir = get_tmpdir ();
+
+      fifo_name = xmalloc (strlen (tmpdir) + CSTRLEN (FNAME_PREFIX)
+                           + INTSTR_LENGTH + 2);
+      sprintf (fifo_name, "%s/" FNAME_PREFIX "%" MK_PRI64_PREFIX "d",
+               tmpdir, (long long)make_pid ());
 
       EINTRLOOP (r, mkfifo (fifo_name, 0600));
       if (r < 0)
@@ -636,11 +645,8 @@ void
 osync_setup ()
 {
   osync_handle = get_tmpfd (&osync_tmpfile);
-  if (osync_handle >= 0)
-    {
-      fd_noinherit (osync_handle);
-      sync_root = 1;
-    }
+  fd_noinherit (osync_handle);
+  sync_root = 1;
 }
 
 char *
@@ -827,3 +833,36 @@ fd_set_append (int fd)
     }
 #endif
 }
+
+/* Return a file descriptor for a new anonymous temp file, or -1.  */
+int
+os_anontmp ()
+{
+  int fd = -1;
+
+#ifdef O_TMPFILE
+  EINTRLOOP (fd, open (get_tmpdir (), O_RDWR | O_TMPFILE | O_EXCL, 0600));
+  if (fd < 0)
+    pfatal_with_name ("open(O_TMPFILE)");
+#elif HAVE_DUP
+  /* We don't have O_TMPFILE but we can dup: if we are creating temp files in
+     the default location then try tmpfile() + dup() + fclose() to avoid ever
+     having a name for a file.  */
+  if (streq (get_tmpdir (), DEFAULT_TMPDIR))
+    {
+      mode_t mask = umask (0077);
+      FILE *tfile;
+      ENULLLOOP (tfile, tmpfile ());
+      if (!tfile)
+        pfatal_with_name ("tmpfile");
+      umask (mask);
+
+      EINTRLOOP (fd, dup (fileno (tfile)));
+      if (fd < 0)
+        pfatal_with_name ("dup");
+      fclose (tfile);
+    }
+#endif
+
+  return fd;
+}
diff --git a/src/vmsjobs.c b/src/vmsjobs.c
index e3226acf..96cec534 100644
--- a/src/vmsjobs.c
+++ b/src/vmsjobs.c
@@ -1242,8 +1242,7 @@ child_execute_job (struct childbase *child, int good_stdin UNUSED, char *argv)
       int cmd_len;
 
       outfile = get_tmpfile (&child->comname);
-      if (outfile == 0)
-        pfatal_with_name (_("fopen (temporary file)"));
+
       comnamelen = strlen (child->comname);
 
       /* The whole DCL "script" is executed as one action, and it behaves as