Skip to content

Read GError.message at its real offset on 64-bit Linux [patch] - #172

Merged
matt-edmondson merged 2 commits into
mainfrom
fix/gerror-message-offset-163
Sep 29, 2026
Merged

matt-edmondson merged 2 commits into
mainfrom
fix/gerror-message-offset-163

Conversation

@matt-edmondson

@matt-edmondson matt-edmondson commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #163

Problem

LinuxSecretServiceCredentialStore.ThrowIfError read the message pointer at IntPtr.Size * 2. On x64 that is offset 16, which is past the end of glib's GError { guint32 domain; gint code; gchar *message; }. The message actually sits at offset 8. So every libsecret failure on 64-bit Linux produced a garbage message, or faulted inside PtrToStringUTF8. That includes the common case of a headless machine with no Secret Service.

Change

  • New Storage/GError.cs declares GError as a [StructLayout(Sequential)] struct, which is the approach the triage comment preferred. GError.ReadMessage(IntPtr) reads the message through Marshal.PtrToStructure<GError>.
  • ThrowIfError now calls GError.ReadMessage. It still frees the error and throws the same way. It changed from private to internal so a test can reach it.
  • I checked for other IntPtr.Size * n offsets in the Linux store, as the triage comment suggested. There are none.

Tests

New file: CredentialCache.Test/GErrorTests.cs.

  • ReadMessageReturnsTheMessageField and ReadMessageReturnsNullForANullMessage: hand-build a GError in unmanaged memory, so they run on every platform. The buffer is zeroed and one pointer longer than the struct, so a read past the end finds null rather than whatever memory follows. With the old offset, both fail.
  • ThrowIfErrorReportsTheMessageOfARealGError: runs on Linux only and reports inconclusive elsewhere. glib's own g_error_new_literal builds the error, and the test asserts the CredentialStoreException carries its message. With the old offset, this test kills the test host with an AccessViolationException, which is the crash described in the issue.

The full suite: 64 passed, 5 skipped. Those 5 skips are native-store tests that were already skipped before this change. CI is green on all three OSes, and SonarCloud reports 100% coverage on new code.

Not covered: exercising libsecret itself with no D-Bus session. That needs libsecret and a Secret Service setup on the runner.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UnqwfbU2boDDiUoqRZSY2D

ThrowIfError read the message pointer at IntPtr.Size * 2, which is offset
16 on x64: past the end of glib's GError { guint32 domain; gint code;
gchar *message; }, whose message sits at offset 8. Every libsecret failure
produced a garbage message or faulted in PtrToStringUTF8.

GError is now declared as a sequential struct and the message read through
it, so the layout is written down rather than hand-computed.

Fixes #163

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UnqwfbU2boDDiUoqRZSY2D
SonarCloud flagged the ThrowIfError call site as uncovered new code. The
new test has glib build a real GError and checks that ThrowIfError reports
its message. It runs on Linux, where glib is present, and reports
inconclusive elsewhere. With the old IntPtr.Size * 2 offset it kills the
test host with an AccessViolationException.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UnqwfbU2boDDiUoqRZSY2D
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Linux store reads GError.message from the wrong offset on 64-bit, so every libsecret failure yields a garbage message (or a crash)

2 participants