[glib/mcatanzaro/posix-spawn2: 2/7] gspawn: Implement fd remapping for posix_spawn codepath

[Michael Catanzaro <mcatanzaro src gnome org>]

commit 7d5bdff6d928deba2f9a999a85027f6517d08f55
Author: Michael Catanzaro <mcatanzaro redhat com>
Date:   Tue Dec 14 13:36:28 2021 -0600

    gspawn: Implement fd remapping for posix_spawn codepath
    
    This means that GSubprocess will (sometimes) be able to use the
    optimized posix_spawn codepath instead of having to fall back to
    fork/exec.

 glib/gspawn.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 61 insertions(+), 7 deletions(-)
---
diff --git a/glib/gspawn.c b/glib/gspawn.c
index fa921ef5d..94ecffa92 100644
--- a/glib/gspawn.c
+++ b/glib/gspawn.c
@@ -1880,18 +1880,24 @@ do_posix_spawn (const gchar * const *argv,
                 gint       *child_close_fds,
                 gint        stdin_fd,
                 gint        stdout_fd,
-                gint        stderr_fd)
+                gint        stderr_fd,
+                const gint *source_fds,
+                const gint *target_fds,
+                gsize       n_fds)
 {
   pid_t pid;
+  gint *duped_source_fds = NULL;
+  gint max_target_fd = 0;
   const gchar * const *argv_pass;
   posix_spawnattr_t attr;
   posix_spawn_file_actions_t file_actions;
   gint parent_close_fds[3];
-  gint num_parent_close_fds = 0;
+  gsize num_parent_close_fds = 0;
   GSList *child_close = NULL;
   GSList *elem;
   sigset_t mask;
-  int i, r;
+  gsize i;
+  int r;
 
   if (*argv[0] == '\0')
     {
@@ -2005,6 +2011,46 @@ do_posix_spawn (const gchar * const *argv,
         goto out_close_fds;
     }
 
+  /* If source_fds[i] != target_fds[i], 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, taking care to ensure the new fds are
+   * larger than any target fd to avoid introducing new conflicts.
+   *
+   * If source_fds[i] == target_fds[i], then we just need to leak
+   * the fd into the child process, which we *could* do by temporarily
+   * unsetting CLOEXEC and then setting it again after we spawn if
+   * it was originally set. POSIX requires that the addup2 action unset
+   * CLOEXEC if source and target are identical, so you'd think doing it
+   * manually wouldn't be needed, but unfortunately as of 2021 many
+   * libcs still don't do so. Example nonconforming libcs:
+   *  Bionic: 
https://android.googlesource.com/platform/bionic/+/f6e5b582604715729b09db3e36a7aeb8c24b36a4/libc/bionic/spawn.cpp#71
+   *  uclibc-ng: 
https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/librt/spawn.c?id=7c36bcae09d66bbaa35cbb02253ae0556f42677e#n88
+   *
+   * Anyway, unsetting CLOEXEC ourselves would open a small race window
+   * where the fd could be inherited into a child process if another
+   * thread spawns something at the same time, because we have not
+   * called fork() and are multithreaded here. This race is avoidable by
+   * using dupfd_cloexec, which we already have to do to handle the
+   * source_fds[i] != target_fds[i] case. So let's always do it!
+   */
+
+  for (i = 0; i < n_fds; i++)
+    max_target_fd = MAX (max_target_fd, target_fds[i]);
+
+  if (max_target_fd == G_MAXINT)
+    goto out_close_fds;
+
+  duped_source_fds = g_new (gint, n_fds);
+  for (i = 0; i < n_fds; i++)
+    duped_source_fds[i] = dupfd_cloexec (source_fds[i], max_target_fd + 1);
+
+  for (i = 0; i < n_fds; i++)
+    {
+      r = posix_spawn_file_actions_adddup2 (&file_actions, duped_source_fds[i], target_fds[i]);
+      if (r != 0)
+        goto out_close_fds;
+    }
+
   /* Intentionally close the fds in the child as the last file action,
    * having been careful not to add the same fd to this list twice.
    *
@@ -2037,6 +2083,13 @@ out_close_fds:
   for (i = 0; i < num_parent_close_fds; i++)
     close_and_invalidate (&parent_close_fds [i]);
 
+  if (duped_source_fds != NULL)
+    {
+      for (i = 0; i < n_fds; i++)
+        close_and_invalidate (&duped_source_fds[i]);
+      g_free (duped_source_fds);
+    }
+
   posix_spawn_file_actions_destroy (&file_actions);
 out_free_spawnattr:
   posix_spawnattr_destroy (&attr);
@@ -2124,10 +2177,8 @@ fork_exec (gboolean              intermediate_child,
   child_close_fds[n_child_close_fds++] = -1;
 
 #ifdef POSIX_SPAWN_AVAILABLE
-  /* FIXME: Handle @source_fds and @target_fds in do_posix_spawn() using the
-   * file actions API. */
   if (!intermediate_child && working_directory == NULL && !close_descriptors &&
-      !search_path_from_envp && child_setup == NULL && n_fds == 0)
+      !search_path_from_envp && child_setup == NULL)
     {
       g_trace_mark (G_TRACE_CURRENT_TIME, 0,
                     "GLib", "posix_spawn",
@@ -2144,7 +2195,10 @@ fork_exec (gboolean              intermediate_child,
                                child_close_fds,
                                stdin_fd,
                                stdout_fd,
-                               stderr_fd);
+                               stderr_fd,
+                               source_fds,
+                               target_fds,
+                               n_fds);
       if (status == 0)
         goto success;