From 9e69f8b280afe8eccd9188cc53b8117e1b238db7 Mon Sep 17 00:00:00 2001

From: Michael Catanzaro <mcatanzaro@redhat.com>

Date: Tue, 12 Oct 2021 15:52:18 -0500

Subject: [PATCH 01/10] gspawn: use close_and_invalidate more

---

 glib/gspawn.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/glib/gspawn.c b/glib/gspawn.c

index a15fb1ca1..5d8422869 100644

--- a/glib/gspawn.c

+++ b/glib/gspawn.c

@@ -1710,7 +1710,7 @@ do_exec (gint                  child_err_report_fd,

                 child_err_report_fd = safe_dup (child_err_report_fd);

               safe_dup2 (source_fds[i], target_fds[i]);

-              (void) close (source_fds[i]);

+              close_and_invalidate (&source_fds[i]);

             }

         }

     }

-- 

2.33.1

From fe2148fd5dd4f2e5c413c5cc0bb56c4a19304887 Mon Sep 17 00:00:00 2001

From: Michael Catanzaro <mcatanzaro@redhat.com>

Date: Thu, 14 Oct 2021 10:43:52 -0500

Subject: [PATCH 02/10] gspawn: Improve error message when dup fails

This error message is no longer accurate now that we allow arbitrary fd

remapping.

---

 glib/gspawn.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/glib/gspawn.c b/glib/gspawn.c

index 5d8422869..e214a3998 100644

--- a/glib/gspawn.c

+++ b/glib/gspawn.c

@@ -2363,7 +2363,7 @@ fork_exec (gboolean              intermediate_child,

               g_set_error (error,

                            G_SPAWN_ERROR,

                            G_SPAWN_ERROR_FAILED,

-                           _("Failed to redirect output or input of child process (%s)"),

+                           _("Failed to duplicate file descriptor for child process (%s)"),

                            g_strerror (buf[1]));

               break;

-- 

2.33.1

From 566eccdb0a2594b4d3ec13c7443028d968b41af8 Mon Sep 17 00:00:00 2001

From: Michael Catanzaro <mcatanzaro@redhat.com>

Date: Tue, 12 Oct 2021 15:33:59 -0500

Subject: [PATCH 03/10] gspawn: fix hangs when duping child_err_report_fd

In case child_err_report_fd conflicts with one of the target_fds, the

code here is careful to dup child_err_report_fd in order to avoid

conflating the two. It was a good idea, but evidently was not tested,

because the newly-created fd is not created with CLOEXEC set. This means

it stays open in the child process, causing the parent to hang forever

waiting to read from the other end of the pipe. Oops!

The fix is simple: just set CLOEXEC. This removes our only usage of the

safe_dup() function, so it can be dropped.

Fixes #2506

---

 glib/gspawn.c | 16 +---------------

 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/glib/gspawn.c b/glib/gspawn.c

index e214a3998..8bbe573f7 100644

--- a/glib/gspawn.c

+++ b/glib/gspawn.c

@@ -1500,20 +1500,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

@@ -1707,7 +1693,7 @@ do_exec (gint                  child_err_report_fd,

           else

             {

               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);

               safe_dup2 (source_fds[i], target_fds[i]);

               close_and_invalidate (&source_fds[i]);

-- 

2.33.1

From b703fa8b760ac9272c5a0ed3e3763b2f71ecf574 Mon Sep 17 00:00:00 2001

From: Michael Catanzaro <mcatanzaro@redhat.com>

Date: Thu, 14 Oct 2021 10:44:57 -0500

Subject: [PATCH 04/10] gspawn: fix fd remapping conflation issue

We currently dup all source fds to avoid possible conflation with the

target fds, but fail to consider that the result of a dup might itself

conflict with one of the target fds. Solve this the easy way by duping

all source_fds to values that are greater than the largest fd in

target_fds.

Fixes #2503

---

 glib/gspawn.c | 43 +++++++++++++++++++++++++------------------

 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/glib/gspawn.c b/glib/gspawn.c

index 8bbe573f7..2b48b5600 100644

--- a/glib/gspawn.c

+++ b/glib/gspawn.c

@@ -1258,13 +1258,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);

@@ -1275,7 +1275,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);

@@ -1563,6 +1563,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,

@@ -1661,39 +1662,45 @@ 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 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]);

+            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 = dupfd_cloexec (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]);

               close_and_invalidate (&source_fds[i]);

-- 

2.33.1

From ecc3538a942760e8b403c319d359711c8e166778 Mon Sep 17 00:00:00 2001

From: Michael Catanzaro <mcatanzaro@gnome.org>

Date: Thu, 25 Feb 2021 12:20:39 -0600

Subject: [PATCH 05/10] 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 | 65 +++++++++++++++++++++++++++++++++++++++++++++------

 1 file changed, 58 insertions(+), 7 deletions(-)

diff --git a/glib/gspawn.c b/glib/gspawn.c

index 2b48b5600..9ef78dbe1 100644

--- a/glib/gspawn.c

+++ b/glib/gspawn.c

@@ -1786,9 +1786,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;

@@ -1797,7 +1802,8 @@ do_posix_spawn (const gchar * const *argv,

   GSList *child_close = NULL;

   GSList *elem;

   sigset_t mask;

-  int i, r;

+  size_t i;

+  int r;

   if (*argv[0] == '\0')

     {

@@ -1911,6 +1917,43 @@ 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]);

+

+  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.

    *

@@ -1940,9 +1983,16 @@ do_posix_spawn (const gchar * const *argv,

     *child_pid = pid;

 out_close_fds:

-  for (i = 0; i < num_parent_close_fds; i++)

+  for (i = 0; i < (size_t) 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);

@@ -2030,10 +2080,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",

@@ -2050,7 +2098,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;

-- 

2.33.1

From 731d6c32105dc97f2b777ef9a34c6b76d1489c04 Mon Sep 17 00:00:00 2001

From: Michael Catanzaro <mcatanzaro@gnome.org>

Date: Thu, 25 Feb 2021 12:21:38 -0600

Subject: [PATCH 06/10] gsubprocess: ensure we test fd remapping on the

 posix_spawn() codepath

We should run test_pass_fd twice, once using gspawn's fork/exec codepath

and once attempting to use its posix_spawn() codepath. There's no

guarantee we'll actually get the posix_spawn() codepath, but it works

for now on Linux.

For good measure, run it a third time with no flags at all.

This causes the test to fail if I separately break the fd remapping

implementation. Without this, we fail to test fd remapping on the

posix_spawn() codepath.

---

 gio/tests/gsubprocess.c | 44 ++++++++++++++++++++++++++++++++++++++---

 1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/gio/tests/gsubprocess.c b/gio/tests/gsubprocess.c

index 7e22678ec..ba49c1c43 100644

--- a/gio/tests/gsubprocess.c

+++ b/gio/tests/gsubprocess.c

@@ -1697,7 +1697,8 @@ test_child_setup (void)

 }

 static void

-test_pass_fd (void)

+do_test_pass_fd (GSubprocessFlags     flags,

+                 GSpawnChildSetupFunc child_setup)

 {

   GError *local_error = NULL;

   GError **error = &local_error;

@@ -1722,9 +1723,11 @@ test_pass_fd (void)

   needdup_fd_str = g_strdup_printf ("%d", needdup_pipefds[1] + 1);

   args = get_test_subprocess_args ("write-to-fds", basic_fd_str, needdup_fd_str, NULL);

-  launcher = g_subprocess_launcher_new (G_SUBPROCESS_FLAGS_NONE);

+  launcher = g_subprocess_launcher_new (flags);

   g_subprocess_launcher_take_fd (launcher, basic_pipefds[1], basic_pipefds[1]);

   g_subprocess_launcher_take_fd (launcher, needdup_pipefds[1], needdup_pipefds[1] + 1);

+  if (child_setup != NULL)

+    g_subprocess_launcher_set_child_setup (launcher, child_setup, NULL, NULL);

   proc = g_subprocess_launcher_spawnv (launcher, (const gchar * const *) args->pdata, error);

   g_ptr_array_free (args, TRUE);

   g_assert_no_error (local_error);

@@ -1754,6 +1757,39 @@ test_pass_fd (void)

   g_object_unref (proc);

 }

+static void

+test_pass_fd (void)

+{

+  do_test_pass_fd (G_SUBPROCESS_FLAGS_NONE, NULL);

+}

+

+static void

+empty_child_setup (gpointer user_data)

+{

+}

+

+static void

+test_pass_fd_empty_child_setup (void)

+{

+  /* Using a child setup function forces gspawn to use fork/exec

+   * rather than posix_spawn.

+   */

+  do_test_pass_fd (G_SUBPROCESS_FLAGS_NONE, empty_child_setup);

+}

+

+static void

+test_pass_fd_inherit_fds (void)

+{

+  /* Try to test the optimized posix_spawn codepath instead of

+   * fork/exec. Currently this requires using INHERIT_FDS since gspawn's

+   * posix_spawn codepath does not currently handle closing

+   * non-inherited fds. Note that using INHERIT_FDS means our testing of

+   * g_subprocess_launcher_take_fd() is less-comprehensive than when

+   * using G_SUBPROCESS_FLAGS_NONE.

+   */

+  do_test_pass_fd (G_SUBPROCESS_FLAGS_INHERIT_FDS, NULL);

+}

+

 #endif

 static void

@@ -1891,7 +1927,9 @@ main (int argc, char **argv)

   g_test_add_func ("/gsubprocess/stdout-file", test_stdout_file);

   g_test_add_func ("/gsubprocess/stdout-fd", test_stdout_fd);

   g_test_add_func ("/gsubprocess/child-setup", test_child_setup);

-  g_test_add_func ("/gsubprocess/pass-fd", test_pass_fd);

+  g_test_add_func ("/gsubprocess/pass-fd/basic", test_pass_fd);

+  g_test_add_func ("/gsubprocess/pass-fd/empty-child-setup", test_pass_fd_empty_child_setup);

+  g_test_add_func ("/gsubprocess/pass-fd/inherit-fds", test_pass_fd_inherit_fds);

 #endif

   g_test_add_func ("/gsubprocess/launcher-environment", test_launcher_environment);

-- 

2.33.1

From 4608940466a04a32d4e6e71dbe872cfecb136118 Mon Sep 17 00:00:00 2001

From: Michael Catanzaro <mcatanzaro@redhat.com>

Date: Thu, 14 Oct 2021 11:01:33 -0500

Subject: [PATCH 07/10] gspawn: Check from errors from safe_dup2() and

 dupfd_cloexec()

Although unlikely, these functions can fail, e.g. if we run out of file

descriptors. Check for errors to improve robustness. This is especially

important now that I changed our use of dupfd_cloexec() to avoid

returning fds smaller than the largest fd in target_fds. An application

that attempts to remap to the highest-allowed fd value deserves at least

some sort of attempt at error reporting, not silent failure.

---

 glib/gspawn.c | 40 +++++++++++++++++++++++++++++-----------

 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/glib/gspawn.c b/glib/gspawn.c

index 9ef78dbe1..7ef678047 100644

--- a/glib/gspawn.c

+++ b/glib/gspawn.c

@@ -1572,7 +1572,6 @@ do_exec (gint                  child_err_report_fd,

   /* Redirect pipes as required */

   if (stdin_fd >= 0)

     {

-      /* dup2 can't actually fail here I don't think */

       if (safe_dup2 (stdin_fd, 0) < 0)

         write_err_and_exit (child_err_report_fd,

                             CHILD_DUP2_FAILED);

@@ -1588,13 +1587,14 @@ do_exec (gint                  child_err_report_fd,

       if (read_null < 0)

         write_err_and_exit (child_err_report_fd,

                             CHILD_DUP2_FAILED);

-      safe_dup2 (read_null, 0);

+      if (safe_dup2 (read_null, 0) < 0)

+        write_err_and_exit (child_err_report_fd,

+                            CHILD_DUP2_FAILED);

       close_and_invalidate (&read_null);

     }

   if (stdout_fd >= 0)

     {

-      /* dup2 can't actually fail here I don't think */

       if (safe_dup2 (stdout_fd, 1) < 0)

         write_err_and_exit (child_err_report_fd,

                             CHILD_DUP2_FAILED);

@@ -1609,13 +1609,14 @@ do_exec (gint                  child_err_report_fd,

       if (write_null < 0)

         write_err_and_exit (child_err_report_fd,

                             CHILD_DUP2_FAILED);

-      safe_dup2 (write_null, 1);

+      if (safe_dup2 (write_null, 1) < 0)

+        write_err_and_exit (child_err_report_fd,

+                            CHILD_DUP2_FAILED);

       close_and_invalidate (&write_null);

     }

   if (stderr_fd >= 0)

     {

-      /* dup2 can't actually fail here I don't think */

       if (safe_dup2 (stderr_fd, 2) < 0)

         write_err_and_exit (child_err_report_fd,

                             CHILD_DUP2_FAILED);

@@ -1630,7 +1631,9 @@ do_exec (gint                  child_err_report_fd,

       if (write_null < 0)

         write_err_and_exit (child_err_report_fd,

                             CHILD_DUP2_FAILED);

-      safe_dup2 (write_null, 2);

+      if (safe_dup2 (write_null, 2) < 0)

+        write_err_and_exit (child_err_report_fd,

+                            CHILD_DUP2_FAILED);

       close_and_invalidate (&write_null);

     }

@@ -1643,7 +1646,8 @@ do_exec (gint                  child_err_report_fd,

     {

       if (child_setup == NULL && n_fds == 0)

         {

-          safe_dup2 (child_err_report_fd, 3);

+          if (safe_dup2 (child_err_report_fd, 3) < 0)

+            write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED);

           set_cloexec (GINT_TO_POINTER (0), 3);

           safe_closefrom (4);

           child_err_report_fd = 3;

@@ -1682,7 +1686,11 @@ do_exec (gint                  child_err_report_fd,

       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);

+            {

+              source_fds[i] = dupfd_cloexec (source_fds[i], max_target_fd + 1);

+              if (source_fds[i] < 0)

+                write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED);

+            }

         }

       for (i = 0; i < n_fds; i++)

@@ -1700,9 +1708,15 @@ do_exec (gint                  child_err_report_fd,

                * dup it so it doesn’t get conflated.

                */

               if (target_fds[i] == child_err_report_fd)

-                child_err_report_fd = dupfd_cloexec (child_err_report_fd, max_target_fd + 1);

+                {

+                  child_err_report_fd = dupfd_cloexec (child_err_report_fd, max_target_fd + 1);

+                  if (child_err_report_fd < 0)

+                    write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED);

+                }

+

+              if (safe_dup2 (source_fds[i], target_fds[i]) < 0)

+                write_err_and_exit (child_err_report_fd, CHILD_DUP2_FAILED);

-              safe_dup2 (source_fds[i], target_fds[i]);

               close_and_invalidate (&source_fds[i]);

             }

         }

@@ -1945,7 +1959,11 @@ do_posix_spawn (const gchar * const *argv,

   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);

+    {

+      duped_source_fds[i] = dupfd_cloexec (source_fds[i], max_target_fd + 1);

+      if (duped_source_fds[i] < 0)

+        goto out_close_fds;

+    }

   for (i = 0; i < n_fds; i++)

     {

-- 

2.33.1

From 0198b6a1c8c215f524d7c6ed2d240fb1b31d9865 Mon Sep 17 00:00:00 2001

From: Michael Catanzaro <mcatanzaro@redhat.com>

Date: Wed, 20 Oct 2021 16:51:44 -0500

Subject: [PATCH 08/10] gspawn: add new error message for open() failures

Reporting these as dup2() failures is bogus.

---

 glib/gspawn.c | 17 +++++++++++++----

 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/glib/gspawn.c b/glib/gspawn.c

index 7ef678047..c2fe306dc 100644

--- a/glib/gspawn.c

+++ b/glib/gspawn.c

@@ -1532,6 +1532,7 @@ enum

 {

   CHILD_CHDIR_FAILED,

   CHILD_EXEC_FAILED,

+  CHILD_OPEN_FAILED,

   CHILD_DUP2_FAILED,

   CHILD_FORK_FAILED

 };

@@ -1586,7 +1587,7 @@ do_exec (gint                  child_err_report_fd,

       gint read_null = safe_open ("/dev/null", O_RDONLY);

       if (read_null < 0)

         write_err_and_exit (child_err_report_fd,

-                            CHILD_DUP2_FAILED);

+                            CHILD_OPEN_FAILED);

       if (safe_dup2 (read_null, 0) < 0)

         write_err_and_exit (child_err_report_fd,

                             CHILD_DUP2_FAILED);

@@ -1608,7 +1609,7 @@ do_exec (gint                  child_err_report_fd,

       gint write_null = safe_open ("/dev/null", O_WRONLY);

       if (write_null < 0)

         write_err_and_exit (child_err_report_fd,

-                            CHILD_DUP2_FAILED);

+                            CHILD_OPEN_FAILED);

       if (safe_dup2 (write_null, 1) < 0)

         write_err_and_exit (child_err_report_fd,

                             CHILD_DUP2_FAILED);

@@ -1630,7 +1631,7 @@ do_exec (gint                  child_err_report_fd,

       gint write_null = safe_open ("/dev/null", O_WRONLY);

       if (write_null < 0)

         write_err_and_exit (child_err_report_fd,

-                            CHILD_DUP2_FAILED);

+                            CHILD_OPEN_FAILED);

       if (safe_dup2 (write_null, 2) < 0)

         write_err_and_exit (child_err_report_fd,

                             CHILD_DUP2_FAILED);

@@ -2420,7 +2421,15 @@ fork_exec (gboolean              intermediate_child,

                            g_strerror (buf[1]));

               break;

-              

+

+            case CHILD_OPEN_FAILED:

+              g_set_error (error,

+                           G_SPAWN_ERROR,

+                           G_SPAWN_ERROR_FAILED,

+                           _("Failed to open file to remap file descriptor (%s)"),

+                           g_strerror (buf[1]));

+              break;

+

             case CHILD_DUP2_FAILED:

               g_set_error (error,

                            G_SPAWN_ERROR,

-- 

2.33.1

From e4abb5f3db85b2f730e192e6398f26934e41ba21 Mon Sep 17 00:00:00 2001

From: Michael Catanzaro <mcatanzaro@redhat.com>

Date: Tue, 26 Oct 2021 21:27:15 -0500

Subject: [PATCH 09/10] Add tests for GSubprocess fd conflation issues

This tests for #2503. It's fragile, but there is no non-fragile way to

test this. If the test breaks in the future, it will pass without

successfully testing the bug, not fail spuriously, so I think this is

OK.

---

 gio/tests/gsubprocess-testprog.c |  53 +++++++++++-

 gio/tests/gsubprocess.c          | 144 +++++++++++++++++++++++++++++++

 2 files changed, 195 insertions(+), 2 deletions(-)

diff --git a/gio/tests/gsubprocess-testprog.c b/gio/tests/gsubprocess-testprog.c

index c9b06c2a2..58cb1c71d 100644

--- a/gio/tests/gsubprocess-testprog.c

+++ b/gio/tests/gsubprocess-testprog.c

@@ -5,8 +5,6 @@

 #include <errno.h>

 #ifdef G_OS_UNIX

 #include <unistd.h>

-#include <gio/gunixinputstream.h>

-#include <gio/gunixoutputstream.h>

 #else

 #include <io.h>

 #endif

@@ -150,6 +148,55 @@ write_to_fds (int argc, char **argv)

   return 0;

 }

+static int

+read_from_fd (int argc, char **argv)

+{

+  int fd;

+  const char expectedResult[] = "Yay success!";

+  guint8 buf[sizeof (expectedResult) + 1];

+  gsize bytes_read;

+  FILE *f;

+

+  if (argc != 3)

+    {

+      g_print ("Usage: %s read-from-fd FD\n", argv[0]);

+      return 1;

+    }

+

+  fd = atoi (argv[2]);

+  if (fd == 0)

+    {

+      g_warning ("Argument \"%s\" does not look like a valid nonzero file descriptor", argv[2]);

+      return 1;

+    }

+

+  f = fdopen (fd, "r");

+  if (f == NULL)

+    {

+      g_warning ("Failed to open fd %d: %s", fd, g_strerror (errno));

+      return 1;

+    }

+

+  bytes_read = fread (buf, 1, sizeof (buf), f);

+  if (bytes_read != sizeof (expectedResult))