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
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
|
From 0d82f2f3ba4a856504f23a9db1dd5f37bd076337 Mon Sep 17 00:00:00 2001
From: Changqing Li <changqing.li@windriver.com>
Date: Mon, 23 Aug 2021 14:16:53 +0800
Subject: [PATCH] Fix close_on_exec multithreaded decompression issue.
Upstream-Status: Backport [https://github.com/file/file/commit/81f15c2b0d6e9eaf524ff7bab37426c21af75fb7]
Signed-off-by: Changqing Li <changqing.li@windriver.com>
---
ChangeLog | 5 +++++
configure.ac | 2 +-
src/compress.c | 25 ++++++++++++++++++++++---
src/file.h | 12 +++++++++++-
src/funcs.c | 24 +++++++++++++++++++++++-
src/magic.c | 7 +++++--
6 files changed, 67 insertions(+), 8 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index f877ad22..8c4a43d4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2020-12-08 16:24 Christos Zoulas <christos@zoulas.com>
+
+ * fix multithreaded decompression file descriptor issue
+ by using close-on-exec (Denys Vlasenko)
+
2020-06-14 20:02 Christos Zoulas <christos@zoulas.com>
* release 5.39
diff --git a/configure.ac b/configure.ac
index 64c9f42e..521dc12d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -166,7 +166,7 @@ else
fi])
dnl Checks for functions
-AC_CHECK_FUNCS(strndup mkstemp mkostemp utimes utime wcwidth strtof newlocale uselocale freelocale memmem)
+AC_CHECK_FUNCS(strndup mkstemp mkostemp utimes utime wcwidth strtof newlocale uselocale freelocale memmem pipe2)
dnl Provide implementation of some required functions if necessary
AC_REPLACE_FUNCS(getopt_long asprintf vasprintf strlcpy strlcat getline ctime_r asctime_r localtime_r gmtime_r pread strcasestr fmtcheck dprintf)
diff --git a/src/compress.c b/src/compress.c
index 9670b72c..9f65e4fa 100644
--- a/src/compress.c
+++ b/src/compress.c
@@ -35,7 +35,7 @@
#include "file.h"
#ifndef lint
-FILE_RCSID("@(#)$File: compress.c,v 1.127 2020/05/31 00:11:06 christos Exp $")
+FILE_RCSID("@(#)$File: compress.c,v 1.129 2020/12/08 21:26:00 christos Exp $")
#endif
#include "magic.h"
@@ -844,8 +844,23 @@ uncompressbuf(int fd, size_t bytes_max, size_t method, const unsigned char *old,
for (i = 0; i < __arraycount(fdp); i++)
fdp[i][0] = fdp[i][1] = -1;
- if ((fd == -1 && pipe(fdp[STDIN_FILENO]) == -1) ||
- pipe(fdp[STDOUT_FILENO]) == -1 || pipe(fdp[STDERR_FILENO]) == -1) {
+ /*
+ * There are multithreaded users who run magic_file()
+ * from dozens of threads. If two parallel magic_file() calls
+ * analyze two large compressed files, both will spawn
+ * an uncompressing child here, which writes out uncompressed data.
+ * We read some portion, then close the pipe, then waitpid() the child.
+ * If uncompressed data is larger, child shound get EPIPE and exit.
+ * However, with *parallel* calls OTHER child may unintentionally
+ * inherit pipe fds, thus keeping pipe open and making writes in
+ * our child block instead of failing with EPIPE!
+ * (For the bug to occur, two threads must mutually inherit their pipes,
+ * and both must have large outputs. Thus it happens not that often).
+ * To avoid this, be sure to create pipes with O_CLOEXEC.
+ */
+ if ((fd == -1 && file_pipe_closexec(fdp[STDIN_FILENO]) == -1) ||
+ file_pipe_closexec(fdp[STDOUT_FILENO]) == -1 ||
+ file_pipe_closexec(fdp[STDERR_FILENO]) == -1) {
closep(fdp[STDIN_FILENO]);
closep(fdp[STDOUT_FILENO]);
return makeerror(newch, n, "Cannot create pipe, %s",
@@ -876,16 +891,20 @@ uncompressbuf(int fd, size_t bytes_max, size_t method, const unsigned char *old,
if (fdp[STDIN_FILENO][1] > 2)
(void) close(fdp[STDIN_FILENO][1]);
}
+ file_clear_closexec(STDIN_FILENO);
+
///FIXME: if one of the fdp[i][j] is 0 or 1, this can bomb spectacularly
if (copydesc(STDOUT_FILENO, fdp[STDOUT_FILENO][1]))
(void) close(fdp[STDOUT_FILENO][1]);
if (fdp[STDOUT_FILENO][0] > 2)
(void) close(fdp[STDOUT_FILENO][0]);
+ file_clear_closexec(STDOUT_FILENO);
if (copydesc(STDERR_FILENO, fdp[STDERR_FILENO][1]))
(void) close(fdp[STDERR_FILENO][1]);
if (fdp[STDERR_FILENO][0] > 2)
(void) close(fdp[STDERR_FILENO][0]);
+ file_clear_closexec(STDERR_FILENO);
(void)execvp(compr[method].argv[0],
RCAST(char *const *, RCAST(intptr_t, compr[method].argv)));
diff --git a/src/file.h b/src/file.h
index 28ebc0c1..48f4b698 100644
--- a/src/file.h
+++ b/src/file.h
@@ -27,7 +27,7 @@
*/
/*
* file.h - definitions for file(1) program
- * @(#)$File: file.h,v 1.220 2020/06/08 17:38:27 christos Exp $
+ * @(#)$File: file.h,v 1.223 2020/12/08 21:26:00 christos Exp $
*/
#ifndef __file_h__
@@ -143,6 +143,14 @@
#define MAX(a,b) (((a) > (b)) ? (a) : (b))
#endif
+#ifndef O_CLOEXEC
+# define O_CLOEXEC 0
+#endif
+
+#ifndef FD_CLOEXEC
+# define FD_CLOEXEC 1
+#endif
+
#define FILE_BADSIZE CAST(size_t, ~0ul)
#define MAXDESC 64 /* max len of text description/MIME type */
#define MAXMIME 80 /* max len of text MIME type */
@@ -538,6 +546,8 @@ protected char * file_printable(char *, size_t, const char *, size_t);
protected int file_os2_apptype(struct magic_set *, const char *, const void *,
size_t);
#endif /* __EMX__ */
+protected int file_pipe_closexec(int *);
+protected int file_clear_closexec(int);
protected void buffer_init(struct buffer *, int, const struct stat *,
const void *, size_t);
diff --git a/src/funcs.c b/src/funcs.c
index b66510c6..d6c485fe 100644
--- a/src/funcs.c
+++ b/src/funcs.c
@@ -27,7 +27,7 @@
#include "file.h"
#ifndef lint
-FILE_RCSID("@(#)$File: funcs.c,v 1.115 2020/02/20 15:50:20 christos Exp $")
+FILE_RCSID("@(#)$File: funcs.c,v 1.118 2020/12/08 21:26:00 christos Exp $")
#endif /* lint */
#include "magic.h"
@@ -36,6 +36,9 @@ FILE_RCSID("@(#)$File: funcs.c,v 1.115 2020/02/20 15:50:20 christos Exp $")
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
+#ifdef HAVE_UNISTD_H
+#include <unistd.h> /* for pipe2() */
+#endif
#if defined(HAVE_WCHAR_H)
#include <wchar.h>
#endif
@@ -783,3 +786,22 @@ file_print_guid(char *str, size_t len, const uint64_t *guid)
g->data4[2], g->data4[3], g->data4[4], g->data4[5],
g->data4[6], g->data4[7]);
}
+
+protected int
+file_pipe_closexec(int *fds)
+{
+#ifdef HAVE_PIPE2
+ return pipe2(fds, O_CLOEXEC);
+#else
+ if (pipe(fds) == -1)
+ return -1;
+ (void)fcntl(fds[0], F_SETFD, FD_CLOEXEC);
+ (void)fcntl(fds[1], F_SETFD, FD_CLOEXEC);
+ return 0;
+#endif
+}
+
+protected int
+file_clear_closexec(int fd) {
+ return fcntl(fd, F_SETFD, 0);
+}
diff --git a/src/magic.c b/src/magic.c
index 17a7077d..89f4e16c 100644
--- a/src/magic.c
+++ b/src/magic.c
@@ -33,7 +33,7 @@
#include "file.h"
#ifndef lint
-FILE_RCSID("@(#)$File: magic.c,v 1.112 2020/06/08 19:44:10 christos Exp $")
+FILE_RCSID("@(#)$File: magic.c,v 1.113 2020/12/08 21:26:00 christos Exp $")
#endif /* lint */
#include "magic.h"
@@ -436,7 +436,7 @@ file_or_fd(struct magic_set *ms, const char *inname, int fd)
_setmode(STDIN_FILENO, O_BINARY);
#endif
if (inname != NULL) {
- int flags = O_RDONLY|O_BINARY|O_NONBLOCK;
+ int flags = O_RDONLY|O_BINARY|O_NONBLOCK|O_CLOEXEC;
errno = 0;
if ((fd = open(inname, flags)) < 0) {
okstat = stat(inname, &sb) == 0;
@@ -460,6 +460,9 @@ file_or_fd(struct magic_set *ms, const char *inname, int fd)
rv = 0;
goto done;
}
+#if O_CLOEXEC == 0
+ (void)fcntl(fd, F_SETFD, FD_CLOEXEC);
+#endif
}
if (fd != -1) {
--
2.17.1
|
