From 9ce4ea8d312316a5747877757dbe8881bdd56df0 Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Thu, 15 Oct 2015 15:01:34 -0400 Subject: [PATCH] Fix deadlock in gnome-keyring when using autologin Resolves: #1269581 --- fix-autologin.patch | 456 ++++++++++++++++++++++++++++++++++++++++++++ gnome-keyring.spec | 9 +- 2 files changed, 464 insertions(+), 1 deletion(-) create mode 100644 fix-autologin.patch diff --git a/fix-autologin.patch b/fix-autologin.patch new file mode 100644 index 0000000..a0efa4a --- /dev/null +++ b/fix-autologin.patch @@ -0,0 +1,456 @@ +From eb6d8d221b34a93e57c22cefa47d924350251c4c Mon Sep 17 00:00:00 2001 +From: Ray Strode +Date: Thu, 15 Oct 2015 14:37:33 -0400 +Subject: [PATCH] daemon: fork before threads are spawned + +It's not really a good idea to fork after glib has initialized, +since it has helper threads that may have taken locks etc. + +This commit forks really early to prevent locks from leaking +and causing deadlock. +--- + daemon/gkd-main.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++--------- + 1 file changed, 63 insertions(+), 12 deletions(-) + +diff --git a/daemon/gkd-main.c b/daemon/gkd-main.c +index f567633..4cc8552 100644 +--- a/daemon/gkd-main.c ++++ b/daemon/gkd-main.c +@@ -98,60 +98,61 @@ EGG_SECURE_DECLARE (daemon_main); + # else + # define DEFAULT_COMPONENTS GKD_COMP_PKCS11 "," GKD_COMP_SECRETS + # endif + #endif + + /* + * If --login is used and then daemon is not initialized within LOGIN_TIMEOUT + * seconds, then we exit. See on_login_timeout() below. + */ + + #define LOGIN_TIMEOUT 120 + + static gchar* run_components = DEFAULT_COMPONENTS; + static gboolean pkcs11_started = FALSE; + static gboolean secrets_started = FALSE; + static gboolean ssh_started = FALSE; + static gboolean dbus_started = FALSE; + + static gboolean run_foreground = FALSE; + static gboolean run_daemonized = FALSE; + static gboolean run_version = FALSE; + static gboolean run_for_login = FALSE; + static gboolean perform_unlock = FALSE; + static gboolean run_for_start = FALSE; + static gboolean run_for_replace = FALSE; + static gchar* login_password = NULL; + static gchar* control_directory = NULL; + static guint timeout_id = 0; + static gboolean initialization_completed = FALSE; + static GMainLoop *loop = NULL; ++static int parent_wakeup_fd = -1; + + static GOptionEntry option_entries[] = { + { "start", 's', 0, G_OPTION_ARG_NONE, &run_for_start, + "Start a dameon or initialize an already running daemon." }, + { "replace", 'r', 0, G_OPTION_ARG_NONE, &run_for_replace, + "Replace the daemon for this desktop login environment." }, + { "foreground", 'f', 0, G_OPTION_ARG_NONE, &run_foreground, + "Run in the foreground", NULL }, + { "daemonize", 'd', 0, G_OPTION_ARG_NONE, &run_daemonized, + "Run as a daemon", NULL }, + { "login", 'l', 0, G_OPTION_ARG_NONE, &run_for_login, + "Run by PAM for a user login. Read login password from stdin", NULL }, + { "unlock", 0, 0, G_OPTION_ARG_NONE, &perform_unlock, + "Prompt for login keyring password, or read from stdin", NULL }, + { "components", 'c', 0, G_OPTION_ARG_STRING, &run_components, + "The optional components to run", DEFAULT_COMPONENTS }, + { "control-directory", 'C', 0, G_OPTION_ARG_FILENAME, &control_directory, + "The directory for sockets and control data", NULL }, + { "version", 'V', 0, G_OPTION_ARG_NONE, &run_version, + "Show the version number and exit.", NULL }, + { NULL } + }; + + static void + parse_arguments (int *argc, char** argv[]) + { + GError *err = NULL; + GOptionContext *context; + + context = g_option_context_new ("- The Gnome Keyring Daemon"); +@@ -474,60 +475,110 @@ read_login_password (int fd) + } + + egg_secure_free (buf); + return ret; + } + + static void + cleanup_and_exit (int code) + { + egg_cleanup_perform (); + exit (code); + } + + static void + clear_login_password (void) + { + if(login_password) + egg_secure_strfree (login_password); + login_password = NULL; + } + + static void + print_environment (void) + { + const gchar **env; + for (env = gkd_util_get_environment (); *env; ++env) + printf ("%s\n", *env); + fflush (stdout); + } + ++ ++static void ++print_environment_from_fd (int fd) ++{ ++ char *output; ++ gsize output_size; ++ gsize bytes_read; ++ ++ bytes_read = read (fd, &output_size, sizeof (output_size)); ++ ++ if (bytes_read < sizeof (output_size)) ++ exit (1); ++ ++ output = g_malloc0 (output_size); ++ bytes_read = read (fd, output, output_size); ++ ++ if (bytes_read < output_size) ++ exit (1); ++ ++ printf ("%s\n", output); ++ fflush (stdout); ++ g_free (output); ++} ++ ++static void ++send_environment_and_finish_parent (int fd) ++{ ++ char *output; ++ gsize output_size; ++ gsize bytes_written; ++ ++ if (fd < 0) { ++ print_environment (); ++ return; ++ } ++ ++ output = g_strjoinv ("\n", (gchar **) gkd_util_get_environment ()); ++ output_size = strlen (output) + 1; ++ bytes_written = write (fd, &output_size, sizeof (output_size)); ++ ++ if (bytes_written < sizeof (output_size)) ++ exit (1); ++ ++ bytes_written = write (fd, output, output_size); ++ if (bytes_written < output_size) ++ exit (1); ++ ++ g_free (output); ++} ++ + static gboolean + initialize_daemon_at (const gchar *directory) + { + gchar **ourenv, **daemonenv, **e; + + /* Exchange environment variables, and try to initialize daemon */ + ourenv = gkd_util_build_environment (GKD_UTIL_IN_ENVIRONMENT); + daemonenv = gkd_control_initialize (directory, run_components, + (const gchar**)ourenv); + g_strfreev (ourenv); + + /* Initialization failed, start this process up as a daemon */ + if (!daemonenv) + return FALSE; + + /* Setup all the environment variables we were passed */ + for (e = daemonenv; *e; ++e) + gkd_util_push_environment_full (*e); + g_strfreev (daemonenv); + + return TRUE; + } + + static gboolean + replace_daemon_at (const gchar *directory) + { + gboolean ret; + + /* + * The first control_directory is the environment one, always +@@ -577,136 +628,134 @@ discover_other_daemon (DiscoverFunc callback, gboolean acquire) + + /* Or the default location when no evironment variable */ + control_env = g_getenv ("XDG_RUNTIME_DIR"); + if (control_env) { + control = g_build_filename (control_env, "keyring", NULL); + ret = (callback) (control); + g_free (control); + if (ret == TRUE) + return TRUE; + } + + /* See if we can contact a daemon running, that didn't set an env variable */ + if (acquire && !gkd_dbus_singleton_acquire (&acquired)) + return FALSE; + + /* We're the main daemon */ + if (acquired) + return FALSE; + + control = gkd_dbus_singleton_control (); + if (control) { + ret = (callback) (control); + g_free (control); + if (ret == TRUE) + return TRUE; + } + + return FALSE; + } + +-static void ++static int + fork_and_print_environment (void) + { + int status; + pid_t pid; + int fd, i; ++ int wakeup_fds[2] = { -1, -1 }; + +- if (run_foreground) { +- print_environment (); +- return; +- } ++ g_unix_open_pipe (wakeup_fds, FD_CLOEXEC, NULL); + + pid = fork (); + + if (pid != 0) { +- + /* Here we are in the initial process */ + + if (run_daemonized) { + + /* Initial process, waits for intermediate child */ + if (pid == -1) + exit (1); + + waitpid (pid, &status, 0); + if (WEXITSTATUS (status) != 0) + exit (WEXITSTATUS (status)); + + } else { + /* Not double forking */ +- print_environment (); ++ print_environment_from_fd (wakeup_fds[0]); + } + + /* The initial process exits successfully */ + exit (0); + } + + if (run_daemonized) { + + /* + * Become session leader of a new session, process group leader of a new + * process group, and detach from the controlling TTY, so that SIGHUP is + * not sent to this process when the previous session leader dies + */ + setsid (); + + /* Double fork if need to daemonize properly */ + pid = fork (); + + if (pid != 0) { +- + /* Here we are in the intermediate child process */ + + /* + * This process exits, so that the final child will inherit + * init as parent to avoid zombies + */ + if (pid == -1) + exit (1); + + /* We've done two forks. */ +- print_environment (); ++ print_environment_from_fd (wakeup_fds[0]); + + /* The intermediate child exits */ + exit (0); + } + + } + + /* Here we are in the resulting daemon or background process. */ + + for (i = 0; i < 3; ++i) { + fd = open ("/dev/null", O_RDONLY); + sane_dup2 (fd, i); + close (fd); + } ++ ++ return wakeup_fds[1]; + } + + static gboolean + gkr_daemon_startup_steps (const gchar *components) + { + g_assert (components); + + /* + * Startup that must run before forking. + * Note that we set initialized flags early so that two + * initializations don't overlap + */ + + #ifdef WITH_SSH + if (strstr (components, GKD_COMP_SSH)) { + if (ssh_started) { + g_message ("The SSH agent was already initialized"); + } else { + ssh_started = TRUE; + if (!gkd_daemon_startup_ssh ()) { + ssh_started = FALSE; + return FALSE; + } + } + } + #endif + + return TRUE; + } + +@@ -849,112 +898,114 @@ main (int argc, char *argv[]) + #ifdef HAVE_LOCALE_H + /* internationalisation */ + setlocale (LC_ALL, ""); + #endif + + #ifdef HAVE_GETTEXT + bindtextdomain (GETTEXT_PACKAGE, GNOMELOCALEDIR); + textdomain (GETTEXT_PACKAGE); + bind_textdomain_codeset (GETTEXT_PACKAGE, "UTF-8"); + #endif + + egg_libgcrypt_initialize (); + + /* Send all warning or error messages to syslog */ + prepare_logging (); + + parse_arguments (&argc, &argv); + + /* The --version option. This is machine parseable output */ + if (run_version) { + g_print ("gnome-keyring-daemon: %s\n", VERSION); + g_print ("testing: %s\n", + #ifdef WITH_DEBUG + "enabled"); + #else + "disabled"); + #endif + exit (0); + } + ++ /* The whole forking and daemonizing dance starts here. */ ++ parent_wakeup_fd = fork_and_print_environment(); ++ + /* The --start option */ + if (run_for_start) { + if (discover_other_daemon (initialize_daemon_at, TRUE)) { + /* + * Another daemon was initialized, print out environment + * for any callers, and quit or go comatose. + */ +- print_environment (); ++ send_environment_and_finish_parent (parent_wakeup_fd); + if (run_foreground) + while (sleep(0x08000000) == 0); + cleanup_and_exit (0); + } + + /* The --replace option */ + } else if (run_for_replace) { + discover_other_daemon (replace_daemon_at, FALSE); + if (control_directory) + g_message ("Replacing daemon, using directory: %s", control_directory); + else + g_message ("Could not find daemon to replace, staring normally"); + } + + /* Initialize the main directory */ + gkd_util_init_master_directory (control_directory); + + /* Initialize our daemon main loop and threading */ + loop = g_main_loop_new (NULL, FALSE); + + /* Initialize our control socket */ + if (!gkd_control_listen ()) + return FALSE; + + if (perform_unlock) { + login_password = read_login_password (STDIN); + atexit (clear_login_password); + } + + /* The --login option. Delayed initialization */ + if (run_for_login) { + timeout_id = g_timeout_add_seconds (LOGIN_TIMEOUT, (GSourceFunc) on_login_timeout, NULL); + + /* Not a login daemon. Startup stuff now.*/ + } else { + /* These are things that can run before forking */ + if (!gkr_daemon_startup_steps (run_components)) + cleanup_and_exit (1); + } + + signal (SIGPIPE, SIG_IGN); + +- /* The whole forking and daemonizing dance starts here. */ +- fork_and_print_environment(); ++ send_environment_and_finish_parent (parent_wakeup_fd); + + g_unix_signal_add (SIGTERM, on_signal_term, loop); + g_unix_signal_add (SIGHUP, on_signal_term, loop); + g_unix_signal_add (SIGUSR1, on_signal_usr1, loop); + + /* Prepare logging a second time, since we may be in a different process */ + prepare_logging(); + + /* Remainder initialization after forking, if initialization not delayed */ + if (!run_for_login) { + gkr_daemon_initialize_steps (run_components); + + /* + * Close stdout and so that the caller knows that we're + * all initialized, (when run in foreground mode). + * + * However since some logging goes to stdout, redirect that + * to stderr. We don't want the caller confusing that with + * valid output anyway. + */ + if (dup2 (2, 1) < 1) + g_warning ("couldn't redirect stdout to stderr"); + + g_debug ("initialization complete"); + } + + g_main_loop_run (loop); + + /* This wraps everything up in order */ + egg_cleanup_perform (); +-- +2.5.0 + diff --git a/gnome-keyring.spec b/gnome-keyring.spec index 4dee69f..7207525 100644 --- a/gnome-keyring.spec +++ b/gnome-keyring.spec @@ -6,12 +6,13 @@ Summary: Framework for managing passwords and other secrets Name: gnome-keyring Version: 3.18.0 -Release: 2%{?dist} +Release: 3%{?dist} License: GPLv2+ and LGPLv2+ Group: System Environment/Libraries #VCS: git:git://git.gnome.org/gnome-keyring Source: https://download.gnome.org/sources/%{name}/3.15/%{name}-%{version}.tar.xz Patch0: 0001-dbus-Initialize-secret-service-before-claiming-name.patch +Patch1: fix-autologin.patch URL: https://wiki.gnome.org/Projects/GnomeKeyring BuildRequires: pkgconfig(dbus-1) >= %{dbus_version} @@ -53,6 +54,7 @@ automatically unlock the "login" keyring when the user logs in. %prep %setup -q -n gnome-keyring-%{version} %patch0 -p1 +%patch1 -p1 %build %configure \ @@ -70,6 +72,7 @@ make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p" rm $RPM_BUILD_ROOT%{_libdir}/security/*.la rm $RPM_BUILD_ROOT%{_libdir}/pkcs11/*.la rm $RPM_BUILD_ROOT%{_libdir}/gnome-keyring/devel/*.la +rm -f $RPM_BUILD_ROOT%{_sysconfdir}/xdg/autostart/gnome-keyring-secrets.desktop %find_lang gnome-keyring @@ -109,6 +112,10 @@ glib-compile-schemas %{_datadir}/glib-2.0/schemas >&/dev/null || : %changelog +* Thu Oct 15 2015 Ray Strode 3.18.0-3 +- Fix deadlock in gnome-keyring when using autologin + Resolves: #1269581 + * Wed Oct 14 2015 Kalev Lember - 3.18.0-2 - dbus: Initialize secret service before claiming name