[glib/mcatanzaro/posix-spawn2: 9/12] gspawn: Check from errors from safe_dup2() and dupfd_cloexec()

[Michael Catanzaro <mcatanzaro src gnome org>]

commit 7ab65c09da9ff111b28252b61fa91a6a0bffc59a
Author: Michael Catanzaro <mcatanzaro redhat com>
Date:   Thu Oct 14 11:01:33 2021 -0500

    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 ed08b329f..bd4a53b7f 100644
--- a/glib/gspawn.c
+++ b/glib/gspawn.c
@@ -1660,7 +1660,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);
@@ -1676,13 +1675,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);
@@ -1697,13 +1697,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);
@@ -1718,7 +1719,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);
     }
 
@@ -1731,7 +1734,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;
@@ -1770,7 +1774,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++)
@@ -1788,9 +1796,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]);
             }
         }
@@ -2033,7 +2047,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++)
     {