This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Those obnoxious "mktemp is not safe" warnings


This patch splits libiberty/choose-temp.c into three pieces: one for
the unsafe choose_temp_base, one for the safe make_temp_file, and one
for the code shared between them (which was formerly duplicated in
both).  It will cause a slight code size reduction, but more
importantly it will stop the linker from warning us that mktemp is not
safe, unless we compile a program that actually *uses*
choose_temp_base, or mktemp itself.

I've verified that this does silence the warnings, and that
make_temp_file still works; I have not tested choose_temp_base.  I am
running an i686-linux bootstrap now.  Assuming it passes, ok to commit?

zw

libiberty:
	* choose-temp.c: Split out make_temp_file and directory search
	logic to...
	* make-temp-file.c, tmpdir.c: New files.  Sanitize directory
	search logic.
	* Makefile.in (CFILES): Add make-temp-file.c and tmpdir.c.
	(REQUIRED_OFILES): Add make-temp-file.o and tmpdir.o.

include:
	* ansidecl.h: If GCC_VERSION > 2007, define inline to
	__inline__.
	* libiberty.h: Prototype choose_tmpdir and mkstemps; update
	commentary.

===================================================================
Index: libiberty/Makefile.in
--- libiberty/Makefile.in	2001/03/10 10:41:24	1.59
+++ libiberty/Makefile.in	2001/03/19 19:45:42
@@ -125,12 +125,13 @@ CFILES = asprintf.c alloca.c argv.c atex
         cp-demangle.c dyn-string.c fdmatch.c fnmatch.c getcwd.c		      \
 	getpwd.c getopt.c getopt1.c getpagesize.c getruntime.c		      \
 	floatformat.c hashtab.c hex.c index.c insque.c lbasename.c            \
-	md5.c memchr.c							      \
+	make-temp-file.c md5.c memchr.c					      \
 	memcmp.c memcpy.c memmove.c memset.c mkstemps.c objalloc.c obstack.c  \
 	partition.c pexecute.c putenv.c random.c rename.c rindex.c setenv.c   \
 	sigsetmask.c safe-ctype.c sort.c spaces.c splay-tree.c strcasecmp.c   \
 	strncasecmp.c strchr.c strdup.c strerror.c strncmp.c strrchr.c        \
-	strsignal.c strstr.c strtod.c strtol.c strtoul.c tmpnam.c vasprintf.c \
+	strsignal.c strstr.c strtod.c strtol.c strtoul.c tmpnam.c tmpdir.c    \
+	vasprintf.c							      \
 	vfork.c vfprintf.c vprintf.c vsprintf.c waitpid.c xatexit.c xexit.c   \
 	xmalloc.c xmemdup.c xstrdup.c xstrerror.c
 
@@ -138,10 +139,10 @@ CFILES = asprintf.c alloca.c argv.c atex
 REQUIRED_OFILES = argv.o alloca.o choose-temp.o concat.o cplus-dem.o          \
 	cp-demangle.o dyn-string.o fdmatch.o fnmatch.o getopt.o getopt1.o     \
 	getpwd.o getruntime.o hashtab.o hex.o floatformat.o lbasename.o       \
-        md5.o objalloc.o						      \
+        make-temp-file.o md5.o objalloc.o				      \
 	obstack.o partition.o pexecute.o safe-ctype.o sort.o spaces.o         \
-	splay-tree.o strerror.o strsignal.o xatexit.o xexit.o xmalloc.o       \
-	xmemdup.o xstrdup.o xstrerror.o
+	splay-tree.o strerror.o strsignal.o tmpdir.o xatexit.o xexit.o 	      \
+	xmalloc.o xmemdup.o xstrdup.o xstrerror.o
 
 $(TARGETLIB): $(REQUIRED_OFILES) $(EXTRA_OFILES) $(LIBOBJS)
 	-rm -f $(TARGETLIB)
===================================================================
Index: libiberty/choose-temp.c
--- libiberty/choose-temp.c	2000/06/07 21:13:14	1.15
+++ libiberty/choose-temp.c	2001/03/19 19:45:42
@@ -1,5 +1,5 @@
 /* Utility to pick a temporary filename prefix.
-   Copyright (C) 1996, 1997, 1998 Free Software Foundation, Inc.
+   Copyright (C) 1996, 1997, 1998, 2001 Free Software Foundation, Inc.
 
 This file is part of the libiberty library.
 Libiberty is free software; you can redistribute it and/or
@@ -17,85 +17,26 @@ License along with libiberty; see the fi
 write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
 Boston, MA 02111-1307, USA.  */
 
-/* This file exports two functions: choose_temp_base and make_temp_file.  */
-
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
 
-#include <stdio.h>	/* May get P_tmpdir.  */
-#include <sys/types.h>
-#ifdef HAVE_UNISTD_H
-#include <unistd.h>
-#endif
+#include <stdio.h>	/* for mktemp */
 #ifdef HAVE_STDLIB_H
 #include <stdlib.h>
 #endif
 #ifdef HAVE_STRING_H
 #include <string.h>
 #endif
-#ifdef HAVE_SYS_FILE_H
-#include <sys/file.h>   /* May get R_OK, etc. on some systems.  */
-#endif
-
-#ifndef R_OK
-#define R_OK 4
-#define W_OK 2
-#define X_OK 1
-#endif
 
 #include "libiberty.h"
-extern int mkstemps ();
-
-#ifndef IN_GCC
-#if defined (__MSDOS__) || (defined (_WIN32) && ! defined (__CYGWIN__) && ! defined (_UWIN))
-#define DIR_SEPARATOR '\\'
-#endif
-#endif
 
-#ifndef DIR_SEPARATOR
-#define DIR_SEPARATOR '/'
-#endif
-
-/* On MSDOS, write temp files in current dir
-   because there's no place else we can expect to use.  */
-/* ??? Although the current directory is tried as a last resort,
-   this is left in so that on MSDOS it is preferred to /tmp on the
-   off chance that someone requires this, since that was the previous
-   behaviour.  */
-#ifdef __MSDOS__
-#ifndef P_tmpdir
-#define P_tmpdir "."
-#endif
-#endif
-
 /* Name of temporary file.
    mktemp requires 6 trailing X's.  */
 #define TEMP_FILE "ccXXXXXX"
-
-/* Subroutine of choose_temp_base.
-   If BASE is non-NULL, return it.
-   Otherwise it checks if DIR is a usable directory.
-   If success, DIR is returned.
-   Otherwise NULL is returned.  */
-
-static const char *try PARAMS ((const char *, const char *));
-
-static const char *
-try (dir, base)
-     const char *dir, *base;
-{
-  if (base != 0)
-    return base;
-  if (dir != 0
-      && access (dir, R_OK | W_OK | X_OK) == 0)
-    return dir;
-  return 0;
-}
+#define TEMP_FILE_LEN (sizeof(TEMP_FILE) - 1)
 
 /* Return a prefix for temporary file names or NULL if unable to find one.
-   The current directory is chosen if all else fails so the program is
-   exited if a temporary directory can't be found (mktemp fails).
    The buffer for the result is obtained with xmalloc. 
 
    This function is provided for backwards compatability only.  It use
@@ -104,102 +45,17 @@ try (dir, base)
 char *
 choose_temp_base ()
 {
-  const char *base = 0;
+  const char *base = choose_tmpdir ();
   char *temp_filename;
   int len;
-  static char tmp[] = { DIR_SEPARATOR, 't', 'm', 'p', 0 };
-  static char usrtmp[] = { DIR_SEPARATOR, 'u', 's', 'r', DIR_SEPARATOR, 't', 'm', 'p', 0 };
 
-  base = try (getenv ("TMPDIR"), base);
-  base = try (getenv ("TMP"), base);
-  base = try (getenv ("TEMP"), base);
-
-#ifdef P_tmpdir
-  base = try (P_tmpdir, base);
-#endif
-
-  /* Try /usr/tmp, then /tmp.  */
-  base = try (usrtmp, base);
-  base = try (tmp, base);
- 
-  /* If all else fails, use the current directory!  */
-  if (base == 0)
-    base = ".";
-
   len = strlen (base);
-  temp_filename = xmalloc (len + 1 /*DIR_SEPARATOR*/
-			   + strlen (TEMP_FILE) + 1);
+  temp_filename = xmalloc (len + TEMP_FILE_LEN + 1);
   strcpy (temp_filename, base);
-
-  if (len != 0
-      && temp_filename[len-1] != '/'
-      && temp_filename[len-1] != DIR_SEPARATOR)
-    temp_filename[len++] = DIR_SEPARATOR;
   strcpy (temp_filename + len, TEMP_FILE);
 
   mktemp (temp_filename);
   if (strlen (temp_filename) == 0)
-    abort ();
-  return temp_filename;
-}
-/* Return a temporary file name (as a string) or NULL if unable to create
-   one.  */
-
-char *
-make_temp_file (suffix)
-     const char *suffix;
-{
-  const char *base = 0;
-  char *temp_filename;
-  int base_len, suffix_len;
-  int fd;
-  static char tmp[] = { DIR_SEPARATOR, 't', 'm', 'p', 0 };
-  static char usrtmp[] = { DIR_SEPARATOR, 'u', 's', 'r', DIR_SEPARATOR, 't', 'm', 'p', 0 };
-
-  base = try (getenv ("TMPDIR"), base);
-  base = try (getenv ("TMP"), base);
-  base = try (getenv ("TEMP"), base);
-
-#ifdef P_tmpdir
-  base = try (P_tmpdir, base);
-#endif
-
-  /* Try /usr/tmp, then /tmp.  */
-  base = try (usrtmp, base);
-  base = try (tmp, base);
- 
-  /* If all else fails, use the current directory!  */
-  if (base == 0)
-    base = ".";
-
-  base_len = strlen (base);
-
-  if (suffix)
-    suffix_len = strlen (suffix);
-  else
-    suffix_len = 0;
-
-  temp_filename = xmalloc (base_len + 1 /*DIR_SEPARATOR*/
-			   + strlen (TEMP_FILE)
-			   + suffix_len + 1);
-  strcpy (temp_filename, base);
-
-  if (base_len != 0
-      && temp_filename[base_len-1] != '/'
-      && temp_filename[base_len-1] != DIR_SEPARATOR)
-    temp_filename[base_len++] = DIR_SEPARATOR;
-  strcpy (temp_filename + base_len, TEMP_FILE);
-
-  if (suffix)
-    strcat (temp_filename, suffix);
-
-  fd = mkstemps (temp_filename, suffix_len);
-  /* If mkstemps failed, then something bad is happening.  Maybe we should
-     issue a message about a possible security attack in progress?  */
-  if (fd == -1)
-    abort ();
-  /* Similarly if we can not close the file.  */
-  if (close (fd))
     abort ();
   return temp_filename;
 }
===================================================================
Index: libiberty/make-temp-file.c
--- libiberty/make-temp-file.c	Tue May  5 13:32:27 1998
+++ libiberty/make-temp-file.c	Mon Mar 19 11:45:42 2001
@@ -0,0 +1,77 @@
+/* Temporary file creator.
+   Copyright (C) 1996, 1997, 1998, 2001 Free Software Foundation, Inc.
+
+This file is part of the libiberty library.
+Libiberty is free software; you can redistribute it and/or
+modify it under the terms of the GNU Library General Public
+License as published by the Free Software Foundation; either
+version 2 of the License, or (at your option) any later version.
+
+Libiberty is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+Library General Public License for more details.
+
+You should have received a copy of the GNU Library General Public
+License along with libiberty; see the file COPYING.LIB.  If not,
+write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+Boston, MA 02111-1307, USA.  */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <stdio.h>	/* for mktemp */
+#ifdef HAVE_STDLIB_H
+#include <stdlib.h>
+#endif
+#ifdef HAVE_STRING_H
+#include <string.h>
+#endif
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+
+#include "libiberty.h"
+
+/* Name of temporary file.
+   mkstemps requires 6 trailing X's.  */
+#define TEMP_FILE "ccXXXXXX"
+#define TEMP_FILE_LEN (sizeof(TEMP_FILE) - 1)
+
+/* Utility to create a temporary file in an appropriate directory and
+   with a specified suffix on its name.  Returns the name of the temp
+   file, which will have been created, as a string; aborts if unable
+   to create one.  */
+
+char *
+make_temp_file (suffix)
+     const char *suffix;
+{
+  const char *base = choose_tmpdir ();
+  char *temp_filename;
+  int base_len, suffix_len;
+  int fd;
+
+  if (suffix == 0)
+    suffix = "";
+  base_len = strlen (base);
+  suffix_len = strlen (suffix);
+
+  temp_filename = xmalloc (base_len
+			   + TEMP_FILE_LEN
+			   + suffix_len + 1);
+  strcpy (temp_filename, base);
+  strcpy (temp_filename + base_len, TEMP_FILE);
+  strcpy (temp_filename + base_len + TEMP_FILE_LEN, suffix);
+
+  fd = mkstemps (temp_filename, suffix_len);
+  /* If mkstemps failed, then something bad is happening.  Maybe we should
+     issue a message about a possible security attack in progress?  */
+  if (fd == -1)
+    abort ();
+  /* Similarly if we can not close the file.  */
+  if (close (fd))
+    abort ();
+  return temp_filename;
+}
===================================================================
Index: libiberty/tmpdir.c
--- libiberty/tmpdir.c	Tue May  5 13:32:27 1998
+++ libiberty/tmpdir.c	Mon Mar 19 11:45:42 2001
@@ -0,0 +1,120 @@
+/* Utility to pick a temporary filename prefix.
+   Copyright (C) 1996, 1997, 1998, 2001 Free Software Foundation, Inc.
+
+This file is part of the libiberty library.
+Libiberty is free software; you can redistribute it and/or
+modify it under the terms of the GNU Library General Public
+License as published by the Free Software Foundation; either
+version 2 of the License, or (at your option) any later version.
+
+Libiberty is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+Library General Public License for more details.
+
+You should have received a copy of the GNU Library General Public
+License along with libiberty; see the file COPYING.LIB.  If not,
+write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+Boston, MA 02111-1307, USA.  */
+
+/* This file determines the appropriate place to store temporary
+   files, e.g. those created by make_temp_file.  */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <stdio.h>	/* May get P_tmpdir.  */
+#include <sys/types.h>
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+#ifdef HAVE_STDLIB_H
+#include <stdlib.h>
+#endif
+#ifdef HAVE_STRING_H
+#include <string.h>
+#endif
+#ifdef HAVE_SYS_FILE_H
+#include <sys/file.h>   /* May get R_OK, etc. on some systems.  */
+#endif
+
+#ifndef R_OK
+#define R_OK 4
+#define W_OK 2
+#define X_OK 1
+#endif
+
+#include "libiberty.h"
+
+/* Note: MSDOS accepts '/' as well as '\', so there is no need
+   to dink around with both.  */
+#ifndef DIR_SEPARATOR
+#define DIR_SEPARATOR '/'
+#endif
+
+/* Subroutine of choose_tempdir.
+   If BASE is non-NULL, return it.
+   Otherwise it checks if DIR is a usable directory.
+   If success, DIR is returned.
+   Otherwise NULL is returned.  */
+
+static inline const char *try PARAMS ((const char *, const char *));
+
+static inline const char *
+try (dir, base)
+     const char *dir, *base;
+{
+  if (base != 0)
+    return base;
+  if (dir != 0 && access (dir, R_OK | W_OK | X_OK) == 0)
+    return dir;
+  return 0;
+}
+
+static const char tmp[] = { DIR_SEPARATOR, 't', 'm', 'p', 0 };
+static const char usrtmp[] =
+  { DIR_SEPARATOR, 'u', 's', 'r', DIR_SEPARATOR, 't', 'm', 'p', 0 };
+static const char vartmp[] =
+  { DIR_SEPARATOR, 'v', 'a', 'r', DIR_SEPARATOR, 't', 'm', 'p', 0 };
+
+static char *memoized_tmpdir;
+
+/* XXX Potential memory leak in presence of multiple threads.  */
+char *
+choose_tmpdir ()
+{
+  const char *base = 0;
+  char *tmpdir;
+  int len;
+
+  if (memoized_tmpdir)
+    return memoized_tmpdir;
+
+  base = try (getenv ("TMPDIR"), base);
+  base = try (getenv ("TMP"), base);
+  base = try (getenv ("TEMP"), base);
+
+#ifdef P_tmpdir
+  base = try (P_tmpdir, base);
+#endif
+
+  /* Try /var/tmp, /usr/tmp, then /tmp.  */
+  base = try (vartmp, base);
+  base = try (usrtmp, base);
+  base = try (tmp, base);
+
+  /* If all else fails, use the current directory!  */
+  if (base == 0)
+    base = ".";
+
+  /* Stick a slash on the end, save it, and return it.  */
+  len = strlen (base);
+  tmpdir = xmalloc (len + 2 /* trailing slash, nul */);
+  strcpy (tmpdir, base);
+  tmpdir[len] = '/';
+  tmpdir[len + 1] = '\0';
+
+  memoized_tmpdir = tmpdir;
+  return tmpdir;
+}
===================================================================
Index: include/ansidecl.h
--- include/ansidecl.h	2001/03/14 19:44:38	1.8
+++ include/ansidecl.h	2001/03/19 19:45:42
@@ -230,4 +230,10 @@ So instead we use the macro below and te
 #define __extension__
 #endif
 
+/* For GCC >2.7, use __inline__ to avoid warnings.  */
+#if GCC_VERSION >=  2007
+# undef inline
+# define inline __inline__
+#endif
+
 #endif	/* ansidecl.h	*/
===================================================================
Index: include/libiberty.h
--- include/libiberty.h	2001/03/14 19:44:38	1.17
+++ include/libiberty.h	2001/03/19 19:45:42
@@ -103,13 +103,27 @@ extern char * getpwd PARAMS ((void));
 
 extern long get_run_time PARAMS ((void));
 
-/* Choose a temporary directory to use for scratch files.  */
+/* Choose a temporary directory to use for scratch files.  Always
+   returns the same string, which is malloced, but not yours to free.
+   The pathname returned is guaranteed to have a trailing slash.  */
 
+extern char *choose_tmpdir PARAMS ((void));
+
+/* Generate a base name for scratch files.  Deprecated due to security
+   issues; use make_temp_file.  */
+
 extern char *choose_temp_base PARAMS ((void)) ATTRIBUTE_MALLOC;
+
+/* Return a temporary file name or NULL if unable to create one.
+   The argument is a suffix to append to the file name.  */
+
+extern char *make_temp_file PARAMS ((const char *suffix)) ATTRIBUTE_MALLOC;
 
-/* Return a temporary file name or NULL if unable to create one.  */
+/* Primitive used by the above.  Like mkstemp, but (a) guaranteed to
+   exist, and (b) there's a suffix after the six X's, which is of
+   length SUFFIX_LEN.  */
 
-extern char *make_temp_file PARAMS ((const char *)) ATTRIBUTE_MALLOC;
+extern int mkstemps PARAMS ((char *template, int suffix_len));
 
 /* Allocate memory filled with spaces.  Allocates using malloc.  */
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]