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

[Michael Catanzaro <mcatanzaro src gnome org>]

commit 31d80451345be3fc1a6f4a98a9fa0deb329a938d
Author: Michael Catanzaro <mcatanzaro gnome org>
Date:   Thu Feb 25 12:20:39 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 | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 52 insertions(+), 5 deletions(-)
---
diff --git a/glib/gspawn.c b/glib/gspawn.c
index 149f5a5a1..f4ef6685c 100644
--- a/glib/gspawn.c
+++ b/glib/gspawn.c
@@ -1874,9 +1874,14 @@ 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;
@@ -1999,6 +2004,41 @@ 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 temporarily in a first pass.
+   *
+   * 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 this
+   * wouldn't be needed, but unfortunately as of 2021 many libcs still
+   * don't do so, and we want this code to be safe for arbitrary libcs.
+   * 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 opens a small race window where the fd
+   * could be inherited into a child process if another thread spawns
+   * something at the same time. It is avoidable by using the same
+   * strategy that we already have to use anyway to handle source_fds[i]
+   * != target_fds[i], so we might as well always dupfd_cloexec here.
+   */
+  for (i = 0; i < n_fds; i++)
+    max_target_fd = MAX (max_target_fd, target_fds[i]);
+
+  duped_source_fds = g_newa (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.
    *
@@ -2031,6 +2071,12 @@ 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]);
+    }
+
   posix_spawn_file_actions_destroy (&file_actions);
 out_free_spawnattr:
   posix_spawnattr_destroy (&attr);
@@ -2118,10 +2164,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",
@@ -2138,7 +2182,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;