diff options
Diffstat (limited to 'working/keyring-crash/status.org')
| -rw-r--r-- | working/keyring-crash/status.org | 118 |
1 files changed, 118 insertions, 0 deletions
diff --git a/working/keyring-crash/status.org b/working/keyring-crash/status.org new file mode 100644 index 0000000..aa0a952 --- /dev/null +++ b/working/keyring-crash/status.org @@ -0,0 +1,118 @@ +#+TITLE: Keyring crash: cause, fix, validation, deployment +#+AUTHOR: Craig Jennings +#+DATE: 2026-10-17 +#+STARTUP: showall + +Supersedes the status in the main checkout's status-and-plan.org from 10-16. +Everything below was done in a disposable VM and this worktree; the host +keyring, its packages and services were untouched until the deployment step. + +* Cause + +gnome-keyring aborts when a Secret Service method call is dispatched before +the daemon has created the caller's per-client record. The record is created +by a G_PRIORITY_HIGH idle queued from a GDBus message filter on the worker +thread; that idle is not reliably ordered against the method dispatch, so +under concurrent bursts a call occasionally arrives first, get_pkcs11_session +asserts, the negotiate helper returns FALSE without a GError, OpenSession +completes with a NULL body, and g_variant_new aborts the daemon. systemd +restarts it, the unlocked state is gone, and the next request shows the +keyring password dialog. + +This is upstream bug 190 (also 194, 195, 196, 199), present in 48 through +51.1, unmerged fix in merge request !112 since 2026-08-25. On ratio the first +abort was 2026-10-11 19:03:59, the evening the ChatGPT desktop app was +installed; openai/codex issue 51694 documents its bundled app-server opening +a fresh Secret Service connection per keyring read in bursts and crashing +gnome-keyring with exactly this signature. The same-day glib and keyring +upgrades were a coincidence: the VM reproduced the crash on 50.0 as well. + +A second, pre-existing defect surfaced while testing the fix: the filter's +alias rewrite reads the alias table on the worker thread while the main +thread replaces and frees its entries on every client registration. +AddressSanitizer confirmed it as a heap-use-after-free read in +gkd_secret_util_build_path from rewrite_default_alias, freed by +g_hash_table_replace in update_default. Stock and !112 both carry it. + +* Fix (packages/gnome-keyring, patch 0002) + +- Register the client at the point of use in get_pkcs11_session and + publish_dispatch (as !112). +- aes_negotiate and plain_negotiate return G_DBUS_ERROR_FAILED instead of + asserting; OpenSession branches on the return value and never completes + with a NULL body (as !112). +- lookup_collection takes the caller's PKCS#11 session before resolving an + alias, so the lazy registration cannot free a borrowed alias string (the + earlier reviewer's use-after-free finding). +- The property-path lookup returns NO_SUCH_OBJECT instead of asserting when + there is no session (upstream 195). +- A mutex guards the alias table; the worker-thread filter and store_default + take owned copies under it; the default alias is replaced only when its + value changes so main-thread borrowers stay valid. + +Packaged as Arch's own recipe for 1:51.1-1 (Cinnamon patch kept, arch-meson, +selinux disabled) built from GNOME's release tarball (published sha256 +dfe7adc5876ef0670e7eba2907799bdbb8875ce58bb4208e1a52169bd16a47ca) plus the +patch, at pkgrel 1.1: above stock, below any future 1:51.1-2 or 51.2, so a +normal upgrade replaces it when a real fix lands. Nothing pins or holds it. + +* Validation + +Probe (working/keyring-crash/probe.py): real GIO clients, seven modes, every +reply validated, a daemon round trip after each batch of 100, the synthetic +secret read back after each mode, bus owner and RSS recorded. Counts: 4000 +clients at 32 workers with bursts of 8 for plain, dh, invalid, disconnect and +pipeline; 2000 for alias and isolation; alias also at 8000. + +| Daemon | Result | +|-----------------------------+-----------------------------------------------------| +| stock 51.1 | aborts on the first plain batch, collection locked, | +| | ratio's exact journal signature (three runs) | +|-----------------------------+-----------------------------------------------------| +| v4 (lazy registration only) | passes all modes; alias at 8000 returns 9 bad paths | +| | and the filter asserts on a malformed object path | +|-----------------------------+-----------------------------------------------------| +| v4 under ASan | heap-use-after-free in rewrite_default_alias | +|-----------------------------+-----------------------------------------------------| +| v5 and v6, normal and ASan | all modes pass, alias at 8000 passes, no sanitizer | +| | report, stable owner, RSS flat, secret intact | +|-----------------------------+-----------------------------------------------------| +| v6 package via pacman -U | all modes pass through the installed /usr/bin daemon| +|-----------------------------+-----------------------------------------------------| + +Independent review of v5 (patch, pristine and patched trees, prior reviewer's +requirements, no rationale): approve, no memory-safety defect after checking +every caller. Two minor notes taken in v6 (store_default under the lock, plain +NULL return); two declined as pre-existing and bounded (a late dispatch after +NameOwnerChanged leaks one PKCS#11 session per vanished peer, unique names +never recur; the remove_filter shutdown race). + +Upstream meson suites, identical options and a private bus, run back to back: +stock and v6 both 56 ok, 1 fail, 5 timeouts, the same names in both +(gnome2-store import fails intermittently; stock scored 57 ok and 0 fail on +its first run; the five dbus fixture tests time out identically). No +regression attributable to the patch. Arch's own recipe ignores this suite's +failures, which is why it is a comparison here and not a gate. + +Installer: install_gnome_keyring_fix runs after pacman_install gnome-keyring, +builds and installs the recipe with makepkg as the user when stock 51.1 (or +older) is installed, is a no-op on the rebuild, and skips with a visible +warning when something newer is installed. Eleven tests in +tests/installer-steps/test_install_gnome_keyring_fix.py; the fast suite (619 +Python plus 17 ERT tests) passes. The false autologin comment on the +keyring-dir block is corrected. + +* Deployment + +working/keyring-crash/deploy-ratio.sh: build (done on ratio 06:53, package at +~/.cache/archsetup-build/gnome-keyring/), install (permission-protected copy +of ~/.local/share/keyrings, pacman -U, restart only +gnome-keyring-daemon.service, verify), rollback (pacman -U of the cached stock +1:51.1-1, restart), verify. One unlock prompt is expected after the restart; +answer it in the system dialog. The same script applies to velox once it is +online, after confirming its versions match. + +Observation period: the stock daemon aborted about three times a day on +ratio. A restart counter that stays flat for several days of normal ChatGPT +and Claude desktop use is the evidence of lasting stability; a short check +after install is not. |