bug#52835: [PATCH v4 1/4] Fix child spawning closing standard fds prematurely.

Josselin Poiret via "Bug reports for GUILE, GNU's Ubiquitous Extension Language" <[email protected]>
Newsgroups gmane.lisp.guile.bugs
Message-ID <[email protected]>
* 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.