From: Josselin Poiret via Bug reports for GNU Guix <bug-guix@gnu.org>
To: "Ludovic Courtès" <ludo@gnu.org>,
"Christopher Baines" <mail@cbaines.net>
Cc: 55441@debbugs.gnu.org, Josselin Poiret <dev@jpoiret.xyz>
Subject: bug#55441: [PATCH 1/2] Fix child spawning closing standard fds prematurely
Date: Thu, 26 May 2022 22:50:46 +0200 [thread overview]
Message-ID: <20220526205047.23960-1-dev@jpoiret.xyz> (raw)
In-Reply-To: <87sfow10lc.fsf@jpoiret.xyz>
* 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 | 82 +++++++++++++++++++++++++++++++-----------------
1 file changed, 53 insertions(+), 29 deletions(-)
diff --git a/libguile/posix.c b/libguile/posix.c
index 3ab12b99e..dc3080b3c 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,27 +1383,25 @@ 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);
--
2.36.0
next prev parent reply other threads:[~2022-05-26 20:51 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <87fslcaznn.fsf@gmail.com>
[not found] ` <87mtfj174l.fsf@gnu.org>
2022-05-16 3:49 ` bug#55441: [cuirass] hang in "In progress..."; runs out of pgsql connections Maxim Cournoyer
2022-05-16 7:32 ` Mathieu Othacehe
2022-05-16 9:13 ` Ludovic Courtès
2022-05-16 10:32 ` Ludovic Courtès
2022-05-16 12:19 ` Ludovic Courtès
2022-05-16 17:32 ` Maxim Cournoyer
2022-05-17 7:45 ` Ludovic Courtès
2022-05-18 7:44 ` Ludovic Courtès
2022-05-20 18:32 ` Ludovic Courtès
2022-05-24 21:02 ` Ludovic Courtès
2022-05-25 16:32 ` Ludovic Courtès
2022-05-25 18:21 ` Ludovic Courtès
2022-05-25 23:13 ` Christopher Baines
2022-05-26 9:44 ` Ludovic Courtès
2022-05-26 20:49 ` Josselin Poiret via Bug reports for GNU Guix
2022-05-26 20:50 ` bug#55441: [PATCH] guix: inferior: Make open-bidirectional-pipe use piped-process Josselin Poiret via Bug reports for GNU Guix
2022-05-26 21:02 ` Maxime Devos
2022-05-28 17:15 ` Ludovic Courtès
2022-05-26 20:50 ` Josselin Poiret via Bug reports for GNU Guix [this message]
2022-05-26 20:55 ` bug#55441: [PATCH 1/2] Fix child spawning closing standard fds prematurely Maxime Devos
2022-05-26 20:50 ` bug#55441: [PATCH 2/2] Improve thread safety of piped-process Josselin Poiret via Bug reports for GNU Guix
2022-05-26 20:58 ` bug#55441: [cuirass] hang in "In progress..."; runs out of pgsql connections Maxime Devos
2022-05-28 14:02 ` Josselin Poiret via Bug reports for GNU Guix
2023-01-26 14:39 ` Ludovic Courtès
2022-05-26 12:29 ` Ludovic Courtès
2022-05-16 12:49 ` Maxim Cournoyer
2022-05-17 12:52 ` Ludovic Courtès
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220526205047.23960-1-dev@jpoiret.xyz \
--to=bug-guix@gnu.org \
--cc=55441@debbugs.gnu.org \
--cc=dev@jpoiret.xyz \
--cc=ludo@gnu.org \
--cc=mail@cbaines.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.