From c42c71a1527dd70417d3966dce7ba9edbcf4bdb4 Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Tue, 23 Feb 2021 12:10:58 +1030 Subject: 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. --- binutils/ChangeLog | 18 ++++++++++++++++++ binutils/ar.c | 11 ++++++----- binutils/arsup.c | 9 +++++---- binutils/bucomm.h | 3 ++- binutils/objcopy.c | 42 +++++++++++++++++++++++++++++++----------- binutils/rename.c | 35 +++++++++++++++++++++-------------- 6 files changed, 83 insertions(+), 35 deletions(-) (limited to 'binutils') diff --git a/binutils/ChangeLog b/binutils/ChangeLog index 80bdf92..42b6954 100644 --- a/binutils/ChangeLog +++ b/binutils/ChangeLog @@ -1,4 +1,22 @@ 2021-02-24 Alan Modra + Siddhesh Poyarekar + + 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 PR 27456 * rename.c: Tidy throughout. diff --git a/binutils/ar.c b/binutils/ar.c index 44df48c..fb19b14 100644 --- 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 f7ce8f0..9982484 100644 --- 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 aa7e33d..f1ae47f 100644 --- 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 abbcb7c..90ae0bd 100644 --- 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 72a9323..f688f35 100644 --- 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); -- cgit v1.1