From f0bcb7f79c865a3cdd6f41b7e08f67a1a3d9d55c Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 12 Oct 2022 23:30:37 +0200
Subject: [PATCH] gspawn: avoid race due to retry with EINTR on close()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Retry on EINTR is wrong on many OS, including Linux. See the comment
in g_close() why that is.

As we cannot use g_close() after fork, we had safe_close(). This had the
wrong retry loop on EINTR. Drop that.

This was especially problematic since commit 6f46294227f8 ('gspawn: Don’t
use g_close() in async-signal-safe context'). Before, safe_close() was
only called after fork, where there is only one thread and there is no
concern about a race.

This patch only exists for easier backporting of the bugfix. The code
will be reworked further next.

Fixes: 6f46294227f8 ('gspawn: Don’t use g_close() in async-signal-safe context')
---
 glib/gspawn.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/glib/gspawn.c b/glib/gspawn.c
index 4e029eedf..c644cebb1 100644
--- a/glib/gspawn.c
+++ b/glib/gspawn.c
@@ -162,7 +162,7 @@ extern char **environ;
  */
 
 
-static gint safe_close (gint fd);
+static void safe_close (gint fd);
 
 static gint g_execute (const gchar  *file,
                        gchar       **argv,
@@ -1340,16 +1340,21 @@ dupfd_cloexec (int old_fd, int new_fd_min)
 
 /* This function is called between fork() and exec() and hence must be
  * async-signal-safe (see signal-safety(7)). */
-static gint
+static void
 safe_close (gint fd)
 {
-  gint ret;
-
-  do
-    ret = close (fd);
-  while (ret < 0 && errno == EINTR);
-
-  return ret;
+  /* Note that this function is (also) called after fork(), so it cannot use
+   * g_close().
+   * Note that it is however called both from the parent and the child
+   * process.
+   *
+   * This function returns no error, because there is nothing what the caller
+   * could do with that information. That is even the case for EINTR. See
+   * g_close() about the specialty of EINTR and why that is correct.
+   * If g_close() ever gets extended to handle EINTR specially, then this place
+   * and all other direct calls to close() need updating.
+   */
+  close (fd);
 }
 
 /* This function is called between fork() and exec() and hence must be
@@ -1358,7 +1363,7 @@ G_GNUC_UNUSED static int
 close_func (void *data, int fd)
 {
   if (fd >= GPOINTER_TO_INT (data))
-    (void) safe_close (fd);
+    safe_close (fd);
 
   return 0;
 }