binutils: Use file descriptors from make_tempname

The purpose of creating a temporary file securely using mkstemp is
defeated if it is closed in make_tempname and reopened later for use;
it is as good as using mktemp.  Get the file descriptor instead and
then use it to create the BFD object.

bfd/

	* opncls.c (bfd_fdopenw): New function.
	* bfd-in2.h: Regenerate.

binutils/

	* bucomm.c (make_tempname): Add argument to return file
	descriptor.
	* bucomm.h (make_tempname): Likewise.
	* ar.c: Include libbfd.h.
	(write_archive): Adjust for change in make_tempname.  Call
	bfd_fdopenw instead of bfd_openw.
	* objcopy.c: Include libbfd.h.
	(copy_file): New argument OFD.  Use bfd_fdopenw instead of
	bfd_openw.
	(strip_main): Adjust for change in make_tempname and
	copy_file.
	(copy_main): Likewise.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 8a32d5c0102909bebadc3601003b423f6ab6571e..99c275ce4f1b5c127ac97c81f8818715930b79ac 100644 (file)
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,9 @@
+2020-12-07  Siddhesh Poyarekar  <siddhesh@sourceware.org>
+
+       PR 26945
+       * opncls.c (bfd_fdopenw): New function.
+       * bfd-in2.h: Regenerate.
+
 2020-12-07  Alan Modra  <amodra@gmail.com>
 
        * elf32-csky.c (csky_relocate_contents): Correct negate test.

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 935ba535b540d94b80496beb1e9e53faf833df53..48e3d9b6647c0e9d8854e3018db2ddc75b028d66 100644 (file)
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -588,6 +588,8 @@ bfd *bfd_openr (const char *filename, const char *target);
 
 bfd *bfd_fdopenr (const char *filename, const char *target, int fd);
 
+bfd *bfd_fdopenw (const char *filename, const char *target, int fd);
+
 bfd *bfd_openstreamr (const char * filename, const char * target,
     void * stream);
 

diff --git a/bfd/opncls.c b/bfd/opncls.c
index c2a1d2fa4df13d649ac6a0c7545766105d800085..f7696b658ca33f3b025c0722c14a2f6a1b9ae02c 100644 (file)
--- a/bfd/opncls.c
+++ b/bfd/opncls.c
@@ -393,6 +393,39 @@ bfd_fdopenr (const char *filename, const char *target, int fd)
   return bfd_fopen (filename, target, mode, fd);
 }
 
+/*
+FUNCTION
+       bfd_fdopenw
+
+SYNOPSIS
+       bfd *bfd_fdopenw (const char *filename, const char *target, int fd);
+
+DESCRIPTION
+       <<bfd_fdopenw>> is exactly like <<bfd_fdopenr>> with the exception that
+       the resulting BFD is suitable for output.
+*/
+
+bfd *
+bfd_fdopenw (const char *filename, const char *target, int fd)
+{
+  bfd *out = bfd_fdopenr (filename, target, fd);
+
+  if (out != NULL)
+    {
+      if (!bfd_write_p (out))
+       {
+         close (fd);
+         _bfd_delete_bfd (out);
+         out = NULL;
+         bfd_set_error (bfd_error_invalid_operation);
+       }
+      else
+       out->direction = write_direction;
+    }
+
+  return out;
+}
+
 /*
 FUNCTION
        bfd_openstreamr

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 26aac38f7c647cf15726254e99640e75b30c8481..e0794e4302cb7624db7b90ddac8a16f31ed4f3f2 100644 (file)
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,20 @@
+2020-12-07  Siddhesh Poyarekar  <siddhesh@sourceware.org>
+
+       PR 26945
+       * bucomm.c (make_tempname): Add argument to return file
+       descriptor.
+       * bucomm.h (make_tempname): Likewise.
+       * ar.c: Include libbfd.h.
+       (write_archive): Adjust for change in make_tempname.  Call
+       bfd_fdopenw instead of bfd_openw.
+       * objcopy.c: Include libbfd.h.
+       (copy_file): New argument OFD.  Use bfd_fdopenw instead of
+       bfd_openw.
+       (strip_main): Adjust for change in make_tempname and
+       copy_file.
+       (copy_main): Likewise.
+
+
 2020-12-07  Nick Clifton  <nickc@redhat.com>
 
        * README-how-to-make-a-release (point releases): Add a note to

diff --git a/binutils/ar.c b/binutils/ar.c
index 7d279d6722a42a4a4fd0976adbbcc8fb8c4aefa8..225324208bd6e59e2de0f1e4d3a65ef89d44811d 100644 (file)
--- a/binutils/ar.c
+++ b/binutils/ar.c
@@ -25,6 +25,7 @@
 
 #include "sysdep.h"
 #include "bfd.h"
+#include "libbfd.h"
 #include "libiberty.h"
 #include "progress.h"
 #include "getopt.h"
@@ -1252,20 +1253,24 @@ write_archive (bfd *iarch)
   bfd *obfd;
   char *old_name, *new_name;
   bfd *contents_head = iarch->archive_next;
+  int ofd = -1;
 
   old_name = (char *) xmalloc (strlen (bfd_get_filename (iarch)) + 1);
   strcpy (old_name, bfd_get_filename (iarch));
-  new_name = make_tempname (old_name);
+  new_name = make_tempname (old_name, &ofd);
 
   if (new_name == NULL)
     bfd_fatal (_("could not create temporary file whilst writing archive"));
 
   output_filename = new_name;
 
-  obfd = bfd_openw (new_name, bfd_get_target (iarch));
+  obfd = bfd_fdopenw (new_name, bfd_get_target (iarch), ofd);
 
   if (obfd == NULL)
-    bfd_fatal (old_name);
+    {
+      close (ofd);
+      bfd_fatal (old_name);
+    }
 
   output_bfd = obfd;
 

diff --git a/binutils/bucomm.c b/binutils/bucomm.c
index 9e6a02843e6c7ba11545f69a6a31e927d177a9b0..53244201f89b2c649a8848a36fafbe0764db74bd 100644 (file)
--- a/binutils/bucomm.c
+++ b/binutils/bucomm.c
@@ -532,7 +532,7 @@ template_in_dir (const char *path)
    as FILENAME.  */
 
 char *
-make_tempname (const char *filename)
+make_tempname (const char *filename, int *ofd)
 {
   char *tmpname = template_in_dir (filename);
   int fd;
@@ -550,7 +550,7 @@ make_tempname (const char *filename)
       free (tmpname);
       return NULL;
     }
-  close (fd);
+  *ofd = fd;
   return tmpname;
 }
 

diff --git a/binutils/bucomm.h b/binutils/bucomm.h
index d8318343f787f091be52a9034a239dae70e407fa..afb8e09c2fd9aed02d1c240952fa30313a660019 100644 (file)
--- a/binutils/bucomm.h
+++ b/binutils/bucomm.h
@@ -51,7 +51,7 @@ int display_info (void);
 
 void print_arelt_descr (FILE *, bfd *, bfd_boolean, bfd_boolean);
 
-char *make_tempname (const char *);
+char *make_tempname (const char *, int *);
 char *make_tempdir (const char *);
 
 bfd_vma parse_vma (const char *, const char *);

diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index ca35df03b66f360ea750d0e23e13034b599e98bc..2eb083c37696243f8e8c5db368664f91ddbf216d 100644 (file)
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -20,6 +20,7 @@
 \f
 #include "sysdep.h"
 #include "bfd.h"
+#include "libbfd.h"
 #include "progress.h"
 #include "getopt.h"
 #include "libiberty.h"
@@ -3727,7 +3728,7 @@ set_long_section_mode (bfd *output_bfd, bfd *input_bfd, enum long_section_name_h
 /* The top-level control.  */
 
 static void
-copy_file (const char *input_filename, const char *output_filename,
+copy_file (const char *input_filename, const char *output_filename, int ofd,
           const char *input_target,   const char *output_target,
           const bfd_arch_info_type *input_arch)
 {
@@ -3802,9 +3803,14 @@ copy_file (const char *input_filename, const char *output_filename,
       else
        force_output_target = TRUE;
 
-      obfd = bfd_openw (output_filename, output_target);
+      if (ofd >= 0)
+       obfd = bfd_fdopenw (output_filename, output_target, ofd);
+      else
+       obfd = bfd_openw (output_filename, output_target);
+
       if (obfd == NULL)
        {
+         close (ofd);
          bfd_nonfatal_message (output_filename, NULL, NULL, NULL);
          status = 1;
          return;
@@ -3832,13 +3838,19 @@ copy_file (const char *input_filename, const char *output_filename,
       if (output_target == NULL)
        output_target = bfd_get_target (ibfd);
 
-      obfd = bfd_openw (output_filename, output_target);
+      if (ofd >= 0)
+       obfd = bfd_fdopenw (output_filename, output_target, ofd);
+      else
+       obfd = bfd_openw (output_filename, output_target);
+
       if (obfd == NULL)
        {
+         close (ofd);
          bfd_nonfatal_message (output_filename, NULL, NULL, NULL);
          status = 1;
          return;
        }
+
       /* This is a no-op on non-Coff targets.  */
       set_long_section_mode (obfd, ibfd, long_section_names);
 
@@ -4802,6 +4814,7 @@ strip_main (int argc, char *argv[])
       int hold_status = status;
       struct stat statbuf;
       char *tmpname;
+      int tmpfd = -1;
 
       if (get_file_size (argv[i]) < 1)
        {
@@ -4816,7 +4829,7 @@ strip_main (int argc, char *argv[])
 
       if (output_file == NULL
          || filename_cmp (argv[i], output_file) == 0)
-       tmpname = make_tempname (argv[i]);
+       tmpname = make_tempname (argv[i], &tmpfd);
       else
        tmpname = output_file;
 
@@ -4829,7 +4842,7 @@ strip_main (int argc, char *argv[])
        }
 
       status = 0;
-      copy_file (argv[i], tmpname, input_target, output_target, NULL);
+      copy_file (argv[i], tmpname, tmpfd, input_target, output_target, NULL);
       if (status == 0)
        {
          if (preserve_dates)
@@ -5049,7 +5062,7 @@ copy_main (int argc, char *argv[])
   bfd_boolean formats_info = FALSE;
   bfd_boolean use_globalize = FALSE;
   bfd_boolean use_keep_global = FALSE;
-  int c;
+  int c, tmpfd = -1;
   struct stat statbuf;
   const bfd_arch_info_type *input_arch = NULL;
 
@@ -5895,7 +5908,7 @@ copy_main (int argc, char *argv[])
      are the same, then create a temp and rename the result into the input.  */
   if (output_filename == NULL
       || filename_cmp (input_filename, output_filename) == 0)
-    tmpname = make_tempname (input_filename);
+    tmpname = make_tempname (input_filename, &tmpfd);
   else
     tmpname = output_filename;
 
@@ -5903,7 +5916,8 @@ copy_main (int argc, char *argv[])
     fatal (_("warning: could not create temporary file whilst copying '%s', (error: %s)"),
           input_filename, strerror (errno));
 
-  copy_file (input_filename, tmpname, input_target, output_target, input_arch);
+  copy_file (input_filename, tmpname, tmpfd, input_target, output_target,
+            input_arch);
   if (status == 0)
     {
       if (preserve_dates)