Skip to content

Commit c85cb3b

Browse files
poetteringpoettering
committed
Revert "basic/fd-util: sort the 'except' array in place"
This reverts commit 9c46228.
1 parent e7e7c07 commit c85cb3b

File tree

6 files changed

+91
-66
lines changed

6 files changed

+91
-66
lines changed

‎src/basic/fd-util.c‎

Lines changed: 83 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,10 @@ static int get_max_fd(void) {
208208
return (int) (m - 1);
209209
}
210210

211-
int close_all_fds(int except[], size_t n_except) {
211+
int close_all_fds(const int except[], size_t n_except) {
212212
static bool have_close_range = true; /* Assume we live in the future */
213213
_cleanup_closedir_ DIR *d = NULL;
214+
struct dirent *de;
214215
int r = 0;
215216

216217
assert(n_except == 0 || except);
@@ -226,104 +227,129 @@ int close_all_fds(int except[], size_t n_except) {
226227
/* Close everything. Yay! */
227228

228229
if (close_range(3, -1, 0) >= 0)
229-
return 0;
230+
return 1;
230231

231-
if (ERRNO_IS_NOT_SUPPORTED(errno) || ERRNO_IS_PRIVILEGE(errno))
232-
have_close_range = false;
233-
else
232+
if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno))
234233
return -errno;
235234

235+
have_close_range = false;
236236
} else {
237-
typesafe_qsort(except, n_except, cmp_int);
237+
_cleanup_free_ int *sorted_malloc = NULL;
238+
size_t n_sorted;
239+
int *sorted;
240+
241+
assert(n_except < SIZE_MAX);
242+
n_sorted = n_except + 1;
243+
244+
if (n_sorted > 64) /* Use heap for large numbers of fds, stack otherwise */
245+
sorted = sorted_malloc = new(int, n_sorted);
246+
else
247+
sorted = newa(int, n_sorted);
248+
249+
if (sorted) {
250+
int c = 0;
251+
252+
memcpy(sorted, except, n_except * sizeof(int));
253+
254+
/* Let's add fd 2 to the list of fds, to simplify the loop below, as this
255+
* allows us to cover the head of the array the same way as the body */
256+
sorted[n_sorted-1] = 2;
238257

239-
for (size_t i = 0; i < n_except; i++) {
240-
int start = i == 0 ? 2 : MAX(except[i-1], 2); /* The first three fds shall always remain open */
241-
int end = MAX(except[i], 2);
258+
typesafe_qsort(sorted, n_sorted, cmp_int);
242259

243-
assert(end >= start);
260+
for (size_t i = 0; i < n_sorted-1; i++) {
261+
int start, end;
244262

245-
if (end - start <= 1)
246-
continue;
263+
start = MAX(sorted[i], 2); /* The first three fds shall always remain open */
264+
end = MAX(sorted[i+1], 2);
265+
266+
assert(end >= start);
267+
268+
if (end - start <= 1)
269+
continue;
270+
271+
/* Close everything between the start and end fds (both of which shall stay open) */
272+
if (close_range(start + 1, end - 1, 0) < 0) {
273+
if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno))
274+
return -errno;
247275

248-
/* Close everything between the start and end fds (both of which shall stay open) */
249-
if (close_range(start + 1, end - 1, 0) < 0) {
250-
if (ERRNO_IS_NOT_SUPPORTED(errno) || ERRNO_IS_PRIVILEGE(errno))
251276
have_close_range = false;
252-
else
253-
return -errno;
254-
goto opendir_fallback;
277+
break;
278+
}
279+
280+
c += end - start - 1;
255281
}
256-
}
257282

258-
/* The loop succeeded. Let's now close everything beyond the end */
283+
if (have_close_range) {
284+
/* The loop succeeded. Let's now close everything beyond the end */
259285

260-
if (except[n_except-1] >= INT_MAX) /* Don't let the addition below overflow */
261-
return 0;
286+
if (sorted[n_sorted-1] >= INT_MAX) /* Dont let the addition below overflow */
287+
return c;
262288

263-
int start = MAX(except[n_except-1], 2);
289+
if (close_range(sorted[n_sorted-1] + 1, -1, 0) >= 0)
290+
return c + 1;
264291

265-
if (close_range(start + 1, -1, 0) >= 0)
266-
return 0;
292+
if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno))
293+
return -errno;
267294

268-
if (ERRNO_IS_NOT_SUPPORTED(errno) || ERRNO_IS_PRIVILEGE(errno))
269-
have_close_range = false;
270-
else
271-
return -errno;
295+
have_close_range = false;
296+
}
297+
}
272298
}
299+
300+
/* Fallback on OOM or if close_range() is not supported */
273301
}
274302

275-
/* Fallback for when close_range() is not supported */
276-
opendir_fallback:
277303
d = opendir("/proc/self/fd");
278-
if (d) {
279-
struct dirent *de;
304+
if (!d) {
305+
int fd, max_fd;
280306

281-
FOREACH_DIRENT(de, d, return -errno) {
282-
int fd = -1, q;
307+
/* When /proc isn't available (for example in chroots) the fallback is brute forcing through
308+
* the fd table */
283309

284-
if (safe_atoi(de->d_name, &fd) < 0)
285-
/* Let's better ignore this, just in case */
286-
continue;
310+
max_fd = get_max_fd();
311+
if (max_fd < 0)
312+
return max_fd;
287313

288-
if (fd < 3)
289-
continue;
314+
/* Refuse to do the loop over more too many elements. It's better to fail immediately than to
315+
* spin the CPU for a long time. */
316+
if (max_fd > MAX_FD_LOOP_LIMIT)
317+
return log_debug_errno(SYNTHETIC_ERRNO(EPERM),
318+
"/proc/self/fd is inaccessible. Refusing to loop over %d potential fds.",
319+
max_fd);
290320

291-
if (fd == dirfd(d))
292-
continue;
321+
for (fd = 3; fd >= 0; fd = fd < max_fd ? fd + 1 : -1) {
322+
int q;
293323

294324
if (fd_in_set(fd, except, n_except))
295325
continue;
296326

297327
q = close_nointr(fd);
298-
if (q < 0 && q != -EBADF && r >= 0) /* Valgrind has its own FD and doesn't want to have it closed */
328+
if (q < 0 && q != -EBADF && r >= 0)
299329
r = q;
300330
}
301331

302332
return r;
303333
}
304334

305-
/* Fallback for when /proc isn't available (for example in chroots) by brute-forcing through the file
306-
* descriptor table. */
335+
FOREACH_DIRENT(de, d, return -errno) {
336+
int fd = -1, q;
307337

308-
int max_fd = get_max_fd();
309-
if (max_fd < 0)
310-
return max_fd;
338+
if (safe_atoi(de->d_name, &fd) < 0)
339+
/* Let's better ignore this, just in case */
340+
continue;
311341

312-
/* Refuse to do the loop over more too many elements. It's better to fail immediately than to
313-
* spin the CPU for a long time. */
314-
if (max_fd > MAX_FD_LOOP_LIMIT)
315-
return log_debug_errno(SYNTHETIC_ERRNO(EPERM),
316-
"/proc/self/fd is inaccessible. Refusing to loop over %d potential fds.",
317-
max_fd);
342+
if (fd < 3)
343+
continue;
318344

319-
for (int fd = 3; fd >= 0; fd = fd < max_fd ? fd + 1 : -1) {
320-
int q;
345+
if (fd == dirfd(d))
346+
continue;
321347

322348
if (fd_in_set(fd, except, n_except))
323349
continue;
324350

325351
q = close_nointr(fd);
326-
if (q < 0 && q != -EBADF && r >= 0)
352+
if (q < 0 && q != -EBADF && r >= 0) /* Valgrind has its own FD and doesn't want to have it closed */
327353
r = q;
328354
}
329355

‎src/basic/fd-util.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(DIR*, closedir, NULL);
5757
int fd_nonblock(int fd, bool nonblock);
5858
int fd_cloexec(int fd, bool cloexec);
5959

60-
int close_all_fds(int except[], size_t n_except);
60+
int close_all_fds(const int except[], size_t n_except);
6161

6262
int same_fd(int a, int b);
6363

‎src/basic/process-util.c‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1246,7 +1246,7 @@ static void restore_sigsetp(sigset_t **ssp) {
12461246

12471247
int safe_fork_full(
12481248
const char *name,
1249-
int except_fds[],
1249+
const int except_fds[],
12501250
size_t n_except_fds,
12511251
ForkFlags flags,
12521252
pid_t *ret_pid) {
@@ -1441,7 +1441,7 @@ int safe_fork_full(
14411441
int namespace_fork(
14421442
const char *outer_name,
14431443
const char *inner_name,
1444-
int except_fds[],
1444+
const int except_fds[],
14451445
size_t n_except_fds,
14461446
ForkFlags flags,
14471447
int pidns_fd,
@@ -1457,8 +1457,7 @@ int namespace_fork(
14571457
* process. This ensures that we are fully a member of the destination namespace, with pidns an all, so that
14581458
* /proc/self/fd works correctly. */
14591459

1460-
r = safe_fork_full(outer_name, except_fds, n_except_fds,
1461-
(flags|FORK_DEATHSIG) & ~(FORK_REOPEN_LOG|FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE), ret_pid);
1460+
r = safe_fork_full(outer_name, except_fds, n_except_fds, (flags|FORK_DEATHSIG) & ~(FORK_REOPEN_LOG|FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE), ret_pid);
14621461
if (r < 0)
14631462
return r;
14641463
if (r == 0) {

‎src/basic/process-util.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,13 +166,13 @@ typedef enum ForkFlags {
166166
FORK_NEW_USERNS = 1 << 13, /* Run child in its own user namespace */
167167
} ForkFlags;
168168

169-
int safe_fork_full(const char *name, int except_fds[], size_t n_except_fds, ForkFlags flags, pid_t *ret_pid);
169+
int safe_fork_full(const char *name, const int except_fds[], size_t n_except_fds, ForkFlags flags, pid_t *ret_pid);
170170

171171
static inline int safe_fork(const char *name, ForkFlags flags, pid_t *ret_pid) {
172172
return safe_fork_full(name, NULL, 0, flags, ret_pid);
173173
}
174174

175-
int namespace_fork(const char *outer_name, const char *inner_name, int except_fds[], size_t n_except_fds, ForkFlags flags, int pidns_fd, int mntns_fd, int netns_fd, int userns_fd, int root_fd, pid_t *ret_pid);
175+
int namespace_fork(const char *outer_name, const char *inner_name, const int except_fds[], size_t n_except_fds, ForkFlags flags, int pidns_fd, int mntns_fd, int netns_fd, int userns_fd, int root_fd, pid_t *ret_pid);
176176

177177
int set_oom_score_adjust(int value);
178178
int get_oom_score_adjust(int *ret);

‎src/shared/exec-util.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ int fexecve_or_execve(int executable_fd, const char *executable, char *const arg
471471
return -errno;
472472
}
473473

474-
int fork_agent(const char *name, int except[], size_t n_except, pid_t *ret_pid, const char *path, ...) {
474+
int fork_agent(const char *name, const int except[], size_t n_except, pid_t *ret_pid, const char *path, ...) {
475475
bool stdout_is_tty, stderr_is_tty;
476476
size_t n, i;
477477
va_list ap;

‎src/shared/exec-util.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,4 @@ ExecCommandFlags exec_command_flags_from_string(const char *s);
4949

5050
int fexecve_or_execve(int executable_fd, const char *executable, char *const argv[], char *const envp[]);
5151

52-
int fork_agent(const char *name, int except[], size_t n_except, pid_t *ret_pid, const char *path, ...) _sentinel_;
52+
int fork_agent(const char *name, const int except[], size_t n_except, pid_t *ret_pid, const char *path, ...) _sentinel_;

0 commit comments

Comments
 (0)