[glib: 2/4] gsubprocesslauncher: Don’t close target FDs in close() method

[Sebastian Dröge <sdroege src gnome org>]

commit 55a75590d0c7e703f32513cc409d6e20a7b761ea
Author: Philip Withnall <pwithnall endlessos org>
Date:   Fri Feb 19 18:20:25 2021 +0000

    gsubprocesslauncher: Don’t close target FDs in close() method
    
    This is a regression introduced in commit 67a589e505311. Previously, the
    source/target FD pairs were stored in `needdup_fd_assignments`, in
    consecutive entries, so source FDs had even indices and target FDs had
    odd indices.
    
    I didn’t notice that the array index was being incremented by 2 when
    closing FDs, when porting from the old code. So previously the code was
    only closing the source FDs; after the port, it was closing source and
    target FDs.
    
    That’s incorrect, as the target FDs are just integers in the parent
    process. It’s only in the child process where they are actually FDs —
    and `g_subprocess_launcher_close()` is never called in the child
    process.
    
    This resulted in some strange misbehaviours in any process which used
    `g_subprocess_launcher_take_fd()` with target FDs which could have
    possibly aliased with other FDs in the parent process (and which weren’t
    equal to their mapped source FDs).
    
    Thanks to Olivier Fourdan for the detailed bug report.
    
    Signed-off-by: Philip Withnall <pwithnall endlessos org>
    
    Fixes: #2332

 gio/gsubprocesslauncher-private.h | 4 ++--
 gio/gsubprocesslauncher.c         | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)
---
diff --git a/gio/gsubprocesslauncher-private.h b/gio/gsubprocesslauncher-private.h
index f8a6516c5..d6fe0d784 100644
--- a/gio/gsubprocesslauncher-private.h
+++ b/gio/gsubprocesslauncher-private.h
@@ -42,8 +42,8 @@ struct _GSubprocessLauncher
   gint stderr_fd;
   gchar *stderr_path;
 
-  GArray *source_fds;
-  GArray *target_fds;  /* always the same length as source_fds */
+  GArray *source_fds;  /* GSubprocessLauncher has ownership of the FD elements */
+  GArray *target_fds;  /* always the same length as source_fds; elements are just integers and not FDs in 
this process */
   gboolean closed_fd;
 
   GSpawnChildSetupFunc child_setup_func;
diff --git a/gio/gsubprocesslauncher.c b/gio/gsubprocesslauncher.c
index 16c47d542..a1c65e947 100644
--- a/gio/gsubprocesslauncher.c
+++ b/gio/gsubprocesslauncher.c
@@ -661,11 +661,11 @@ g_subprocess_launcher_close (GSubprocessLauncher *self)
       g_assert (self->target_fds != NULL);
       g_assert (self->source_fds->len == self->target_fds->len);
 
+      /* Note: Don’t close the target_fds, as they’re only valid FDs in the
+       * child process. This code never executes in the child process. */
       for (i = 0; i < self->source_fds->len; i++)
-        {
-          (void) close (g_array_index (self->source_fds, int, i));
-          (void) close (g_array_index (self->target_fds, int, i));
-        }
+        (void) close (g_array_index (self->source_fds, int, i));
+
       g_clear_pointer (&self->source_fds, g_array_unref);
       g_clear_pointer (&self->target_fds, g_array_unref);
     }