mirror/flatpak
1
0
Fork 0
mirror of https://github.com/flatpak/flatpak.git synced 2026-05-24 08:51:27 -04:00
Code Issues Packages Projects Releases Wiki Activity

Work around deadlocks in g_spawn by manually clo-exec:ing fds

Browse Source 
As per https://gitlab.gnome.org/GNOME/glib/merge_requests/490
there is a bug in glib < 2.60 where g_spawn_* can sometimes deadlock
due to using malloc in the child func to close fds.

We work around this in places where the code is (potentially) threaded
by passing glib flags to leave fds alone and then do a very naive
(but safe) fd cloexec loop ourselves.
This commit is contained in:
Alexander Larsson
2019-09-24 09:00:54 +02:00
committed by Alexander Larsson
parent 9c715096aa
commit d91660fe2a
8 changed files with 43 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
common/flatpak-bwrap.c
View File

@@ -39,6 +39,7 @@
#include "flatpak-bwrap-private.h"
#include "flatpak-utils-private.h"
#include "flatpak-utils-base-private.h"
static void
clear_fd (gpointer data)
@@ -331,6 +332,8 @@ flatpak_bwrap_child_setup_cb (gpointer user_data)
GArray *fd_array = user_data;
int i;
flatpak_close_fds_workaround (3);
/* If no fd_array was specified, don't care. */
if (fd_array == NULL)
return;

3
common/flatpak-dir.c
View File

@@ -6281,10 +6281,11 @@ flatpak_dir_run_triggers (FlatpakDir *self,
commandline = flatpak_quote_argv ((const char **) bwrap->argv->pdata, -1);
g_debug ("Running '%s'", commandline);
/* We use LEAVE_DESCRIPTORS_OPEN to work around dead-lock, see flatpak_close_fds_workaround */
if (!g_spawn_sync ("/",
(char **) bwrap->argv->pdata,
NULL,
G_SPAWN_SEARCH_PATH,
G_SPAWN_SEARCH_PATH | G_SPAWN_LEAVE_DESCRIPTORS_OPEN,
flatpak_bwrap_child_setup_cb, bwrap->fds,
NULL, NULL,
NULL, &trigger_error))

9
common/flatpak-run.c
View File

@@ -851,10 +851,11 @@ start_dbus_proxy (FlatpakBwrap *app_bwrap,
commandline = flatpak_quote_argv ((const char **) proxy_bwrap->argv->pdata, -1);
g_debug ("Running '%s'", commandline);
/* We use LEAVE_DESCRIPTORS_OPEN to work around dead-lock, see flatpak_close_fds_workaround */
if (!g_spawn_async (NULL,
(char **) proxy_bwrap->argv->pdata,
NULL,
G_SPAWN_SEARCH_PATH,
G_SPAWN_SEARCH_PATH | G_SPAWN_LEAVE_DESCRIPTORS_OPEN,
flatpak_bwrap_child_setup_cb, proxy_bwrap->fds,
NULL, error))
return FALSE;
@@ -3122,10 +3123,11 @@ 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 */
if (!g_spawn_sync (NULL,
(char **) bwrap->argv->pdata,
bwrap->envp,
G_SPAWN_SEARCH_PATH,
G_SPAWN_SEARCH_PATH | G_SPAWN_LEAVE_DESCRIPTORS_OPEN,
flatpak_bwrap_child_setup_cb, combined_fd_array,
NULL, NULL,
&exit_status,
@@ -3564,6 +3566,9 @@ flatpak_run_app (const char *app_ref,
if (flags & FLATPAK_RUN_FLAG_DO_NOT_REAP)
spawn_flags |= G_SPAWN_DO_NOT_REAP_CHILD;
/* We use LEAVE_DESCRIPTORS_OPEN to work around dead-lock, see flatpak_close_fds_workaround */
spawn_flags |= G_SPAWN_LEAVE_DESCRIPTORS_OPEN;
if (!g_spawn_async (NULL,
(char **) bwrap->argv->pdata,
bwrap->envp,

1
common/flatpak-utils-base-private.h
View File

@@ -30,5 +30,6 @@ 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__ */

16
common/flatpak-utils-base.c
View File

@@ -98,3 +98,19 @@ 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);
}

2
portal/Makefile.am.inc
View File

@@ -34,6 +34,6 @@ flatpak_portal_SOURCES = \
BUILT_SOURCES += $(nodist_flatpak_portal_SOURCES)
CLEANFILES += $(nodist_flatpak_portal_SOURCES)
flatpak_portal_LDADD = $(AM_LDADD) $(BASE_LIBS)
flatpak_portal_LDADD = $(AM_LDADD) $(BASE_LIBS) libflatpak-common-base.la libglnx.la
flatpak_portal_CFLAGS = $(AM_CFLAGS) $(BASE_CFLAGS) -DFLATPAK_COMPILATION
flatpak_portal_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/portal

6
portal/flatpak-portal.c
View File

@@ -32,6 +32,7 @@
#include "flatpak-portal.h"
#include "flatpak-portal-app-info.h"
#include "flatpak-portal-error.h"
#include "flatpak-utils-base-private.h"
#define IDLE_TIMEOUT_SECS 10 * 60
@@ -176,6 +177,8 @@ child_setup_func (gpointer user_data)
sigset_t set;
int i;
flatpak_close_fds_workaround (3);
/* Unblock all signals */
sigemptyset (&set);
if (pthread_sigmask (SIG_SETMASK, &set, NULL) == -1)
@@ -591,10 +594,11 @@ handle_spawn (PortalFlatpak *object,
g_debug ("Starting: %s\n", cmd->str);
}
/* We use LEAVE_DESCRIPTORS_OPEN to work around dead-lock, see flatpak_close_fds_workaround */
if (!g_spawn_async_with_pipes (NULL,
(char **) flatpak_argv->pdata,
env,
G_SPAWN_SEARCH_PATH | G_SPAWN_DO_NOT_REAP_CHILD,
G_SPAWN_SEARCH_PATH | G_SPAWN_DO_NOT_REAP_CHILD | G_SPAWN_LEAVE_DESCRIPTORS_OPEN,
child_setup_func, &child_setup_data,
&pid,
NULL,

9
system-helper/flatpak-system-helper.c
View File

@@ -32,12 +32,14 @@
#include <gio/gunixfdlist.h>
#include <sys/mount.h>
#include <fcntl.h>
#include <unistd.h>
#include "flatpak-dbus-generated.h"
#include "flatpak-dir-private.h"
#include "flatpak-oci-registry-private.h"
#include "flatpak-error.h"
#include "flatpak-utils-private.h"
#include "flatpak-utils-base-private.h"
static PolkitAuthority *authority = NULL;
static FlatpakSystemHelper *helper = NULL;
@@ -1465,6 +1467,10 @@ revokefs_fuse_backend_child_setup (gpointer user_data)
{
struct passwd *passwd = user_data;
/* We use 4 instead of 3 here, because fd 3 is the inerited socket
and got dup2() into place before this by GSubprocess */
flatpak_close_fds_workaround (4);
if (setgid (passwd->pw_gid) == -1)
{
g_warning ("Failed to setgid(%d) for revokefs backend: %s",
@@ -1548,7 +1554,8 @@ ongoing_pull_new (FlatpakSystemHelper *object,
return NULL;
}
launcher = g_subprocess_launcher_new (G_SUBPROCESS_FLAGS_NONE);
/* We use INHERIT_FDS to work around dead-lock, see flatpak_close_fds_workaround */
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);
fcntl (sockets[1], F_SETFD, FD_CLOEXEC);