Skip to content

Fix Thread Sanitizer SEGV in certificate generation - #17

Open
darioalessandro wants to merge 1 commit into
mainfrom
fix-tsan-cert-segv
Open

Fix Thread Sanitizer SEGV in certificate generation#17
darioalessandro wants to merge 1 commit into
mainfrom
fix-tsan-cert-segv

Conversation

@darioalessandro

Copy link
Copy Markdown
Member

The issue

IdentityCertificate.makeCertificate built its serial number with Certificate.SerialNumber() — swift-certificates' convenience initialiser. That funnels into its @inlinable generic RandomNumberGenerator.bytes(count:), and the specialisation the compiler emits is miscompiled under Thread Sanitizer: the instrumentation ends up treating a freshly generated random value as a memory address, so the process SEGVs on a garbage pointer inside __tsan::MemoryAccess.

ERROR: ThreadSanitizer: SEGV on unknown address 0x4155255554020000
  #0 __tsan::MemoryAccess(...)
  #1 X509.Certificate.SerialNumber.init<A: RandomNumberGenerator>(generator: inout A)
  #3 static Stormo.IdentityCertificate.makeCertificate(for:)

x[1] = 0x4155255554020000 is RNG output being used as a pointer.

Impact: on macOS this first shows up as Swift access race ... Location is global 'value witness table for PeerIdentity.SigningKey' (7 warnings, tests still pass). On the iOS Simulator it is a hard process abort, which makes a TSan-enabled suite impossible to run in a host app — Remote Shutter's 619-test suite could not complete, so that project's "runs clean under TSan" guarantee had silently lapsed.

The fix

Generate the 20 random bytes directly and use the explicit init(bytes:) overload. Semantically identical — 20 random bytes, ASN.1-normalised by that initialiser — while staying out of the miscompiled specialisation.

What this is not

The stack looks like a data race, and it isn't one in this code:

  • A lock around certificate construction did not fix it — and made macOS abort too.
  • Certificate.SerialNumber() alone passes under TSan.
  • CryptoKit P-256 signing alone passes.
  • Certificate.PublicKey / Certificate.PrivateKey wrapping alone passes.
  • Only the full Certificate.init(...) signing path faults.

The blamed frame is an inlined temporary, not where the fault is. Hoisting the argument-list temporaries into locals also made the crash disappear — that confirmed codegen sensitivity, but it is a symptom patch, so it isn't what shipped here.

Root cause is upstream (Swift compiler + TSan instrumentation of that @inlinable generic); this change routes around it.

Verification

before after
swift test --sanitize=thread 7 warnings, then SEGV/abort 76 tests, 19 suites, clean
Host app suite on iOS Simulator, TSan abort at 115/616 619 tests, 0 failures, exit 0

Adds two tests: serial numbers are correctly sized and distinct, and an 8-way concurrent generation test. Reverting the one-line fix makes the suite report the SEGV/race again, so the guard is real.

🤖 Generated with Claude Code

`IdentityCertificate.makeCertificate` used `Certificate.SerialNumber()`,
swift-certificates' convenience initialiser. That funnels into its
`@inlinable` generic `RandomNumberGenerator.bytes(count:)`, and the
specialisation the compiler emits for it is miscompiled under Thread
Sanitizer: the instrumentation ends up treating a freshly generated random
value as a memory address, so the process takes a SEGV on a garbage pointer
inside `__tsan::MemoryAccess`.

The report blames `Certificate.SerialNumber.init`, which is misleading —
that initialiser is fine on its own, and so is CryptoKit signing on its own.
On macOS it surfaces first as `Swift access race ... Location is global
'value witness table for PeerIdentity.SigningKey'`; on the iOS Simulator it
is a hard process abort, which made a TSan-enabled suite impossible to run
in a host app (Remote Shutter's 619-test suite could not complete).

Generate the 20 random bytes directly and use the explicit `init(bytes:)`
overload. Semantically identical — 20 random bytes, ASN.1-normalised by that
initialiser — while staying out of the miscompiled specialisation.

Ruled out along the way: it is not a data race in this code. Serialising
certificate construction behind a lock did not fix it (and made macOS abort
too); `SerialNumber()` alone, CryptoKit signing alone, and
`Certificate.PublicKey`/`PrivateKey` wrapping alone all pass under TSan.
Only the full signing path faults.

Tests: `swift test --sanitize=thread` goes from 7 warnings + abort to 76
tests clean. Adds a concurrent-generation regression test that reproduces
the crash when the fix is reverted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant