From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Josselin Poiret via "Bug reports for GUILE, GNU's Ubiquitous Extension Language" Newsgroups: gmane.lisp.guile.bugs Subject: bug#52835: [PATCH v4 1/4] Fix child spawning closing standard fds prematurely. Date: Sat, 28 May 2022 14:46:31 +0200 Message-ID: <20220528124634.17353-2-dev@jpoiret.xyz> References: <20220207165543.12723-1-dev@jpoiret.xyz> <20220528124634.17353-1-dev@jpoiret.xyz> Reply-To: Josselin Poiret Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="27038"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 52835@debbugs.gnu.org To: Josselin Poiret , Timothy Sample Original-X-From: bug-guile-bounces+guile-bugs=m.gmane-mx.org@gnu.org Sat May 28 14:47:26 2022 Return-path: Envelope-to: guile-bugs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nuvqn-0006kf-U8 for guile-bugs@m.gmane-mx.org; Sat, 28 May 2022 14:47:26 +0200 Original-Received: from localhost ([::1]:47358 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nuvqk-0003qr-7b for guile-bugs@m.gmane-mx.org; Sat, 28 May 2022 08:47:24 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:35446) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nuvqQ-0003qD-VO for bug-guile@gnu.org; Sat, 28 May 2022 08:47:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:42717) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nuvqQ-00048o-L8 for bug-guile@gnu.org; Sat, 28 May 2022 08:47:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1nuvqQ-00010P-El for bug-guile@gnu.org; Sat, 28 May 2022 08:47:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Josselin Poiret Original-Sender: "Debbugs-submit" Resent-CC: bug-guile@gnu.org Resent-Date: Sat, 28 May 2022 12:47:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 52835 X-GNU-PR-Package: guile X-GNU-PR-Keywords: patch Original-Received: via spool by 52835-submit@debbugs.gnu.org id=B52835.16537420183836 (code B ref 52835); Sat, 28 May 2022 12:47:02 +0000 Original-Received: (at 52835) by debbugs.gnu.org; 28 May 2022 12:46:58 +0000 Original-Received: from localhost ([127.0.0.1]:36608 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nuvqL-0000zi-PP for submit@debbugs.gnu.org; Sat, 28 May 2022 08:46:58 -0400 Original-Received: from jpoiret.xyz ([206.189.101.64]:34418) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nuvqI-0000zL-3b for 52835@debbugs.gnu.org; Sat, 28 May 2022 08:46:55 -0400 Original-Received: from authenticated-user (jpoiret.xyz [206.189.101.64]) by jpoiret.xyz (Postfix) with ESMTPA id 1D372184F6B; Sat, 28 May 2022 12:46:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jpoiret.xyz; s=dkim; t=1653742013; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8m63TLUDs5+aH+MUdCMdFwxqTp48mqUGDe1QJLgZMwU=; b=eLQYquPNOUrHm3iPBwwXBhqiZGvjUL1dMclGGsFLktgCwPZXjQz8hy5Ij/sVMqqV0iN2ww mZeVQDTH9eETtN2yWBh9PAAzUjyZMtuMd6z2mV2cww25siL1tAa8BP/A8hJBAOtJQXfolQ 9yR/Q9e3s50w/RbGpI5W1hilnYjLlfP9MBE+v/pOu9fu2VTzBBvBq5vD6g33WIwvnqoXdC 8WovnLKoz1pn4V2fnVluizEvEBcJG61u/vmk/d7zI9c4Xr3LCOjWcg1tA7zJU/GtIXimFl QIafCI+H+P+tdztI1mSsxEAoaRepQDUPeniX93ETeKNPpV7b9mBiXlJ+1ZjMew== In-Reply-To: <20220528124634.17353-1-dev@jpoiret.xyz> Authentication-Results: jpoiret.xyz; auth=pass smtp.auth=jpoiret@jpoiret.xyz smtp.mailfrom=dev@jpoiret.xyz X-Spamd-Bar: / X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-guile@gnu.org List-Id: "Bug reports for GUILE, GNU's Ubiquitous Extension Language" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guile-bounces+guile-bugs=m.gmane-mx.org@gnu.org Original-Sender: "bug-guile" Xref: news.gmane.io gmane.lisp.guile.bugs:10283 Archived-At: * libguile/posix.c (renumber_file_descriptor): Refactor it as dup_handle_error. (dup_handle_error, dup2_handle_error): New functions that wrap around dup and dup2 by retrying on EINTR or EBUSY, as well as erroring out on other errors. (start_child): Close standard file descriptors only after all of them have been dup2'd. --- libguile/posix.c | 84 +++++++++++++++++++++++++++++++----------------- 1 file changed, 54 insertions(+), 30 deletions(-) diff --git a/libguile/posix.c b/libguile/posix.c index 3ab12b99e..e9f49fa27 100644 --- a/libguile/posix.c +++ b/libguile/posix.c @@ -1280,14 +1280,14 @@ SCM_DEFINE (scm_fork, "primitive-fork", 0, 0, 0, #endif /* HAVE_FORK */ #ifdef HAVE_FORK -/* 'renumber_file_descriptor' is a helper function for 'start_child' - below, and is specialized for that particular environment where it - doesn't make sense to report errors via exceptions. It uses dup(2) - to duplicate the file descriptor FD, closes the original FD, and - returns the new descriptor. If dup(2) fails, print an error message - to ERR and abort. */ +/* 'dup_handle_error' is a helper function for 'start_child' below, and + is specialized for that particular environment where it doesn't make + sense to report errors via exceptions. It uses dup(2) to duplicate + the file descriptor FD, does *not* close the original FD, and returns + the new descriptor. If dup(2) fails, print an error message to ERR + and abort. */ static int -renumber_file_descriptor (int fd, int err) +dup_handle_error (int fd, int err) { int new_fd; @@ -1304,7 +1304,33 @@ renumber_file_descriptor (int fd, int err) _exit (127); /* Use exit status 127, as with other exec errors. */ } - close (fd); + return new_fd; +} + +/* 'dup2_handle_error' is a helper function for 'start_child' below, and + is specialized for that particular environment where it doesn't make + sense to report errors via exceptions. It uses dup2(2) to duplicate + the file descriptor FD, does *not* close the original FD, and returns + the new descriptor. If dup2(2) fails, print an error message to ERR + and abort. */ +static int +dup2_handle_error (int fd, int to, int err) +{ + int new_fd; + + do + new_fd = dup2 (fd, to); + while (new_fd == -1 && (errno == EINTR || errno == EBUSY)); + + if (new_fd == -1) + { + /* At this point we are in the child process before exec. We + cannot safely raise an exception in this environment. */ + const char *msg = strerror (errno); + fprintf (fdopen (err, "a"), "start_child: dup failed: %s\n", msg); + _exit (127); /* Use exit status 127, as with other exec errors. */ + } + return new_fd; } #endif /* HAVE_FORK */ @@ -1357,34 +1383,32 @@ start_child (const char *exec_file, char **exec_argv, if (err == -1) err = open ("/dev/null", O_WRONLY); - if (in > 0) - { - if (out == 0) - out = renumber_file_descriptor (out, err); - if (err == 0) - err = renumber_file_descriptor (err, err); - do dup2 (in, 0); while (errno == EINTR); - close (in); - } - if (out > 1) - { - if (err == 1) - err = renumber_file_descriptor (err, err); - do dup2 (out, 1); while (errno == EINTR); - close (out); - } - if (err > 2) - { - do dup2 (err, 2); while (errno == EINTR); - close (err); - } + /* Dup each non-yet-dup2'd fd that's in the way to the next available fd, + so that we can safely dup2 to 0/1/2 without potentially overwriting + in/out/err. Note that dup2 doesn't do anything if its arguments are + equal. */ + if (out == 0) + out = dup_handle_error (out, err); + if (err == 0) + err = dup_handle_error (err, err); + dup2_handle_error (in, 0, err); + + if (err == 1) + err = dup_handle_error (err, err); + dup2_handle_error (out, 1, err); + + dup2_handle_error (err, 2, err); + + if (in > 2) close (in); + if (out > 2) close (out); + if (err > 2) close (err); execvp (exec_file, exec_argv); /* The exec failed! There is nothing sensible to do. */ { const char *msg = strerror (errno); - fprintf (fdopen (2, "a"), "In execvp of %s: %s\n", + dprintf (2, "In execvp of %s: %s\n", exec_file, msg); } -- 2.36.0