diff options
Diffstat (limited to 'working/keyring-crash/gnome-keyring-51.1-secret-service-missing-client.patch')
| -rw-r--r-- | working/keyring-crash/gnome-keyring-51.1-secret-service-missing-client.patch | 275 |
1 files changed, 275 insertions, 0 deletions
diff --git a/working/keyring-crash/gnome-keyring-51.1-secret-service-missing-client.patch b/working/keyring-crash/gnome-keyring-51.1-secret-service-missing-client.patch new file mode 100644 index 0000000..9ef298b --- /dev/null +++ b/working/keyring-crash/gnome-keyring-51.1-secret-service-missing-client.patch @@ -0,0 +1,275 @@ +diff -ruN a/daemon/dbus/gkd-secret-objects.c b/daemon/dbus/gkd-secret-objects.c +--- a/daemon/dbus/gkd-secret-objects.c 2026-09-17 15:41:00.000000000 -0500 ++++ b/daemon/dbus/gkd-secret-objects.c 2026-10-17 06:47:19.275009074 -0500 +@@ -430,7 +430,11 @@ + + /* The session we're using to access the object */ + session = gkd_secret_service_get_pkcs11_session (self->service, sender); +- g_return_val_if_fail (session, FALSE); ++ if (session == NULL) { ++ g_free (c_ident); ++ g_free (i_ident); ++ goto out; ++ } + + if (i_ident) { + gck_builder_add_ulong (&builder, CKA_CLASS, CKO_SECRET_KEY); +@@ -1055,17 +1059,23 @@ + if (!gkd_secret_util_parse_path (path, &identifier, NULL)) + return NULL; + +- if (g_str_has_prefix (path, SECRET_ALIAS_PREFIX)) +- real_identifier = gkd_secret_service_get_alias (self->service, identifier); +- else +- real_identifier = identifier; +- +- /* The session we're using to access the object */ ++ /* The session we're using to access the object. Resolve it before the ++ * alias: fetching a caller's session may register the caller, which ++ * refreshes the default alias and frees the string the alias table ++ * held, so an alias looked up earlier would dangle here. */ + if (caller == NULL) + session = gkd_secret_service_internal_pkcs11_session (self->service); + else + session = gkd_secret_service_get_pkcs11_session (self->service, caller); +- g_return_val_if_fail (session, NULL); ++ if (session == NULL) { ++ g_free (identifier); ++ return NULL; ++ } ++ ++ if (g_str_has_prefix (path, SECRET_ALIAS_PREFIX)) ++ real_identifier = gkd_secret_service_get_alias (self->service, identifier); ++ else ++ real_identifier = identifier; + + gck_builder_add_ulong (&builder, CKA_CLASS, CKO_G_COLLECTION); + gck_builder_add_string (&builder, CKA_ID, real_identifier); +diff -ruN a/daemon/dbus/gkd-secret-service.c b/daemon/dbus/gkd-secret-service.c +--- a/daemon/dbus/gkd-secret-service.c 2026-09-17 15:41:00.000000000 -0500 ++++ b/daemon/dbus/gkd-secret-service.c 2026-10-17 06:47:19.274715149 -0500 +@@ -137,6 +137,9 @@ + GHashTable *clients; + GkdSecretObjects *objects; + GHashTable *aliases; ++ /* The message filter reads aliases on the GDBus worker thread while ++ * the main thread updates them; every access goes through this lock. */ ++ GMutex aliases_lock; + GckSession *internal_session; + gchar *default_path; + }; +@@ -190,6 +193,18 @@ + return g_build_filename (alias_directory, "default", NULL); + } + ++/* An owned copy of an alias, safe to call from any thread. */ ++static gchar * ++lookup_alias_copy (GkdSecretService *self, const gchar *alias) ++{ ++ gchar *copy; ++ ++ g_mutex_lock (&self->aliases_lock); ++ copy = g_strdup (g_hash_table_lookup (self->aliases, alias)); ++ g_mutex_unlock (&self->aliases_lock); ++ return copy; ++} ++ + static void + update_default (GkdSecretService *self) + { +@@ -206,21 +221,31 @@ + /* Default to to 'login' if no default keyring */ + if (contents == NULL) + contents = g_strdup ("login"); +- g_hash_table_replace (self->aliases, g_strdup ("default"), contents); ++ ++ /* Replace only on a real change: callers on the main thread hold the ++ * previous value borrowed, and the common case (another client ++ * registering) must not free it out from under them. */ ++ g_mutex_lock (&self->aliases_lock); ++ if (g_strcmp0 (g_hash_table_lookup (self->aliases, "default"), contents) == 0) ++ g_free (contents); ++ else ++ g_hash_table_replace (self->aliases, g_strdup ("default"), contents); ++ g_mutex_unlock (&self->aliases_lock); + } + + static void + store_default (GkdSecretService *self) + { + GError *error = NULL; +- const gchar *identifier; ++ gchar *identifier; + +- identifier = g_hash_table_lookup (self->aliases, "default"); ++ identifier = lookup_alias_copy (self, "default"); + if (!identifier) + return; + + if (!g_file_set_contents (self->default_path, identifier, -1, &error)) + g_message ("couldn't store default keyring: %s", egg_error_message (error)); ++ g_free (identifier); + } + + static gboolean +@@ -331,7 +356,7 @@ + GDBusMessage *message) + { + const char *path = g_dbus_message_get_path (message); +- const char *replace; ++ char *replace; + char *collection = NULL, *item = NULL; + char *collection_path, *item_path; + GDBusMessage *rewritten; +@@ -346,7 +371,9 @@ + if (!gkd_secret_util_parse_path (path, &collection, &item)) + return message; + +- replace = gkd_secret_service_get_alias (self, collection); ++ /* This runs on the GDBus worker thread: take a copy under the lock ++ * rather than borrowing from a table the main thread may be updating. */ ++ replace = lookup_alias_copy (self, collection); + if (!replace) { + g_free (item); + g_free (collection); +@@ -356,6 +383,9 @@ + rewritten = g_dbus_message_copy (message, &error); + if (error != NULL) { + g_error_free (error); ++ g_free (replace); ++ g_free (item); ++ g_free (collection); + return message; + } + +@@ -372,6 +402,7 @@ + } + + g_free (collection_path); ++ g_free (replace); + g_free (item); + g_free (collection); + g_object_unref (message); +@@ -424,18 +455,30 @@ + GError *error = NULL; + const gchar *caller; + GVariant *input_payload; ++ gboolean opened; + + caller = g_dbus_method_invocation_get_sender (invocation); + + /* Now we can create a session with this information */ + session = gkd_secret_session_new (self, caller); + input_payload = g_variant_get_variant (input); +- gkd_secret_session_handle_open (session, algorithm, input_payload, +- &output, &result, +- &error); ++ opened = gkd_secret_session_handle_open (session, algorithm, input_payload, ++ &output, &result, ++ &error); + g_variant_unref (input_payload); + +- if (error != NULL) { ++ /* Branch on the return value, not on the error alone: a negotiate helper ++ * that bails out through g_return_val_if_fail() reports failure without ++ * ever setting one, and completing the call with a NULL body makes ++ * g_variant_new() abort the whole daemon. */ ++ if (!opened || output == NULL || result == NULL) { ++ if (error == NULL) ++ g_set_error_literal (&error, G_DBUS_ERROR, ++ G_DBUS_ERROR_FAILED, ++ "Couldn't open a session for this caller."); ++ if (output != NULL) ++ g_variant_unref (g_variant_ref_sink (output)); ++ g_free (result); + g_dbus_method_invocation_take_error (invocation, error); + } else { + gkd_secret_service_publish_dispatch (self, caller, +@@ -986,6 +1029,7 @@ + static void + gkd_secret_service_init_aliases (GkdSecretService *self) + { ++ g_mutex_init (&self->aliases_lock); + self->aliases = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); + g_hash_table_insert (self->aliases, g_strdup ("session"), g_strdup ("session")); + /* TODO: We should be using CKA_G_LOGIN_COLLECTION */ +@@ -1151,6 +1195,7 @@ + + g_hash_table_destroy (self->aliases); + self->aliases = NULL; ++ g_mutex_clear (&self->aliases_lock); + + g_free (self->default_path); + self->default_path = NULL; +@@ -1283,7 +1328,18 @@ + g_return_val_if_fail (caller, NULL); + + client = g_hash_table_lookup (self->clients, caller); +- g_return_val_if_fail (client, NULL); ++ if (client == NULL) { ++ /* The record is normally created by a high priority idle queued from ++ * the message filter, but that idle is not ordered against the method ++ * call that queued it: a peer which connects and calls straight away ++ * can reach us first. Create it here instead of failing the call -- ++ * it is a hash table insert, not I/O. Callers must not hold borrowed ++ * alias strings across this call: registering a client refreshes the ++ * default alias. */ ++ gkd_secret_service_ensure_client (self, caller); ++ client = g_hash_table_lookup (self->clients, caller); ++ g_return_val_if_fail (client, NULL); ++ } + + /* Open a new session if necessary */ + if (!client->pkcs11_session) { +@@ -1399,7 +1455,9 @@ + g_return_if_fail (GKD_SECRET_IS_SERVICE (self)); + g_return_if_fail (alias); + ++ g_mutex_lock (&self->aliases_lock); + g_hash_table_replace (self->aliases, g_strdup (alias), g_strdup (identifier)); ++ g_mutex_unlock (&self->aliases_lock); + + if (g_str_equal (alias, "default")) + store_default (self); +@@ -1418,6 +1476,10 @@ + + /* Take ownership of the session */ + client = g_hash_table_lookup (self->clients, caller); ++ if (client == NULL) { ++ gkd_secret_service_ensure_client (self, caller); ++ client = g_hash_table_lookup (self->clients, caller); ++ } + g_return_if_fail (client); + path = gkd_secret_dispatch_get_object_path (object); + g_return_if_fail (!g_hash_table_lookup (client->dispatch, path)); +diff -ruN a/daemon/dbus/gkd-secret-session.c b/daemon/dbus/gkd-secret-session.c +--- a/daemon/dbus/gkd-secret-session.c 2026-09-17 15:41:00.000000000 -0500 ++++ b/daemon/dbus/gkd-secret-session.c 2026-10-17 06:47:19.270295281 -0500 +@@ -183,7 +183,12 @@ + gboolean ret; + + session = gkd_secret_service_get_pkcs11_session (self->service, self->caller); +- g_return_val_if_fail (session, FALSE); ++ if (session == NULL) { ++ g_set_error_literal (error_out, G_DBUS_ERROR, ++ G_DBUS_ERROR_FAILED, ++ "Couldn't open a session for this caller."); ++ return FALSE; ++ } + + if (!aes_create_dh_keys (session, "ietf-ike-grp-modp-1024", &pub, &priv)) { + g_set_error_literal (error_out, G_DBUS_ERROR, +@@ -250,7 +255,12 @@ + GckSession *session; + + session = gkd_secret_service_get_pkcs11_session (self->service, self->caller); +- g_return_val_if_fail (session, FALSE); ++ if (session == NULL) { ++ g_set_error_literal (error_out, G_DBUS_ERROR, ++ G_DBUS_ERROR_FAILED, ++ "Couldn't open a session for this caller."); ++ return FALSE; ++ } + + gck_builder_add_ulong (&builder, CKA_CLASS, CKO_SECRET_KEY); + gck_builder_add_ulong (&builder, CKA_KEY_TYPE, CKK_G_NULL); |