Skip to content

block sync: admission removes a session-gap claim without checking who owns it #1005

Description

@czarcas7ic

Raised by a V12 audit of the paired-serving activation and verified against the current tree. Pre-existing on main, byte-identical there.

What happens

add_peer removes the peer's session-gap claim after releasing the session-table lock, and without checking who owns the claim:

// The admitted session supersedes any gap claim left by its predecessor.

The asymmetry is visible a few hundred lines below in the same file: remove_peer guards the same removal on the owning connection and does it while holding the peer-map lock, and finish_session inserts the claim under that lock. Only this one call site touches the claims map after releasing it.

So a superseded add_peer that is preempted between installing its session and removing the claim can delete a claim installed later by a different connection's teardown. That claim is what keeps the newer connection owned during ordered-stream reopen backoff, so an ownership sample can close a healthy connection.

Scope

Narrow and self-healing. add_peer is synchronous, so this needs genuine thread preemption across a window that must contain a full admit and teardown of the second connection, and the connection reconnects afterwards. Rated Low by the audit, and I agree.

Suggested fix

Move the claim removal inside the block that already holds the peer-map lock, so it matches the guarded, lock-ordered removal in remove_peer. The predicate in that function is the model to copy.

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

    blocksyncanything related to blocksyncp2pLegacy Zcash P2P stack

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions