[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#52835: [PATCH v4 1/4] Fix child spawning closing standard fds premat
From: |
Josselin Poiret |
Subject: |
bug#52835: [PATCH v4 1/4] Fix child spawning closing standard fds prematurely. |
Date: |
Sat, 28 May 2022 14:46:31 +0200 |
* 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