Merge branch 'backport-1968-spawn-fds-fix-glib-2-70' into 'glib-2-70'

Backport !1968 “gspawn: Fix file descriptor conflation issues” to glib-2-70

See merge request GNOME/glib!2394

glib/gspawn.c

@@ -1298,13 +1298,13 @@ unset_cloexec (int fd)
 /* This function is called between fork() and exec() and hence must be
  * async-signal-safe (see signal-safety(7)). */
 static int
-dupfd_cloexec (int parent_fd)
+dupfd_cloexec (int old_fd, int new_fd_min)
 {
   int fd, errsv;
 #ifdef F_DUPFD_CLOEXEC
   do
     {
-      fd = fcntl (parent_fd, F_DUPFD_CLOEXEC, 3);
+      fd = fcntl (old_fd, F_DUPFD_CLOEXEC, new_fd_min);
       errsv = errno;
     }
   while (fd == -1 && errsv == EINTR);
@@ -1315,7 +1315,7 @@ dupfd_cloexec (int parent_fd)
   int result, flags;
   do
     {
-      fd = fcntl (parent_fd, F_DUPFD, 3);
+      fd = fcntl (old_fd, F_DUPFD, new_fd_min);
       errsv = errno;
     }
   while (fd == -1 && errsv == EINTR);
@@ -1588,20 +1588,6 @@ safe_closefrom (int lowfd)
 #endif
 }
 
-/* This function is called between fork() and exec() and hence must be
- * async-signal-safe (see signal-safety(7)). */
-static gint
-safe_dup (gint fd)
-{
-  gint ret;
-
-  do
-    ret = dup (fd);
-  while (ret < 0 && (errno == EINTR || errno == EBUSY));
-
-  return ret;
-}
-
 /* This function is called between fork() and exec() and hence must be
  * async-signal-safe (see signal-safety(7)). */
 static gint
@@ -1665,6 +1651,7 @@ do_exec (gint                  child_err_report_fd,
          gpointer              user_data)
 {
   gsize i;
+  gint max_target_fd = 0;
 
   if (working_directory && chdir (working_directory) < 0)
     write_err_and_exit (child_err_report_fd,
@@ -1763,42 +1750,54 @@ do_exec (gint                  child_err_report_fd,
   /*
    * Work through the @source_fds and @target_fds mapping.
    *
-   * Based on code derived from
+   * Based on code originally derived from
    * gnome-terminal:src/terminal-screen.c:terminal_screen_child_setup(),
-   * used under the LGPLv2+ with permission from author.
+   * used under the LGPLv2+ with permission from author. (The code has
+   * since migrated to vte:src/spawn.cc:SpawnContext::exec and is no longer
+   * terribly similar to what we have here.)
    */
 
-  /* Basic fd assignments (where source == target) we can just unset FD_CLOEXEC
-   *
-   * If we're doing remapping fd assignments, we need to handle
-   * the case where the user has specified e.g.:
-   * 5 -> 4, 4 -> 6
-   *
-   * We do this by duping the source fds temporarily in a first pass.
-   *
-   * If any of the @target_fds conflict with @child_err_report_fd, dup the
-   * latter so it doesn’t get conflated.
-   */
   if (n_fds > 0)
     {
       for (i = 0; i < n_fds; i++)
+        max_target_fd = MAX (max_target_fd, target_fds[i]);
+
+      if (max_target_fd == G_MAXINT)
         {
-          if (source_fds[i] != target_fds[i])
+          errno = EINVAL;
-            source_fds[i] = dupfd_cloexec (source_fds[i]);
+          write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED);
         }
+
+      /* If we're doing remapping fd assignments, we need to handle
+       * the case where the user has specified e.g. 5 -> 4, 4 -> 6.
+       * We do this by duping all source fds, taking care to ensure the new
+       * fds are larger than any target fd to avoid introducing new conflicts.
+       */
       for (i = 0; i < n_fds; i++)
         {
+          if (source_fds[i] != target_fds[i])
+            source_fds[i] = dupfd_cloexec (source_fds[i], max_target_fd + 1);
+        }
+
+      for (i = 0; i < n_fds; i++)
+        {
+          /* For basic fd assignments (where source == target), we can just
+           * unset FD_CLOEXEC.
+           */
           if (source_fds[i] == target_fds[i])
             {
               unset_cloexec (source_fds[i]);
             }
           else
             {
+              /* If any of the @target_fds conflict with @child_err_report_fd,
+               * dup it so it doesn’t get conflated.
+               */
               if (target_fds[i] == child_err_report_fd)
-                child_err_report_fd = safe_dup (child_err_report_fd);
+                child_err_report_fd = dupfd_cloexec (child_err_report_fd, max_target_fd + 1);
 
               safe_dup2 (source_fds[i], target_fds[i]);
-              (void) close (source_fds[i]);
+              close_and_invalidate (&source_fds[i]);
             }
         }
     }