diff options
author | Thomas Haller <thaller@redhat.com> | 2022-10-12 23:30:37 +0200 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2022-10-19 20:54:18 +0200 |
commit | 6418db829b6f56b92beb044f51f94c7a1bd7ae51 (patch) | |
tree | 79a6c311ebf69a2ba799b12ad582db13f918cab6 | |
parent | 06029c5945507ef9348d8df0eb07ec280112d9c7 (diff) | |
download | glib-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.c | 25 |
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; } |