diff options
| -rw-r--r-- | binutils/ChangeLog | 18 | ||||
| -rw-r--r-- | binutils/ar.c | 12 | ||||
| -rw-r--r-- | binutils/arsup.c | 14 | ||||
| -rw-r--r-- | binutils/bucomm.h | 3 | ||||
| -rw-r--r-- | binutils/objcopy.c | 42 | ||||
| -rw-r--r-- | binutils/rename.c | 101 |
6 files changed, 148 insertions, 42 deletions
diff --git a/binutils/ChangeLog b/binutils/ChangeLog index a817cd8..5f6d98f 100644 --- a/binutils/ChangeLog +++ b/binutils/ChangeLog @@ -1,6 +1,24 @@ 2020-12-07 Siddhesh Poyarekar <siddhesh@sourceware.org> PR 26945 + * ar.c + (write_archive) [!defined (_WIN32) || defined (__CYGWIN32__)]: + Initialize TARGET_STAT and OFD to pass to SMART_RENAME. + * arsup.c + (ar_save) [defined (_WIN32) || defined (__CYGWIN32__)]: + Likewise. + * bucomm.h (smart_rename): Add new arguments to declaration. + * objcopy.c + (strip_main)[defined (_WIN32) || defined (__CYGWIN32__)]: + Initialize COPYFD and pass to SMART_RENAME. + (copy_main) [defined (_WIN32) || defined (__CYGWIN32__)]: + Likewise. + * rename.c (try_preserve_permissions): New function. + (smart_rename): Use it and add new arguments. + +2020-12-07 Siddhesh Poyarekar <siddhesh@sourceware.org> + + PR 26945 * objcopy.c (copy_file): New argument IN_STAT. Return stat of ibfd through it. (strip_main): Remove redundant stat calls. adjust copy_file diff --git a/binutils/ar.c b/binutils/ar.c index 2253242..6598dd9 100644 --- a/binutils/ar.c +++ b/binutils/ar.c @@ -1254,6 +1254,8 @@ write_archive (bfd *iarch) char *old_name, *new_name; bfd *contents_head = iarch->archive_next; int ofd = -1; + struct stat target_stat; + bfd_boolean skip_stat = FALSE; old_name = (char *) xmalloc (strlen (bfd_get_filename (iarch)) + 1); strcpy (old_name, bfd_get_filename (iarch)); @@ -1299,6 +1301,14 @@ write_archive (bfd *iarch) if (!bfd_set_archive_head (obfd, contents_head)) bfd_fatal (old_name); +#if !defined (_WIN32) || defined (__CYGWIN32__) + ofd = dup (ofd); + if (iarch == NULL || iarch->iostream == NULL) + skip_stat = TRUE; + else if (ofd == -1 || fstat (fileno (iarch->iostream), &target_stat) != 0) + bfd_fatal (old_name); +#endif + if (!bfd_close (obfd)) bfd_fatal (old_name); @@ -1308,7 +1318,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, 0) != 0) + if (smart_rename (new_name, old_name, ofd, skip_stat ? NULL : &target_stat, 0) != 0) xexit (1); free (old_name); free (new_name); diff --git a/binutils/arsup.c b/binutils/arsup.c index a668f27..8b4437f 100644 --- a/binutils/arsup.c +++ b/binutils/arsup.c @@ -345,13 +345,25 @@ ar_save (void) else { char *ofilename = xstrdup (bfd_get_filename (obfd)); + bfd_boolean skip_stat = FALSE; + struct stat target_stat; + int ofd = -1; if (deterministic > 0) obfd->flags |= BFD_DETERMINISTIC_OUTPUT; +#if !defined (_WIN32) || defined (__CYGWIN32__) + /* It's OK to fail; at worst it will result in SMART_RENAME using a slow + copy fallback to write the output. */ + ofd = dup (fileno (obfd->iostream)); + if (lstat (real_name, &target_stat) != 0) + skip_stat = TRUE; +#endif + bfd_close (obfd); - smart_rename (ofilename, real_name, 0); + smart_rename (ofilename, real_name, ofd, + skip_stat ? NULL : &target_stat, 0); obfd = 0; free (ofilename); } diff --git a/binutils/bucomm.h b/binutils/bucomm.h index afb8e09..9613b92 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 *, int); +extern int smart_rename (const char *, const char *, int, struct stat *, int); + /* In libiberty. */ void *xmalloc (size_t); diff --git a/binutils/objcopy.c b/binutils/objcopy.c index b6cf6ea..04ba95e 100644 --- a/binutils/objcopy.c +++ b/binutils/objcopy.c @@ -4815,6 +4815,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) { @@ -4828,7 +4829,12 @@ strip_main (int argc, char *argv[]) else tmpname = output_file; - if (tmpname == NULL) + if (tmpname == NULL +#if !defined (_WIN32) || defined (__CYGWIN32__) + /* Retain a copy of TMPFD since we will need it for SMART_RENAME. */ + || (tmpfd >= 0 && (copyfd = dup (tmpfd)) == -1) +#endif + ) { bfd_nonfatal_message (argv[i], NULL, NULL, _("could not create temporary file to hold stripped copy")); @@ -4846,12 +4852,18 @@ strip_main (int argc, char *argv[]) if (output_file != tmpname) status = (smart_rename (tmpname, output_file ? output_file : argv[i], - preserve_dates) != 0); + copyfd, &statbuf, preserve_dates) != 0); if (status == 0) status = hold_status; } else - unlink_if_ordinary (tmpname); + { +#if !defined (_WIN32) || defined (__CYGWIN32__) + if (copyfd >= 0) + close (copyfd); +#endif + unlink_if_ordinary (tmpname); + } if (output_file != tmpname) free (tmpname); } @@ -5059,6 +5071,7 @@ copy_main (int argc, char *argv[]) bfd_boolean use_globalize = FALSE; bfd_boolean use_keep_global = FALSE; int c, tmpfd = -1; + int copyfd = -1; struct stat statbuf; const bfd_arch_info_type *input_arch = NULL; @@ -5903,9 +5916,16 @@ copy_main (int argc, char *argv[]) else tmpname = output_filename; - if (tmpname == NULL) - fatal (_("warning: could not create temporary file whilst copying '%s', (error: %s)"), - input_filename, strerror (errno)); + if (tmpname == NULL +#if !defined (_WIN32) || defined (__CYGWIN32__) + /* Retain a copy of TMPFD since we will need it for SMART_RENAME. */ + || (tmpfd >= 0 && (copyfd = dup (tmpfd)) == -1) +#endif + ) + { + fatal (_("warning: could not create temporary file whilst copying '%s', (error: %s)"), + input_filename, strerror (errno)); + } copy_file (input_filename, tmpname, tmpfd, &statbuf, input_target, output_target, input_arch); @@ -5914,11 +5934,17 @@ copy_main (int argc, char *argv[]) if (preserve_dates) set_times (tmpname, &statbuf); if (tmpname != output_filename) - status = (smart_rename (tmpname, input_filename, + status = (smart_rename (tmpname, input_filename, copyfd, &statbuf, preserve_dates) != 0); } else - unlink_if_ordinary (tmpname); + { +#if !defined (_WIN32) || defined (__CYGWIN32__) + if (copyfd >= 0) + close (copyfd); +#endif + unlink_if_ordinary (tmpname); + } if (tmpname != output_filename) free (tmpname); diff --git a/binutils/rename.c b/binutils/rename.c index bf3b68d..6b9165e 100644 --- a/binutils/rename.c +++ b/binutils/rename.c @@ -131,17 +131,55 @@ set_times (const char *destination, const struct stat *statbuf) #endif #endif -/* Rename FROM to TO, copying if TO is a link. - Return 0 if ok, -1 if error. */ +#if !defined (_WIN32) || defined (__CYGWIN32__) +/* Try to preserve the permission bits and ownership of an existing file when + rename overwrites it. FD is the file being renamed and TARGET_STAT has the + status of the file that was overwritten. */ +static void +try_preserve_permissions (int fd, struct stat *target_stat) +{ + struct stat from_stat; + int ret = 0; + + if (fstat (fd, &from_stat) != 0) + return; + + int from_mode = from_stat.st_mode & 0777; + int to_mode = target_stat->st_mode & 0777; + + /* Fix up permissions before we potentially lose ownership with fchown. + Clear the setxid bits because in case the fchown below fails then we don't + want to end up with a sxid file owned by the invoking user. If the user + hasn't changed or if fchown succeeded, we add back the sxid bits at the + end. */ + if (from_mode != to_mode) + fchmod (fd, to_mode); + + /* Fix up ownership, this will clear the setxid bits. */ + if (from_stat.st_uid != target_stat->st_uid + || from_stat.st_gid != target_stat->st_gid) + ret = fchown (fd, target_stat->st_uid, target_stat->st_gid); + + /* Fix up the sxid bits if either the fchown wasn't needed or it + succeeded. */ + if (ret == 0) + fchmod (fd, target_stat->st_mode & 07777); +} +#endif + +/* Rename FROM to TO, copying if TO is either a link or is not a regular file. + FD is an open file descriptor pointing to FROM that we can use to safely fix + up permissions of the file after renaming. TARGET_STAT has the file status + that is used to fix up permissions and timestamps after rename. Return 0 if + ok, -1 if error and FD is closed before returning. */ int -smart_rename (const char *from, const char *to, int preserve_dates ATTRIBUTE_UNUSED) +smart_rename (const char *from, const char *to, int fd ATTRIBUTE_UNUSED, + struct stat *target_stat ATTRIBUTE_UNUSED, + int preserve_dates ATTRIBUTE_UNUSED) { - bfd_boolean exists; - struct stat s; int ret = 0; - - exists = lstat (to, &s) == 0; + bfd_boolean exists = target_stat != NULL; #if defined (_WIN32) && !defined (__CYGWIN32__) /* Win32, unlike unix, will not erase `to' in `rename(from, to)' but @@ -158,36 +196,35 @@ smart_rename (const char *from, const char *to, int preserve_dates ATTRIBUTE_UNU unlink (from); } #else - /* Use rename only if TO is not a symbolic link and has - only one hard link, and we have permission to write to it. */ + /* Avoid a full copy and use rename if we can fix up permissions of the + file after renaming, i.e.: + + - TO is not a symbolic link + - TO is a regular file with only one hard link + - We have permission to write to TO + - FD is available to safely fix up permissions to be the same as the file + we overwrote with the rename. + + Note though that the actual file on disk that TARGET_STAT describes may + have changed and we're only trying to preserve the status we know about. + At no point do we try to interact with the new file changes, so there can + only be two outcomes, i.e. either the external file change survives + without knowledge of our change (if it happens after the rename syscall) + or our rename and permissions fixup survive without any knowledge of the + external change. */ if (! exists - || (!S_ISLNK (s.st_mode) - && S_ISREG (s.st_mode) - && (s.st_mode & S_IWUSR) - && s.st_nlink == 1) + || (fd >= 0 + && !S_ISLNK (target_stat->st_mode) + && S_ISREG (target_stat->st_mode) + && (target_stat->st_mode & S_IWUSR) + && target_stat->st_nlink == 1) ) { ret = rename (from, to); if (ret == 0) { if (exists) - { - /* Try to preserve the permission bits and ownership of - TO. First get the mode right except for the setuid - bit. Then change the ownership. Then fix the setuid - bit. We do the chmod before the chown because if the - chown succeeds, and we are a normal user, we won't be - able to do the chmod afterward. We don't bother to - fix the setuid bit first because that might introduce - a fleeting security problem, and because the chown - will clear the setuid bit anyhow. We only fix the - setuid bit if the chown succeeds, because we don't - want to introduce an unexpected setuid file owned by - the user running objcopy. */ - chmod (to, s.st_mode & 0777); - if (chown (to, s.st_uid, s.st_gid) >= 0) - chmod (to, s.st_mode & 07777); - } + try_preserve_permissions (fd, target_stat); } else { @@ -203,9 +240,11 @@ smart_rename (const char *from, const char *to, int preserve_dates ATTRIBUTE_UNU non_fatal (_("unable to copy file '%s'; reason: %s"), to, strerror (errno)); if (preserve_dates) - set_times (to, &s); + set_times (to, target_stat); unlink (from); } + if (fd >= 0) + close (fd); #endif /* _WIN32 && !__CYGWIN32__ */ return ret; |