Use make_tempname file descriptor in smart_rename

This patch makes use of the temp file descriptor in smart_rename
rather than reopening the file.  I don't believe there is a security
issue in reopening the file, but this way is one less directory
operation.  The patch also attempts to preserve S_ISUID and S_ISGID.

	PR 27456
	* bucomm.h (smart_rename): Update prototype.
	* rename.c (smart_rename): Add fromfd and preserve_dates params.
	Pass fromfd and target_stat to simple_copy.  Call set_times
	when preserve_dates.
	(simple_copy): Accept fromfd rather than from filename.  Add
	target_stat param.  Rewind fromfd rather than opening.  Open
	"to" file without O_CREAT.  Try to preserve S_ISUID and S_ISGID.
	* ar.c (write_archive): Rename ofd to tmpfd.  Dup tmpfd before
	closing output temp file, and pass tmpfd to smart_rename.
	* arsup.c (temp_fd): Rename from real_fd.
	(ar_save): Dup temp_fd and pass to smart_rename.
	* objcopy.c (strip_main, copy_main): Likewise, and pass
	preserve_dates.

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 80bdf92c9a14763f26f9007f9d921816fd0c5ce5..42b6954911f5e8bf23d1b55f0382c5a5da2377a8 100644 (file)
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,21 @@
+2021-02-24  Alan Modra  <amodra@gmail.com>
+           Siddhesh Poyarekar  <siddhesh@gotplt.org>
+
+       PR 27456
+       * bucomm.h (smart_rename): Update prototype.
+       * rename.c (smart_rename): Add fromfd and preserve_dates params.
+       Pass fromfd and target_stat to simple_copy.  Call set_times
+       when preserve_dates.
+       (simple_copy): Accept fromfd rather than from filename.  Add
+       target_stat param.  Rewind fromfd rather than opening.  Open
+       "to" file without O_CREAT.  Try to preserve S_ISUID and S_ISGID.
+       * ar.c (write_archive): Rename ofd to tmpfd.  Dup tmpfd before
+       closing output temp file, and pass tmpfd to smart_rename.
+       * arsup.c (temp_fd): Rename from real_fd.
+       (ar_save): Dup temp_fd and pass to smart_rename.
+       * objcopy.c (strip_main, copy_main): Likewise, and pass
+       preserve_dates.
+
 2021-02-24  Alan Modra  <amodra@gmail.com>
 
        PR 27456

diff --git a/binutils/ar.c b/binutils/ar.c
index 44df48c5c677c9ab53b8ad4856a62479de29a38e..fb19b14fec5784698d2d1a61811b34484db8e424 100644 (file)
--- a/binutils/ar.c
+++ b/binutils/ar.c
@@ -1252,21 +1252,21 @@ write_archive (bfd *iarch)
   bfd *obfd;
   char *old_name, *new_name;
   bfd *contents_head = iarch->archive_next;
-  int ofd = -1;
+  int tmpfd = -1;
 
   old_name = xstrdup (bfd_get_filename (iarch));
-  new_name = make_tempname (old_name, &ofd);
+  new_name = make_tempname (old_name, &tmpfd);
 
   if (new_name == NULL)
     bfd_fatal (_("could not create temporary file whilst writing archive"));
 
   output_filename = new_name;
 
-  obfd = bfd_fdopenw (new_name, bfd_get_target (iarch), ofd);
+  obfd = bfd_fdopenw (new_name, bfd_get_target (iarch), tmpfd);
 
   if (obfd == NULL)
     {
-      close (ofd);
+      close (tmpfd);
       bfd_fatal (old_name);
     }
 
@@ -1297,6 +1297,7 @@ write_archive (bfd *iarch)
   if (!bfd_set_archive_head (obfd, contents_head))
     bfd_fatal (old_name);
 
+  tmpfd = dup (tmpfd);
   if (!bfd_close (obfd))
     bfd_fatal (old_name);
 
@@ -1306,7 +1307,7 @@ write_archive (bfd *iarch)
   /* We don't care if this fails; we might be creating the archive.  */
   bfd_close (iarch);
 
-  if (smart_rename (new_name, old_name, NULL) != 0)
+  if (smart_rename (new_name, old_name, tmpfd, NULL, FALSE) != 0)
     xexit (1);
   free (old_name);
   free (new_name);

diff --git a/binutils/arsup.c b/binutils/arsup.c
index f7ce8f0bc82066cfe287f8c0a9d089bdad9f438d..9982484dbe0cfbc5659b91ae781f2222b4e487ff 100644 (file)
--- a/binutils/arsup.c
+++ b/binutils/arsup.c
@@ -43,7 +43,7 @@ extern int deterministic;
 static bfd *obfd;
 static char *real_name;
 static char *temp_name;
-static int real_ofd;
+static int temp_fd;
 static FILE *outfile;
 
 static void
@@ -152,7 +152,7 @@ void
 ar_open (char *name, int t)
 {
   real_name = xstrdup (name);
-  temp_name = make_tempname (real_name, &real_ofd);
+  temp_name = make_tempname (real_name, &temp_fd);
 
   if (temp_name == NULL)
     {
@@ -162,7 +162,7 @@ ar_open (char *name, int t)
       return;
     }
 
-  obfd = bfd_fdopenw (temp_name, NULL, real_ofd);
+  obfd = bfd_fdopenw (temp_name, NULL, temp_fd);
 
   if (!obfd)
     {
@@ -348,6 +348,7 @@ ar_save (void)
       if (deterministic > 0)
         obfd->flags |= BFD_DETERMINISTIC_OUTPUT;
 
+      temp_fd = dup (temp_fd);
       bfd_close (obfd);
 
       if (stat (real_name, &target_stat) != 0)
@@ -363,7 +364,7 @@ ar_save (void)
            }
        }
 
-      smart_rename (temp_name, real_name, NULL);
+      smart_rename (temp_name, real_name, temp_fd, NULL, FALSE);
       obfd = 0;
       free (temp_name);
       free (real_name);

diff --git a/binutils/bucomm.h b/binutils/bucomm.h
index aa7e33d8cd15e04ab12691bb29cb4365080998fc..f1ae47fa1b5660fff74d82f58fb412e95a88efe8 100644 (file)
--- a/binutils/bucomm.h
+++ b/binutils/bucomm.h
@@ -71,7 +71,8 @@ extern void print_version (const char *);
 /* In rename.c.  */
 extern void set_times (const char *, const struct stat *);
 
-extern int smart_rename (const char *, const char *, struct stat *);
+extern int smart_rename (const char *, const char *, int,
+                        struct stat *, bfd_boolean);
 
 
 /* In libiberty.  */

diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index abbcb7c51907527e2dfa1d8c1f59c550929f3479..90ae0bd46bd0ff421fdbaa55c35fa92d5e8783c7 100644 (file)
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -4837,6 +4837,7 @@ strip_main (int argc, char *argv[])
       struct stat statbuf;
       char *tmpname;
       int tmpfd = -1;
+      int copyfd = -1;
 
       if (get_file_size (argv[i]) < 1)
        {
@@ -4846,7 +4847,11 @@ strip_main (int argc, char *argv[])
 
       if (output_file == NULL
          || filename_cmp (argv[i], output_file) == 0)
-       tmpname = make_tempname (argv[i], &tmpfd);
+       {
+         tmpname = make_tempname (argv[i], &tmpfd);
+         if (tmpfd >= 0)
+           copyfd = dup (tmpfd);
+       }
       else
        tmpname = output_file;
 
@@ -4864,14 +4869,18 @@ strip_main (int argc, char *argv[])
       if (status == 0)
        {
          if (output_file != tmpname)
-           status = (smart_rename (tmpname,
-                                   output_file ? output_file : argv[i],
-                                   preserve_dates ? &statbuf : NULL) != 0);
+           status = smart_rename (tmpname,
+                                  output_file ? output_file : argv[i],
+                                  copyfd, &statbuf, preserve_dates) != 0;
          if (status == 0)
            status = hold_status;
        }
       else
-       unlink_if_ordinary (tmpname);
+       {
+         if (copyfd >= 0)
+           close (copyfd);
+         unlink_if_ordinary (tmpname);
+       }
       if (output_file != tmpname)
        free (tmpname);
     }
@@ -5078,7 +5087,9 @@ copy_main (int argc, char *argv[])
   bfd_boolean formats_info = FALSE;
   bfd_boolean use_globalize = FALSE;
   bfd_boolean use_keep_global = FALSE;
-  int c, tmpfd = -1;
+  int c;
+  int tmpfd = -1;
+  int copyfd;
   struct stat statbuf;
   const bfd_arch_info_type *input_arch = NULL;
 
@@ -5916,10 +5927,15 @@ copy_main (int argc, char *argv[])
     }
 
   /* If there is no destination file, or the source and destination files
-     are the same, then create a temp and rename the result into the input.  */
+     are the same, then create a temp and copy the result into the input.  */
+  copyfd = -1;
   if (output_filename == NULL
       || filename_cmp (input_filename, output_filename) == 0)
-    tmpname = make_tempname (input_filename, &tmpfd);
+    {
+      tmpname = make_tempname (input_filename, &tmpfd);
+      if (tmpfd >= 0)
+       copyfd = dup (tmpfd);
+    }
   else
     tmpname = output_filename;
 
@@ -5934,11 +5950,15 @@ copy_main (int argc, char *argv[])
   if (status == 0)
     {
       if (tmpname != output_filename)
-       status = (smart_rename (tmpname, input_filename,
-                               preserve_dates ? &statbuf : NULL) != 0);
+       status = smart_rename (tmpname, input_filename, copyfd,
+                              &statbuf, preserve_dates) != 0;
     }
   else
-    unlink_if_ordinary (tmpname);
+    {
+      if (copyfd >= 0)
+       close (copyfd);
+      unlink_if_ordinary (tmpname);
+    }
 
   if (tmpname != output_filename)
     free (tmpname);

diff --git a/binutils/rename.c b/binutils/rename.c
index 72a9323d72c44207d55609e023486373302a717c..f688f350d51f2a812d05557f31941f075e9678fd 100644 (file)
--- a/binutils/rename.c
+++ b/binutils/rename.c
@@ -31,24 +31,21 @@
 /* The number of bytes to copy at once.  */
 #define COPY_BUF 8192
 
-/* Copy file FROM to file TO, performing no translations.
+/* Copy file FROMFD to file TO, performing no translations.
    Return 0 if ok, -1 if error.  */
 
 static int
-simple_copy (const char *from, const char *to)
+simple_copy (int fromfd, const char *to, struct stat *target_stat)
 {
-  int fromfd, tofd, nread;
+  int tofd, nread;
   int saved;
   char buf[COPY_BUF];
 
-  fromfd = open (from, O_RDONLY | O_BINARY);
-  if (fromfd < 0)
+  if (fromfd < 0
+      || lseek (fromfd, 0, SEEK_SET) != 0)
     return -1;
-#ifdef O_CREAT
-  tofd = open (to, O_CREAT | O_WRONLY | O_TRUNC | O_BINARY, 0777);
-#else
-  tofd = creat (to, 0777);
-#endif
+
+  tofd = open (to, O_WRONLY | O_TRUNC | O_BINARY);
   if (tofd < 0)
     {
       saved = errno;
@@ -56,6 +53,7 @@ simple_copy (const char *from, const char *to)
       errno = saved;
       return -1;
     }
+
   while ((nread = read (fromfd, buf, sizeof buf)) > 0)
     {
       if (write (tofd, buf, nread) != nread)
@@ -67,7 +65,16 @@ simple_copy (const char *from, const char *to)
          return -1;
        }
     }
+
   saved = errno;
+
+#if !defined (_WIN32) || defined (__CYGWIN32__)
+  /* Writing to a setuid/setgid file may clear S_ISUID and S_ISGID.
+     Try to restore them, ignoring failure.  */
+  if (target_stat != NULL)
+    fchmod (tofd, target_stat->st_mode);
+#endif
+
   close (fromfd);
   close (tofd);
   if (nread < 0)
@@ -118,17 +125,17 @@ set_times (const char *destination, const struct stat *statbuf)
    various systems.  So now we just copy.  */
 
 int
-smart_rename (const char *from, const char *to,
-             struct stat *target_stat)
+smart_rename (const char *from, const char *to, int fromfd,
+             struct stat *target_stat, bfd_boolean preserve_dates)
 {
   int ret;
 
-  ret = simple_copy (from, to);
+  ret = simple_copy (fromfd, to, target_stat);
   if (ret != 0)
     non_fatal (_("unable to copy file '%s'; reason: %s"),
               to, strerror (errno));
 
-  if (target_stat != NULL)
+  if (preserve_dates)
     set_times (to, target_stat);
   unlink (from);