bug#52835: [PATCH v3] Fix child spawning closing standard fds prematurely

* 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.
---
Hello,

This is a new version of the fix that should now handle possible
dup/dup2 errors properly.

Best,
Josselin
 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.34.0