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);