Skip to content

Fix data race on HashedTxHistory endorsed certificate #8273

Description

Describe the bug
HashedTxHistory::endorsed_cert is read and written concurrently without synchronization.

The Long Test TSAN job for #8243 reported a write from the node-endorsed-certificate local hook:

  • NodeState::setup_basic_hooks() calls history->set_endorsed_certificate() at src/node/node_state.h:3335.
  • HashedTxHistory::set_endorsed_certificate() assigns the std::optional<ccf::crypto::Pem> at src/node/history.h:1025.

At the same time, the periodic signature task reads the same Pem:

  • HashedTxHistory::start_signature_emit_timer() -> emit_signature() -> Store::commit() -> MerkleTreeHistoryPendingTx::call().
  • The conflicting copy occurs while constructing PrimarySignature at src/node/history.h:367.

MerkleTreeHistoryPendingTx stores ccf::crypto::Pem& endorsed_cert (src/node/history.h:316), referencing the mutable certificate owned by HashedTxHistory. Replacing that certificate while a pending signature transaction uses it is undefined behavior. TSAN emitted two warnings for the same logical race, covering the std::string buffer and length accesses.

This is unrelated to #8243: that PR only changes CHANGELOG.md, src/kv/store.h, and src/kv/test/kv_test.cpp. It is also distinct from #8135, which synchronizes the certificate fields owned by NodeState, not this history-owned certificate.

To Reproduce

  1. Run the Long Test TSAN workflow with the full end-to-end suite and multiple enclave worker threads.
  2. During recovery or certificate renewal, overlap the node-endorsed-certificate local hook with periodic signature emission.
  3. Observe the TSAN reports in full_test_suite.

Observed on commit af77be08ec348c6208576c42122c5ab392bd103b. The sanitizer report caused node shutdown to fail, leaving 75/76 tests passing and CTest exiting with code 8.

Expected behavior
Signature construction must use a stable endorsed-certificate snapshot while certificate updates may occur concurrently, with no unsynchronized access or reference to mutable certificate storage.

A likely fix is to synchronize access to HashedTxHistory::endorsed_cert, copy it while protected when creating a signature transaction, and make MerkleTreeHistoryPendingTx own the Pem by value rather than retaining a reference.

Environment information

  • Workflow: Long Test / TSAN
  • Runner image: Azure Linux 3 (azl3-vmss-nvme)
  • Runner SKU: Standard_D16ads_v6
  • CCF commit: af77be08ec348c6208576c42122c5ab392bd103b

Additional context
The certificate reference and unsynchronized setter predate the failing PR, so rerunning may avoid the interleaving but would only hide a genuine race. No currently open CCF PR found during investigation fixes this history-owned certificate access.

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

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions