summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThomas Haller <thaller@redhat.com>2022-10-12 23:30:37 +0200
committerThomas Haller <thaller@redhat.com>2022-10-19 20:54:18 +0200
commit6418db829b6f56b92beb044f51f94c7a1bd7ae51 (patch)
tree79a6c311ebf69a2ba799b12ad582db13f918cab6
parent06029c5945507ef9348d8df0eb07ec280112d9c7 (diff)
downloadglib-6418db829b6f56b92beb044f51f94c7a1bd7ae51.tar.gz
gspawn: avoid race due to retry with EINTR on close()
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')
-rw-r--r--glib/gspawn.c25
1 files 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;
}