From e44e07a6bff0dd21a3fb08f28cd161e03360328b Mon Sep 17 00:00:00 2001 From: Tavian Barnes Date: Fri, 10 Nov 2023 10:08:26 -0500 Subject: exec: Don't do setrlimit() in the parent This was hacky, but it's also broken because it can make posix_spawn() fail with EMFILE, as happens on at least musl. Fixes: 7d69fef6a0b80ad57acde7bac8a22b83531fec0d Link: https://www.austingroupbugs.net/view.php?id=603 Link: https://sourceware.org/bugzilla/show_bug.cgi?id=31049 --- src/exec.c | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/src/exec.c b/src/exec.c index ba82439..87250ac 100644 --- a/src/exec.c +++ b/src/exec.c @@ -356,8 +356,6 @@ static int bfs_exec_spawn(const struct bfs_exec *execbuf) { } pid_t pid = -1; - int error; - bool reset_nofile = false; struct bfs_spawn spawn; if (bfs_spawn_init(&spawn) != 0) { @@ -376,31 +374,15 @@ static int bfs_exec_spawn(const struct bfs_exec *execbuf) { // Reset RLIMIT_NOFILE if necessary, to avoid breaking applications that use select() if (rlim_cmp(ctx->orig_nofile.rlim_cur, ctx->cur_nofile.rlim_cur) < 0) { - // posix_spawn() doesn't have a setrlimit() action, so adding one would force us - // to use the slower fork()/exec() path. Instead, drop the rlimit temporarily in - // the parent. This can race with other threads, but we always recover from - // EMFILE in the main thread anyway. - if (spawn.flags & BFS_SPAWN_USE_POSIX) { - if (setrlimit(RLIMIT_NOFILE, &ctx->orig_nofile) != 0) { - goto fail; - } - reset_nofile = true; - } else { - if (bfs_spawn_addsetrlimit(&spawn, RLIMIT_NOFILE, &ctx->orig_nofile) != 0) { - goto fail; - } + if (bfs_spawn_addsetrlimit(&spawn, RLIMIT_NOFILE, &ctx->orig_nofile) != 0) { + goto fail; } } pid = bfs_spawn(execbuf->argv[0], &spawn, execbuf->argv, NULL); -fail: - error = errno; - if (reset_nofile) { - if (setrlimit(RLIMIT_NOFILE, &ctx->cur_nofile) != 0) { - bfs_bug("setrlimit(RLIMIT_NOFILE): %s", xstrerror(errno)); - } - } +fail:; + int error = errno; bfs_spawn_destroy(&spawn); if (pid < 0) { -- cgit v1.2.3