This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[patch] temporary file handling
- From: Vasiliy Kulikov <segoon at cvs dot openwall dot com>
- To: binutils at sourceware dot org
- Cc: Solar Designer <solar at openwall dot com>
- Date: Sun, 10 Oct 2010 13:42:58 +0400
- Subject: [patch] temporary file handling
Hi,
This patch fixes some temporary file handling issues:
* mktemp() returns an empty string on error, not NULL. The code
wrongly checked for the latter, but not for the former. This is now
corrected.
* make_tempname() and make_tempdir() didn't free the pointer returned
by template_in_dir() if an error occurred in a subsequent call, such
as to mkstemp() or open(). This might have made no difference since
binutils programs would typically call fatal() if make_temp*() fails,
however it makes more of a difference along with the change below:
* make_tempname() and make_tempdir() on systems without mkstemp()
and mkdtemp(), respectively, would abort immediately if the filename
is taken (by another process) between the calls to mktemp() and
open()/mkdir(). This patch improves the code to emulate mkstemp()
and mkdtemp() more closely by retrying the mktemp() and open()/mkdir()
in a loop (much like what mkstemp() and mkdtemp() typically do).
* choose_temp_base() is not a safe API since a filename made from a
* unique
filename with a suffix appended is not guaranteed unique anymore, and
since the filename was not reserved anyway. The patch replaces the uses
of choose_temp_base() with those of make_temp_file().
* tmpnam() is a racy API. Libiberty's implementation is buggy even
without the race: it checks file existence with fopen(file, "r").
If file is accessible only for write, then fopen() fails and tmpnam()
returns the name of an already existing file. tic54x_mlib() from
tc-tic54x.c called fopen(name, "w") with the name returned from tmpnam().
So a file with no read permissions could be overwritten, including over a
(sym)link. The patch corrects this by using make_temp_file() instead.
The tempbase field from struct pex_obj was needed only for
choose_temp_base(), so I've deleted it. pexecute() and some functions
don't need the tempbase argument anymore. However, in this patch I've
only commented out some code to ignore tempbase. You might want to fix
this in a cleaner but more invasive manner.
I've tested the patch by rebuilding the Openwall GNU/*/Linux userland with
the patched binutils in place (twice on each of i686 and x86_64, with the
second rebuild using development/build tools built the first time).
This patch was originally created for H. J. Lu's version of binutils,
but it looks OK for GNU's version.
Also you can find this patch (or possible revisions of it) here:
http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/binutils/
Thanks,
--
Vasiliy
diff -uNrp binutils-2.20.51.0.11.old/binutils/bucomm.c binutils-2.20.51.0.11/binutils/bucomm.c
--- binutils-2.20.51.0.11.old/binutils/bucomm.c 2010-09-30 16:55:35.000000000 +0000
+++ binutils-2.20.51.0.11/binutils/bucomm.c 2010-10-04 19:11:31.000000000 +0000
@@ -491,19 +491,33 @@ template_in_dir (const char *path)
char *
make_tempname (char *filename)
{
- char *tmpname = template_in_dir (filename);
+ char *tmpname;
int fd;
#ifdef HAVE_MKSTEMP
+ tmpname = template_in_dir (filename);
fd = mkstemp (tmpname);
#else
- tmpname = mktemp (tmpname);
- if (tmpname == NULL)
- return NULL;
- fd = open (tmpname, O_RDWR | O_CREAT | O_EXCL, 0600);
+ int i;
+ fd = -1;
+ for (i = 0; i < 1000; i++)
+ {
+ tmpname = template_in_dir (filename);
+ (void) mktemp (tmpname);
+ if (tmpname[0] == 0)
+ break;
+ fd = open (tmpname, O_RDWR | O_CREAT | O_EXCL, 0600);
+ if (fd != -1)
+ break;
+ free (tmpname);
+ tmpname = NULL;
+ }
#endif
if (fd == -1)
- return NULL;
+ {
+ free (tmpname);
+ return NULL;
+ }
close (fd);
return tmpname;
}
@@ -514,23 +528,36 @@ make_tempname (char *filename)
char *
make_tempdir (char *filename)
{
- char *tmpname = template_in_dir (filename);
-
+ char *tmpname, *res;
#ifdef HAVE_MKDTEMP
- return mkdtemp (tmpname);
+ tmpname = template_in_dir (filename);
+ res = mkdtemp (tmpname);
#else
- tmpname = mktemp (tmpname);
- if (tmpname == NULL)
- return NULL;
+ int i;
+ res = NULL;
+ for (i = 0; i < 1000; i++)
+ {
+ tmpname = template_in_dir (filename);
+ (void) mktemp (tmpname);
+ if (tmpname[0] == 0)
+ break;
+
#if defined (_WIN32) && !defined (__CYGWIN32__)
- if (mkdir (tmpname) != 0)
- return NULL;
+ if (mkdir (tmpname) == 0)
#else
- if (mkdir (tmpname, 0700) != 0)
- return NULL;
+ if (mkdir (tmpname, 0700) == 0)
#endif
- return tmpname;
+ {
+ res = tmpname;
+ break;
+ }
+ free (tmpname);
+ tmpname = NULL;
+ }
#endif
+ if (res == NULL)
+ free (tmpname);
+ return res;
}
/* Parse a string into a VMA, with a fatal error if it can't be
diff -uNrp binutils-2.20.51.0.11.old/binutils/dlltool.c binutils-2.20.51.0.11/binutils/dlltool.c
--- binutils-2.20.51.0.11.old/binutils/dlltool.c 2010-09-30 16:55:35.000000000 +0000
+++ binutils-2.20.51.0.11/binutils/dlltool.c 2010-10-01 12:59:09.000000000 +0000
@@ -1297,7 +1297,6 @@ run (const char *what, char *args)
int i;
const char **argv;
char *errmsg_fmt, *errmsg_arg;
- char *temp_base = choose_temp_base ();
inform ("run: %s %s", what, args);
@@ -1324,7 +1323,7 @@ run (const char *what, char *args)
}
argv[i++] = NULL;
- pid = pexecute (argv[0], (char * const *) argv, program_name, temp_base,
+ pid = pexecute (argv[0], (char * const *) argv, program_name, 0,
&errmsg_fmt, &errmsg_arg, PEXECUTE_ONE | PEXECUTE_SEARCH);
if (pid == -1)
diff -uNrp binutils-2.20.51.0.11.old/binutils/dllwrap.c binutils-2.20.51.0.11/binutils/dllwrap.c
--- binutils-2.20.51.0.11.old/binutils/dllwrap.c 2010-09-30 16:55:35.000000000 +0000
+++ binutils-2.20.51.0.11/binutils/dllwrap.c 2010-10-01 12:59:26.000000000 +0000
@@ -358,7 +358,6 @@ run (const char *what, char *args)
int i;
const char **argv;
char *errmsg_fmt, *errmsg_arg;
- char *temp_base = choose_temp_base ();
int in_quote;
char sep;
@@ -397,7 +396,7 @@ run (const char *what, char *args)
if (dry_run)
return 0;
- pid = pexecute (argv[0], (char * const *) argv, prog_name, temp_base,
+ pid = pexecute (argv[0], (char * const *) argv, prog_name, 0,
&errmsg_fmt, &errmsg_arg, PEXECUTE_ONE | PEXECUTE_SEARCH);
if (pid == -1)
@@ -823,13 +822,7 @@ main (int argc, char **argv)
if (! def_file_seen)
{
- char *fileprefix = choose_temp_base ();
-
- def_file_name = (char *) xmalloc (strlen (fileprefix) + 5);
- sprintf (def_file_name, "%s.def",
- (dontdeltemps) ? mybasename (fileprefix) : fileprefix);
- delete_def_file = 1;
- free (fileprefix);
+ def_file_name = make_temp_file(".def");
delete_def_file = 1;
warn (_("no export definition file provided.\n\
Creating one, but that may not be what you want"));
@@ -1036,12 +1029,8 @@ Creating one, but that may not be what y
if (! base_file_name)
{
- char *fileprefix = choose_temp_base ();
- base_file_name = (char *) xmalloc (strlen (fileprefix) + 6);
- sprintf (base_file_name, "%s.base",
- (dontdeltemps) ? mybasename (fileprefix) : fileprefix);
+ base_file_name = make_temp_file(".base");
delete_base_file = 1;
- free (fileprefix);
}
{
diff -uNrp binutils-2.20.51.0.11.old/binutils/resrc.c binutils-2.20.51.0.11/binutils/resrc.c
--- binutils-2.20.51.0.11.old/binutils/resrc.c 2010-09-30 16:55:35.000000000 +0000
+++ binutils-2.20.51.0.11/binutils/resrc.c 2010-10-01 12:59:41.000000000 +0000
@@ -207,7 +207,6 @@ run_cmd (char *cmd, const char *redir)
int i;
const char **argv;
char *errmsg_fmt, *errmsg_arg;
- char *temp_base = choose_temp_base ();
int in_quote;
char sep;
int redir_handle = -1;
@@ -270,7 +269,7 @@ run_cmd (char *cmd, const char *redir)
/* Redirect stdout to our output file. */
dup2 (redir_handle, STDOUT_FILENO);
- pid = pexecute (argv[0], (char * const *) argv, program_name, temp_base,
+ pid = pexecute (argv[0], (char * const *) argv, program_name, 0,
&errmsg_fmt, &errmsg_arg, PEXECUTE_ONE | PEXECUTE_SEARCH);
/* Restore stdout to its previous setting. */
@@ -318,12 +317,7 @@ open_input_stream (char *cmd)
{
if (istream_type == ISTREAM_FILE)
{
- char *fileprefix;
-
- fileprefix = choose_temp_base ();
- cpp_temp_file = (char *) xmalloc (strlen (fileprefix) + 5);
- sprintf (cpp_temp_file, "%s.irc", fileprefix);
- free (fileprefix);
+ cpp_temp_file = make_temp_file(".irc");
if (run_cmd (cmd, cpp_temp_file))
fatal (_("can't execute `%s': %s"), cmd, strerror (errno));
diff -uNrp binutils-2.20.51.0.11.old/gas/config/tc-tic54x.c binutils-2.20.51.0.11/gas/config/tc-tic54x.c
--- binutils-2.20.51.0.11.old/gas/config/tc-tic54x.c 2010-09-30 16:55:34.000000000 +0000
+++ binutils-2.20.51.0.11/gas/config/tc-tic54x.c 2010-10-01 13:26:34.000000000 +0000
@@ -2365,7 +2365,7 @@ tic54x_mlib (int ignore ATTRIBUTE_UNUSED
/* Get a size at least as big as the archive member. */
bfd_size_type size = bfd_get_size (mbfd);
char *buf = xmalloc (size);
- char *fname = tmpnam (NULL);
+ char *fname = make_temp_file(NULL);
FILE *ftmp;
/* We're not sure how big it is, but it will be smaller than "size". */
diff -uNrp binutils-2.20.51.0.11.old/include/libiberty.h binutils-2.20.51.0.11/include/libiberty.h
--- binutils-2.20.51.0.11.old/include/libiberty.h 2010-09-30 16:55:35.000000000 +0000
+++ binutils-2.20.51.0.11/include/libiberty.h 2010-10-01 12:26:10.000000000 +0000
@@ -220,9 +220,11 @@ extern char *make_relative_prefix (const
extern char *make_relative_prefix_ignore_links (const char *, const char *,
const char *) ATTRIBUTE_MALLOC;
+#if 0
/* Choose a temporary directory to use for scratch files. */
extern char *choose_temp_base (void) ATTRIBUTE_MALLOC;
+#endif
/* Return a temporary file name or NULL if unable to create one. */
diff -uNrp binutils-2.20.51.0.11.old/libiberty/choose-temp.c binutils-2.20.51.0.11/libiberty/choose-temp.c
--- binutils-2.20.51.0.11.old/libiberty/choose-temp.c 2010-09-30 16:55:35.000000000 +0000
+++ binutils-2.20.51.0.11/libiberty/choose-temp.c 2010-10-01 13:00:24.000000000 +0000
@@ -57,6 +57,7 @@ not recommended.
*/
+#if 0
char *
choose_temp_base (void)
{
@@ -73,3 +74,4 @@ choose_temp_base (void)
abort ();
return temp_filename;
}
+#endif
diff -uNrp binutils-2.20.51.0.11.old/libiberty/pex-common.c binutils-2.20.51.0.11/libiberty/pex-common.c
--- binutils-2.20.51.0.11.old/libiberty/pex-common.c 2010-09-30 16:55:35.000000000 +0000
+++ binutils-2.20.51.0.11/libiberty/pex-common.c 2010-10-01 13:02:25.000000000 +0000
@@ -58,7 +58,7 @@ pex_init_common (int flags, const char *
obj = XNEW (struct pex_obj);
obj->flags = flags;
obj->pname = pname;
- obj->tempbase = tempbase;
+ (void) tempbase;
obj->next_input = STDIN_FILE_NO;
obj->next_input_name = NULL;
obj->next_input_name_allocated = 0;
@@ -94,6 +94,7 @@ pex_add_remove (struct pex_obj *obj, con
obj->remove[obj->remove_count - 1] = add;
}
+#if 0
/* Generate a temporary file name based on OBJ, FLAGS, and NAME.
Return NULL if we were unable to reserve a temporary filename.
@@ -143,6 +144,7 @@ temp_file (struct pex_obj *obj, int flag
return name;
}
+#endif
/* As for pex_run (), but permits the environment for the child process
@@ -222,14 +224,14 @@ pex_run_in_environment (struct pex_obj *
out = STDOUT_FILE_NO;
else if ((flags & PEX_SUFFIX) != 0)
{
- outname = concat (obj->tempbase, outname, NULL);
+ outname = make_temp_file(outname);
outname_allocated = 1;
}
obj->next_input = -1;
}
else if ((obj->flags & PEX_USE_PIPES) == 0)
{
- outname = temp_file (obj, flags, outname);
+ outname = make_temp_file(outname);
if (! outname)
{
*err = 0;
@@ -391,7 +393,7 @@ pex_input_file (struct pex_obj *obj, int
return NULL;
}
- name = temp_file (obj, flags, name);
+ name = make_temp_file(name);
if (! name)
return NULL;
diff -uNrp binutils-2.20.51.0.11.old/libiberty/pex-common.h binutils-2.20.51.0.11/libiberty/pex-common.h
--- binutils-2.20.51.0.11.old/libiberty/pex-common.h 2010-09-30 16:55:35.000000000 +0000
+++ binutils-2.20.51.0.11/libiberty/pex-common.h 2010-10-01 12:31:29.000000000 +0000
@@ -57,8 +57,10 @@ struct pex_obj
int flags;
/* Name of calling program, for error messages. */
const char *pname;
+#if 0
/* Base name to use for temporary files. */
const char *tempbase;
+#endif
/* Pipe to use as stdin for next process. */
int next_input;
/* File name to use as stdin for next process. */
diff -uNrp binutils-2.20.51.0.11.old/libiberty/pex-msdos.c binutils-2.20.51.0.11/libiberty/pex-msdos.c
--- binutils-2.20.51.0.11.old/libiberty/pex-msdos.c 2010-09-30 16:55:35.000000000 +0000
+++ binutils-2.20.51.0.11/libiberty/pex-msdos.c 2010-10-01 13:03:10.000000000 +0000
@@ -161,7 +161,6 @@ pex_msdos_exec_child (struct pex_obj *ob
{
struct pex_msdos *ms;
char *temp_base;
- int temp_base_allocated;
char *rf;
int inindex;
char *infile;
@@ -177,6 +176,7 @@ pex_msdos_exec_child (struct pex_obj *ob
/* FIXME: I don't know how to redirect stderr, so we ignore ERRDES
and PEX_STDERR_TO_STDOUT. */
+#if 0
temp_base = obj->temp_base;
if (temp_base != NULL)
temp_base_allocated = 0;
@@ -190,6 +190,8 @@ pex_msdos_exec_child (struct pex_obj *ob
if (temp_base_allocated)
free (temp_base);
+#endif
+ rf = make_temp_file(".gp");
if (in == STDIN_FILE_NO)
{