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

1 files 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) {