[glib/th/gspawn-no-safe-close] gspawn: avoid race due to retry with EINTR on close()




commit 444e357aea999021fe5d3a21794b4d86a9f5b988
Author: Thomas Haller <thaller redhat com>
Date:   Wed Oct 12 23:30:37 2022 +0200

    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.
    
    Don't only drop the retry loop, instead just use close() directly.
    Maybe keeping an internal wrapper around close() would be beneficial,
    to possibly abstract the handling of EINTR. However, glib has many
    direct calls to close(), that likewise don't wrap this. If you want
    a wrapper around close() (that is async-signal-safe, below g_close()
    and ), then add one project wide, not only inside "gspawn.c".
    
    This is 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.
    
    Fixes: 6f46294227f8 ('gspawn: Don’t use g_close() in async-signal-safe context')

 glib/gspawn.c | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)
---
diff --git a/glib/gspawn.c b/glib/gspawn.c
index 4e029eedf5..e9ef091a8a 100644
--- a/glib/gspawn.c
+++ b/glib/gspawn.c
@@ -162,8 +162,6 @@ extern char **environ;
  */
 
 
-static gint safe_close (gint fd);
-
 static gint g_execute (const gchar  *file,
                        gchar       **argv,
                        gchar       **argv_buffer,
@@ -269,7 +267,7 @@ close_and_invalidate (gint *fd)
     return;
   else
     {
-      safe_close (*fd);
+      close (*fd);
       *fd = -1;
     }
 }
@@ -1338,27 +1336,13 @@ dupfd_cloexec (int old_fd, int new_fd_min)
   return fd;
 }
 
-/* This function is called between fork() and exec() and hence must be
- * async-signal-safe (see signal-safety(7)). */
-static gint
-safe_close (gint fd)
-{
-  gint ret;
-
-  do
-    ret = close (fd);
-  while (ret < 0 && errno == EINTR);
-
-  return ret;
-}
-
 /* This function is called between fork() and exec() and hence must be
  * async-signal-safe (see signal-safety(7)). */
 G_GNUC_UNUSED static int
 close_func (void *data, int fd)
 {
   if (fd >= GPOINTER_TO_INT (data))
-    (void) safe_close (fd);
+    close (fd);
 
   return 0;
 }
@@ -1453,7 +1437,7 @@ safe_fdwalk (int (*cb)(void *data, int fd), void *data)
             }
         }
 
-      safe_close (dir_fd);
+      close (dir_fd);
       return res;
     }
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]