Skip to content

bridge: SeqQuote's payload is accessed non-atomically, so a torn read is UB before the version check can reject it #388

Description

@userFRM

Problem

SeqQuote's payload is read and written through plain, non-atomic access while a concurrent reader may be looking at it. That is a data race by the Rust abstract machine regardless of how the version counter is ordered, and Miri reports it as UB.

The seqlock's version counter tells a reader whether the snapshot it took is trustworthy, and the retry loop discards a torn one. What it cannot do is make the torn read itself defined: the reader has already performed a non-atomic read of memory a writer is concurrently writing, and that is UB before the version check ever runs.

Reproduced with Miri across ten interleaving seeds. It fires at the same rate whether the odd-version transition is annotated Release or AcqRel, which is what distinguishes it from the ordering defect in #267 — that one is about where the payload write may be reordered to, this one is about the access itself.

Impact

No misbehaviour has been observed on x86, where a core does not reorder its own accesses relative to each other and the compiler has not chosen to exploit the UB. That is a statement about one target and one optimiser, not a guarantee. A compiler is entitled to assume the race cannot happen and optimise accordingly, and aarch64 has a weaker memory model.

It also means the type cannot be validated under Miri, so a genuine future defect in it would be lost among the reported races.

Solution

Give the payload per-field atomic or volatile access so the read and the write are both defined, and keep the version counter as the freshness test it already is. Quote is a small struct of scalars, so per-field AtomicI64/AtomicU32 with relaxed ordering is the usual shape — the seqlock's fences already supply the ordering, and relaxed atomics only need to make the access itself defined.

This is materially larger than the ordering fix in #267 and is deliberately separated from it.

Notes

Read from v0.7.1 (9367845). Confirmed with Miri on the nightly toolchain; a plain 30-million-write stress across twelve threads in both debug and release showed no observable divergence on x86, which is why this needs the tool rather than a test.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions