mirrors/meta-openembedded
1
0
Fork 0
mirror of https://github.com/openembedded/meta-openembedded.git synced 2026-09-28 00:50:18 +00:00
Code Issues Projects Releases Wiki Activity

gnome-keyring: fix the use-after-free in the secret service

Browse Source 
Unlocking a keyring from a client that disconnects afterwards crashed
gnome-keyring-daemon: free_client() drops the PKCS#11 session before the
dispatch table that still owns GkdSecretSession objects referencing it,
gkd_secret_unlock used the service after dispose, and the session held
one reference too few on its GckSession. Three patches address the three
places; debug-mode is turned off so the daemon does not keep the
allocator poisoning enabled.

AI-Generated: Uses Claude Code (Claude Opus 5)
Signed-off-by: Markus Volk <f_l_k@t-online.de>
Signed-off-by: Khem Raj <khem.raj@oss.qualcomm.com>
This commit is contained in:
Markus Volk
2026-09-21 16:21:44 -07:00
committed by Khem Raj
parent 3409b8b443
commit c216ad818d
4 changed files with 262 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
meta-gnome/recipes-gnome/gnome-keyring/gnome-keyring/0002-gkd-secret-service-destroy-the-dispatch-table-before.patch
+76
View File
@@ -0,0 +1,76 @@
From b105e7f122805aceef8c3674b04f5d6b7865ad17 Mon Sep 17 00:00:00 2001
From: Markus Volk <f_l_k@t-online.de>
Date: Tue, 15 Sep 2026 18:24:00 +0200
Subject: [PATCH 2/3] gkd-secret-service: destroy the dispatch table before the
PKCS#11 session
free_client() drops its reference to client->pkcs11_session before it
destroys client->dispatch. The dispatch table still holds the client's
GkdSecretSession objects, whose GckObject keeps a pointer to that very
GckSession and unrefs it from gck_object_finalize(), so tearing the
session down first leaves a dangling pointer behind.
Measured on a running system with a breakpoint in free_client():
=== before unref of pkcs11_session ===
pkcs11_session = 0x55b347f29040 refcount = 1
dispatch size = 1
=== after unref, before destroying dispatch ===
pkcs11_session = 0x55b347f29040 refcount = 2863311530 (0xAAAAAAAA)
=== session_dispose: unref self->key ===
key->pv->session = 0x55b347f29040 (already freed)
The resulting use after free kills the daemon:
g_object_unref
gck_object_finalize gck/gck-object.c:135
gkd_secret_session_dispose daemon/dbus/gkd-secret-session.c:357
dispose_and_unref daemon/dbus/gkd-secret-service.c:245
g_hash_table_destroy
free_client daemon/dbus/gkd-secret-service.c:269
Any client that opens a session, reads a secret and then leaves the bus
triggers this - an ordinary "secret-tool lookup" is enough. systemd
socket activation restarts the daemon, but the new instance no longer
holds the login password, so the keyring stays locked from then on and
every further access needs an unlock prompt, which crashes it again.
Destroy the dispatch table first, then release the session. Holding an
extra reference across free_client() under gdb has the same effect and
lets the daemon survive repeated client cycles.
Upstream-Status: Pending
AI-Generated: Uses Claude Code (Claude Opus 5)
---
daemon/dbus/gkd-secret-service.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/daemon/dbus/gkd-secret-service.c b/daemon/dbus/gkd-secret-service.c
index 72b3110..391c445 100644
--- a/daemon/dbus/gkd-secret-service.c
+++ b/daemon/dbus/gkd-secret-service.c
@@ -257,6 +257,13 @@ free_client (gpointer data)
/* Info about our client */
g_free (client->caller_peer);
+ /*
+ * The sessions and prompts the client has open. These have to go
+ * first: a GkdSecretSession holds a GckObject whose GckSession is
+ * the pkcs11_session below, and it unrefs it when it is finalized.
+ */
+ g_hash_table_destroy (client->dispatch);
+
/* The session we use for accessing as our client */
if (client->pkcs11_session) {
#if 0
@@ -265,9 +272,6 @@ free_client (gpointer data)
g_object_unref (client->pkcs11_session);
}
- /* The sessions and prompts the client has open */
- g_hash_table_destroy (client->dispatch);
-
g_free (client);
}
meta-gnome/recipes-gnome/gnome-keyring/gnome-keyring/0003-gkd-secret-unlock-do-not-use-the-service-after-dispo.patch
+122
View File
@@ -0,0 +1,122 @@
From 65693eab7baa49aff81f7534dd3d687fecbc4900 Mon Sep 17 00:00:00 2001
From: Markus Volk <f_l_k@t-online.de>
Date: Tue, 15 Sep 2026 18:24:30 +0200
Subject: [PATCH 3/3] gkd-secret-unlock: do not use the service after dispose
When the calling client goes away while an unlock is in flight,
free_client() runs g_object_run_dispose() on the GkdSecretUnlock.
gkd_secret_unlock_dispose() clears skeleton and service but never
cancels self->cancellable - g_cancellable_cancel() is only called from
the dismiss handler. The gck_session_create_object_async() in flight
holds its own reference and keeps running, and its callback then works
on the gutted object:
gkd_secret_service_get_objects: assertion 'GKD_SECRET_IS_SERVICE (self)' failed
gkd_secret_objects_lookup_collection: assertion 'GKD_SECRET_IS_OBJECTS (self)' failed
g_dbus_interface_skeleton_get_object_path: assertion 'G_IS_DBUS_INTERFACE_SKELETON (interface_)' failed
g_dbus_connection_emit_signal: assertion 'G_IS_DBUS_CONNECTION (connection)' failed
GkdSecretPrompt guards against this with g_return_val_if_fail
(self->pv->service) in gkd_secret_prompt_get_objects(), but
GkdSecretUnlock derives straight from GObject and dereferences
self->service unchecked.
Cancel the pending operation from dispose and check for the disposed
state before touching service or skeleton. The unlock queue is still
handed on to the next waiting prompt in that case.
Upstream-Status: Pending
AI-Generated: Uses Claude Code (Claude Opus 5)
---
daemon/dbus/gkd-secret-unlock.c | 42 ++++++++++++++++++++++++++++++++-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/daemon/dbus/gkd-secret-unlock.c b/daemon/dbus/gkd-secret-unlock.c
index b04689f..55322c8 100644
--- a/daemon/dbus/gkd-secret-unlock.c
+++ b/daemon/dbus/gkd-secret-unlock.c
@@ -93,7 +93,13 @@ EGG_SECURE_DECLARE (secret_unlock);
static GckObject*
lookup_collection (GkdSecretUnlock *self, const gchar *path)
{
- GkdSecretObjects *objects = gkd_secret_service_get_objects (self->service);
+ GkdSecretObjects *objects;
+
+ /* Already disposed, e.g. because the calling client went away */
+ if (self->service == NULL)
+ return NULL;
+
+ objects = gkd_secret_service_get_objects (self->service);
return gkd_secret_objects_lookup_collection (objects, self->caller, path);
}
@@ -104,6 +110,10 @@ emit_collection_unlocked (GkdSecretUnlock *self,
GkdSecretObjects *objects;
GckObject *collection;
+ /* Already disposed, e.g. because the calling client went away */
+ if (self->service == NULL)
+ return;
+
objects = gkd_secret_service_get_objects (self->service);
collection = gkd_secret_objects_lookup_collection (objects, self->caller, path);
if (collection != NULL) {
@@ -158,6 +168,14 @@ mark_as_complete (GkdSecretUnlock *self, gboolean dismissed)
g_free (self->current);
self->current = NULL;
+ /*
+ * If we have already been disposed there is no skeleton left to emit
+ * the signal from, and no client left that would care. Still hand the
+ * queue on to the next waiting prompt below.
+ */
+ if (self->skeleton == NULL)
+ goto next_in_queue;
+
g_variant_builder_init (&builder, G_VARIANT_TYPE ("ao"));
for (i = 0; i < self->results->len; ++i) {
value = g_array_index (self->results, gchar*, i);
@@ -173,6 +191,7 @@ mark_as_complete (GkdSecretUnlock *self, gboolean dismissed)
g_variant_new ("(b@v)", dismissed, variant),
NULL);
+next_in_queue:
/* Fire off the next item in the unlock prompt queue */
other = g_queue_pop_head (&unlock_prompt_queue);
if (other != NULL) {
@@ -198,6 +217,20 @@ on_unlock_complete (GObject *object, GAsyncResult *res, gpointer user_data)
else
g_warning ("unlock prompt queue is out of sync with prompts");
+ /*
+ * We were disposed while this call was in flight, because the calling
+ * client went away. There is nobody left to report to, but the global
+ * unlock queue still has to be handed on to the next waiting prompt.
+ */
+ if (self->skeleton == NULL) {
+ cred = gck_session_create_object_finish (GCK_SESSION (object), res, &error);
+ g_clear_object (&cred);
+ g_clear_error (&error);
+ mark_as_complete (self, TRUE);
+ g_object_unref (self);
+ return;
+ }
+
/* Now process the results */
cred = gck_session_create_object_finish (GCK_SESSION (object), res, &error);
@@ -402,6 +435,13 @@ gkd_secret_unlock_dispose (GObject *obj)
{
GkdSecretUnlock *self = GKD_SECRET_UNLOCK (obj);
+ /*
+ * Abort any PKCS#11 call that is still in flight. It holds its own
+ * reference on us, and its callback would otherwise run against the
+ * state we are about to tear down.
+ */
+ g_cancellable_cancel (self->cancellable);
+
if (self->skeleton) {
g_dbus_interface_skeleton_unexport (G_DBUS_INTERFACE_SKELETON (self->skeleton));
g_clear_object (&self->skeleton);
meta-gnome/recipes-gnome/gnome-keyring/gnome-keyring/0004-gkd-secret-session-drop-the-duplicate-session-unref.patch
+58
View File
@@ -0,0 +1,58 @@
From 011d89df54d0f441d17c449513ae0ef8aa21c4cd Mon Sep 17 00:00:00 2001
From: Markus Volk <f_l_k@t-online.de>
Date: Tue, 15 Sep 2026 19:22:41 +0200
Subject: [PATCH] gkd-secret-session: drop the duplicate session unref
gkd_secret_session_get_item_secret() unrefs the session returned by
gck_object_get_session() twice: once right after
gck_session_wrap_key_full(), and once more below with a comment claiming
the reference would otherwise leak. gck_object_get_session() hands out a
single new reference, so the second unref drops one reference too many
on every GetSecret call.
The GckSession is freed while client->pkcs11_session and the GckObject
of the session key still point at it. The daemon dies when the client
leaves the bus and its dispatch table is torn down:
service_name_owner_changed daemon/dbus/gkd-secret-service.c:944
free_client daemon/dbus/gkd-secret-service.c:264
dispose_and_unref daemon/dbus/gkd-secret-service.c:245
gkd_secret_session_dispose daemon/dbus/gkd-secret-session.c:357
gck_object_finalize gck/gck-object.c:135
g_object_unref -> SIGSEGV, rax = 0xaaaaaaaaaaaaaaaa
Reproduced on a running system with a single "secret-tool lookup",
symbolized against /usr/src/debug. The older crashes reported as
"segfault at f00dface in libgck-1.so" are the same use after free caught
at an earlier point.
gkd-secret-lock.c:49 and gkd-secret-unlock.c:726 do it correctly, with
exactly one unref per gck_object_get_session().
Upstream-Status: Pending
Still present in gnome-keyring main as of 2026-09-15.
AI-Generated: Uses Claude Code (Claude Opus 5)
---
daemon/dbus/gkd-secret-session.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/daemon/dbus/gkd-secret-session.c b/daemon/dbus/gkd-secret-session.c
index 16bb368..6799b4c 100644
--- a/daemon/dbus/gkd-secret-session.c
+++ b/daemon/dbus/gkd-secret-session.c
@@ -609,13 +609,6 @@ gkd_secret_session_get_item_secret (GkdSecretSession *self, GckObject *item,
NULL, &error);
g_object_unref (session);
- /*
- * gck_object_get_session() returns a new reference, so drop it again.
- * Not doing so leaks a GckSession on every GetSecret call, which in turn
- * keeps the client's PKCS#11 session alive after the client is gone.
- */
- g_object_unref (session);
-
if (error != NULL) {
if (g_error_matches (error, GCK_ERROR, CKR_USER_NOT_LOGGED_IN)) {
g_set_error_literal (error_out, GKD_SECRET_ERROR,
meta-gnome/recipes-gnome/gnome-keyring/gnome-keyring_51.0.bb
+6 -1
View File
@@ -21,7 +21,11 @@ DEPENDS = " \
inherit gnomebase gsettings gettext
SRC_URI[archive.sha256sum] = "2aebaa2d474cc31507c87a7bbbdb3e16dbe26b1cfef9f206457f3f9df43558b0"
SRC_URI += "file://0001-meson-allow-setting-the-paths-to-ssh-agent-and-ssh-add-by-option.patch"
SRC_URI += "file://0001-meson-allow-setting-the-paths-to-ssh-agent-and-ssh-add-by-option.patch \
file://0002-gkd-secret-service-destroy-the-dispatch-table-before.patch \
file://0003-gkd-secret-unlock-do-not-use-the-service-after-dispo.patch \
file://0004-gkd-secret-session-drop-the-duplicate-session-unref.patch \
"
PACKAGECONFIG ??= " \
libcap-ng \
@@ -35,6 +39,7 @@ PACKAGECONFIG[ssh-agent] = "-Dssh-agent=true -Dssh-agent-path=${bindir}/ssh-agen
PACKAGECONFIG[systemd] = "-Dsystemd=enabled,-Dsystemd=disabled,systemd"
EXTRA_OEMESON = " \
-Ddebug-mode=false \
-Dmanpage=false \
-Dpkcs11-config=${datadir}/p11-kit/modules \
-Dpkcs11-modules=${libdir}/pkcs11 \