1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
|
From 8e03235147a9e774d3ba084e93c2da1aa94d1cec Mon Sep 17 00:00:00 2001
From: Siddhesh Poyarekar <siddhesh@gotplt.org>
Date: Mon, 22 Feb 2021 20:45:50 +0530
Subject: [PATCH] binutils: Avoid renaming over existing files
Renaming over existing files needs additional care to restore
permissions and ownership, which may not always succeed.
Additionally, other properties of the file such as extended attributes
may be lost, making the operation flaky.
For predictable results, resort to rename() only if the file does not
exist, otherwise copy the file contents into the existing file. This
ensures that no additional tricks are needed to retain file
properties.
This also allows dropping of the redundant set_times on the tmpfile in
objcopy/strip since now we no longer rename over existing files.
binutils/
* ar.c (write_archive): Adjust call to SMART_RENAME.
* arsup.c (ar_save): Likewise.
* objcopy (strip_main): Don't set times on temporary file and
adjust call to SMART_RENAME.
(copy_main): Likewise.
* rename.c [!S_ISLNK]: Remove definitions.
(try_preserve_permissions): Remove function.
(smart_rename): Replace PRESERVE_DATES argument with
TARGET_STAT. Use rename system call only if TO does not exist.
* bucomm.h (smart_rename): Adjust declaration.
(cherry picked from commit 3685de750e6a091663a0abe42528cad29e960e35)
Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8e03235147a9e774d3ba084e93c2da1aa94d1cec]
CVE: CVE-2021-20197
Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
---
binutils/ar.c | 2 +-
binutils/arsup.c | 2 +-
binutils/bucomm.h | 3 ++-
binutils/objcopy.c | 8 ++-----
binutils/rename.c | 55 +++++++++-------------------------------------
6 files changed, 29 insertions(+), 54 deletions(-)
diff --git a/binutils/ar.c b/binutils/ar.c
index 45a34e3a6cf..3a91708b51c 100644
--- a/binutils/ar.c
+++ b/binutils/ar.c
@@ -1308,7 +1308,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, NULL) != 0)
xexit (1);
free (old_name);
free (new_name);
diff --git a/binutils/arsup.c b/binutils/arsup.c
index 5403a0c5d74..0a1f63f6456 100644
--- a/binutils/arsup.c
+++ b/binutils/arsup.c
@@ -351,7 +351,7 @@ ar_save (void)
bfd_close (obfd);
- smart_rename (ofilename, real_name, 0);
+ smart_rename (ofilename, real_name, NULL);
obfd = 0;
free (ofilename);
}
diff --git a/binutils/bucomm.h b/binutils/bucomm.h
index 91f6a5b228f..aa7e33d8cd1 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 *, struct stat *);
+
/* In libiberty. */
void *xmalloc (size_t);
diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index eab3b6db585..07a872b5a80 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -4861,12 +4861,10 @@ strip_main (int argc, char *argv[])
output_target, NULL);
if (status == 0)
{
- if (preserve_dates)
- set_times (tmpname, &statbuf);
if (output_file != tmpname)
status = (smart_rename (tmpname,
output_file ? output_file : argv[i],
- preserve_dates) != 0);
+ preserve_dates ? &statbuf : NULL) != 0);
if (status == 0)
status = hold_status;
}
@@ -5931,11 +5929,9 @@ copy_main (int argc, char *argv[])
output_target, input_arch);
if (status == 0)
{
- if (preserve_dates)
- set_times (tmpname, &statbuf);
if (tmpname != output_filename)
status = (smart_rename (tmpname, input_filename,
- preserve_dates) != 0);
+ preserve_dates ? &statbuf : NULL) != 0);
}
else
unlink_if_ordinary (tmpname);
diff --git a/binutils/rename.c b/binutils/rename.c
index 65ad5bf52c4..f471b45fd3f 100644
--- a/binutils/rename.c
+++ b/binutils/rename.c
@@ -122,20 +122,13 @@ set_times (const char *destination, const struct stat *statbuf)
non_fatal (_("%s: cannot set time: %s"), destination, strerror (errno));
}
-#ifndef S_ISLNK
-#ifdef S_IFLNK
-#define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK)
-#else
-#define S_ISLNK(m) 0
-#define lstat stat
-#endif
-#endif
-
-/* Rename FROM to TO, copying if TO is a link.
- Return 0 if ok, -1 if error. */
+/* Rename FROM to TO, copying if TO exists. TARGET_STAT has the file status
+ that, if non-NULL, is used to fix up timestamps after rename. Return 0 if
+ ok, -1 if error. */
int
-smart_rename (const char *from, const char *to, int preserve_dates ATTRIBUTE_UNUSED)
+smart_rename (const char *from, const char *to,
+ struct stat *target_stat ATTRIBUTE_UNUSED)
{
bfd_boolean exists;
struct stat s;
@@ -158,38 +151,10 @@ 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. */
- if (! exists
- || (!S_ISLNK (s.st_mode)
- && S_ISREG (s.st_mode)
- && (s.st_mode & S_IWUSR)
- && s.st_nlink == 1)
- )
+ /* Avoid a full copy and use rename if TO does not exist. */
+ if (!exists)
{
- 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);
- }
- }
- else
+ if ((ret = rename (from, to)) != 0)
{
/* We have to clean up here. */
non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
@@ -202,8 +167,8 @@ smart_rename (const char *from, const char *to, int preserve_dates ATTRIBUTE_UNU
if (ret != 0)
non_fatal (_("unable to copy file '%s'; reason: %s"), to, strerror (errno));
- if (preserve_dates)
- set_times (to, &s);
+ if (target_stat != NULL)
+ set_times (to, target_stat);
unlink (from);
}
#endif /* _WIN32 && !__CYGWIN32__ */
--
2.31.1
|
