Replace flatpak_close_fds_workaround() with g_fdwalk_set_cloexec()

flatpak_close_fds_workaround() wasn't technically async-signal-safe,
because the requirement for sysconf() to be async-signal-safe was
removed in POSIX.1-2008.

It could also leave high fds open in some cases: in practice
sysconf(_SC_OPEN_MAX) returns the soft resource limit, but if our
resource limit has been reduced by an ancestor process, we could
conceivably still have fds open and inherited above that number.

We can fix this by using g_fdwalk_set_cloexec() with GLib >= 2.79.2,
or the backport in libglnx with older GLib. This uses close_range()
if possible, falling back to rummaging in /proc with async-signal-safe
syscalls.

Signed-off-by: Simon McVittie <[email protected]>

common/flatpak-bwrap.c

@@ -498,8 +498,14 @@ flatpak_bwrap_child_setup (GArray *fd_array,
 {
  int i;
 
+ /* There is a dead-lock in glib versions before 2.60 when it closes
+ * the fds. See: https://gitlab.gnome.org/GNOME/glib/merge_requests/490
+ * This was hitting the test-suite a lot, so we work around it by using
+ * the G_SPAWN_LEAVE_DESCRIPTORS_OPEN/G_SUBPROCESS_FLAGS_INHERIT_FDS flag
+ * and setting CLOEXEC ourselves.
+ */
  if (close_fd_workaround)
- flatpak_close_fds_workaround (3);
+ g_fdwalk_set_cloexec (3);
 
  /* If no fd_array was specified, don't care. */
  if (fd_array == NULL)

common/flatpak-dir.c

@@ -7143,7 +7143,8 @@ flatpak_dir_run_triggers (FlatpakDir *self,
  commandline = flatpak_quote_argv ((const char **) bwrap->argv->pdata, -1);
  g_info ("Running '%s'", commandline);
 
- /* We use LEAVE_DESCRIPTORS_OPEN to work around dead-lock, see flatpak_close_fds_workaround */
+ /* We use LEAVE_DESCRIPTORS_OPEN and close them in the child_setup
+ * to work around a deadlock in GLib < 2.60 */
  if (!g_spawn_sync ("/",
  (char **) bwrap->argv->pdata,
  NULL,

common/flatpak-run-dbus.c

@@ -161,7 +161,8 @@ flatpak_run_maybe_start_dbus_proxy (FlatpakBwrap *app_bwrap,
  commandline = flatpak_quote_argv ((const char **) proxy_bwrap->argv->pdata, -1);
  g_info ("Running '%s'", commandline);
 
- /* We use LEAVE_DESCRIPTORS_OPEN to work around dead-lock, see flatpak_close_fds_workaround */
+ /* We use LEAVE_DESCRIPTORS_OPEN and close them in the child_setup
+ * to work around a deadlock in GLib < 2.60 */
  if (!g_spawn_async (NULL,
  (char **) proxy_bwrap->argv->pdata,
  NULL,

common/flatpak-run.c

@@ -2643,7 +2643,8 @@ regenerate_ld_cache (GPtrArray *base_argv_array,
  g_array_append_vals (combined_fd_array, base_fd_array->data, base_fd_array->len);
  g_array_append_vals (combined_fd_array, bwrap->fds->data, bwrap->fds->len);
 
- /* We use LEAVE_DESCRIPTORS_OPEN to work around dead-lock, see flatpak_close_fds_workaround */
+ /* We use LEAVE_DESCRIPTORS_OPEN and close them in the child_setup
+ * to work around a deadlock in GLib < 2.60 */
  if (!g_spawn_sync (NULL,
  (char **) bwrap->argv->pdata,
  bwrap->envp,
@@ -3453,7 +3454,9 @@ flatpak_run_app (FlatpakDecomposed *app_ref,
  else
  child_setup = flatpak_bwrap_child_setup_inherit_fds_cb;
 
- /* We use LEAVE_DESCRIPTORS_OPEN to work around dead-lock, see flatpak_close_fds_workaround */
+ /* Even in the case where we want them closed, we use
+ * LEAVE_DESCRIPTORS_OPEN and close them in the child_setup
+ * to work around a deadlock in GLib < 2.60 */
  spawn_flags |= G_SPAWN_LEAVE_DESCRIPTORS_OPEN;
 
  /* flatpak_bwrap_envp_to_args() moved the environment variables to

common/flatpak-utils-base-private.h

@@ -36,6 +36,5 @@ char * flatpak_readlink (const char *path,
 char * flatpak_resolve_link (const char *path,
  GError **error);
 char * flatpak_canonicalize_filename (const char *path);
-void flatpak_close_fds_workaround (int start_fd);
 
 #endif /* __FLATPAK_UTILS_BASE_H__ */

common/flatpak-utils-base.c

@@ -112,19 +112,3 @@ flatpak_canonicalize_filename (const char *path)
  g_autoptr(GFile) file = g_file_new_for_path (path);
  return g_file_get_path (file);
 }
-
-/* There is a dead-lock in glib versions before 2.60 when it closes
- * the fds. See: https://gitlab.gnome.org/GNOME/glib/merge_requests/490
- * This was hitting the test-suite a lot, so we work around it by using
- * the G_SPAWN_LEAVE_DESCRIPTORS_OPEN/G_SUBPROCESS_FLAGS_INHERIT_FDS flag
- * and setting CLOEXEC ourselves.
- */
-void
-flatpak_close_fds_workaround (int start_fd)
-{
- int max_open_fds = sysconf (_SC_OPEN_MAX);
- int fd;
-
- for (fd = start_fd; fd < max_open_fds; fd++)
- fcntl (fd, F_SETFD, FD_CLOEXEC);
-}

portal/flatpak-portal.c

@@ -482,7 +482,7 @@ child_setup_func (gpointer user_data)
  sigset_t set;
  gsize i;
 
- flatpak_close_fds_workaround (3);
+ g_fdwalk_set_cloexec (3);
 
  if (data->instance_id_fd != -1)
  drop_cloexec (data->instance_id_fd);
@@ -1518,7 +1518,8 @@ handle_spawn (PortalFlatpak *object,
  child_setup_data.fd_map = &g_array_index (fd_map, FdMapEntry, 0);
  child_setup_data.fd_map_len = fd_map->len;
 
- /* We use LEAVE_DESCRIPTORS_OPEN to work around dead-lock, see flatpak_close_fds_workaround */
+ /* We use LEAVE_DESCRIPTORS_OPEN and close them in the child_setup
+ * to work around a deadlock in GLib < 2.60 */
  if (!g_spawn_async_with_pipes (NULL,
  (char **) flatpak_argv->pdata,
  env,
@@ -2612,7 +2613,7 @@ update_child_setup_func (gpointer user_data)
  int *socket = user_data;
 
  dup2 (*socket, 3);
- flatpak_close_fds_workaround (4);
+ g_fdwalk_set_cloexec (4);
 }
 
 /* This is the meat of the update process, its run out of process (via

system-helper/flatpak-system-helper.c

@@ -1495,7 +1495,7 @@ revokefs_fuse_backend_child_setup (gpointer user_data)
  /* We use 5 instead of 3 here, because fd 3 is the inherited SOCK_SEQPACKET
  * socket and fd 4 is the --close-with-fd pipe; both were dup2()'d into place
  * before this by GSubprocess */
- flatpak_close_fds_workaround (5);
+ g_fdwalk_set_cloexec (5);
 
  if (setgid (passwd->pw_gid) == -1)
  {
@@ -1580,7 +1580,8 @@ ongoing_pull_new (FlatpakSystemHelper *object,
  return NULL;
  }
 
- /* We use INHERIT_FDS to work around dead-lock, see flatpak_close_fds_workaround */
+ /* We use INHERIT_FDS and close them in the child_setup
+ * to work around a deadlock in GLib < 2.60 */
  launcher = g_subprocess_launcher_new (G_SUBPROCESS_FLAGS_INHERIT_FDS);
  g_subprocess_launcher_set_child_setup (launcher, revokefs_fuse_backend_child_setup, passwd, NULL);
  g_subprocess_launcher_take_fd (launcher, sockets[0], 3);