Skip to content

[Adversarial review] Extreme durations wrap scheduler jitter and panic Compio TCP/TLS tasks #171

Description

@al8n

Summary

The audit retained exactly two correctness findings involving extreme Duration values. Both require operator-controlled pathological configuration; neither is remotely triggerable through protocol input.

ID Severity Component Finding
F1 Low memberlist-proto Initial scheduler staggering truncates intervals modulo (2^{64}) nanoseconds
F2 Medium memberlist-compio Accepted TCP/TLS timeouts can panic detached Compio runtime tasks on host Instant overflow

No consensus-safety violation, durable-data corruption, or recurring scheduler busy loop was identified.

Audit scope and repository pin

  • Repository: al8n/memberlist
  • Commit: aeef956bc0bdf0ed21ba386b329dab6aae64b47e
  • Scope: time, deadline, counter, and ID exhaustion algebra across:
    • memberlist-proto
    • memberlist-compio
    • memberlist-reactor
  • Existing issues #157 through #170 were reviewed to avoid duplicating already-filed findings.

F1 — Initial scheduler stagger narrows valid Duration values modulo (2^{64}) nanoseconds

Severity: Low
Trigger: Operator-provided configuration
Affected component: memberlist-proto

Affected code

  • memberlist-proto/src/endpoint/mod.rs:4051-4069
    • Calls random_stagger when calculating the initial probe, gossip, and push/pull deadlines.
    • Adds the returned stagger to now.
  • memberlist-proto/src/endpoint/mod.rs:4111-4120
    • Converts the configured interval with:
      let nanos = interval.as_nanos() as u64;
    • Returns Duration::ZERO if the narrowed value is zero.
    • Otherwise samples from 0..nanos.

The relevant configuration builders accept Duration values without an upper bound.

Invariant

A valid configured interval should retain its full Duration value when defining the random startup-stagger range. Converting the interval to a sampling domain must not silently reduce it modulo (2^{64}) nanoseconds.

Reproduction

The boundary value:

Duration::new(18_446_744_073, 709_551_616)

is exactly (2^{64}) nanoseconds.

At this boundary:

interval.as_nanos() = 18446744073709551616
interval.as_nanos() as u64 = 0

One nanosecond below the boundary narrows to:

18446744073709551615

At exactly (2^{64}) nanoseconds, random_stagger therefore returns Duration::ZERO unconditionally. All enabled initial probe, gossip, and push/pull deadlines are set to now, despite the configured interval being approximately 584.5 years.

Above the boundary, the sampling range is based on the interval modulo (2^{64}) nanoseconds rather than the configured interval.

Impact

A pathological but accepted operator configuration can make the first probe, gossip, and push/pull operations occur immediately and together, contradicting the intended random startup staggering.

This affects only initial scheduling. Subsequent ticks use the full configured interval, so this finding does not establish a recurring busy loop.

Root cause

The full-width u128 result of Duration::as_nanos() is narrowed with a lossy as u64 cast before sampling.

Recommended remediation

Preserve the full Duration domain when constructing the random range. Suitable approaches include:

  • Sampling seconds and subsecond nanoseconds separately.
  • Sampling a u128 range using an RNG interface that supports it.
  • Introducing another unbiased Duration-aware sampler.

Rejecting or capping intervals before scheduling would also prevent truncation, but preserving the valid Duration domain is preferable unless the configuration contract intentionally imposes a smaller maximum.

Regression tests

Add boundary coverage for:

  • (2^{64}\text{ ns} - 1)
  • (2^{64}\text{ ns})
  • (2^{64}\text{ ns} + 1)

The tests should verify that the sampling domain remains nonzero and does not wrap at the boundary. They should not depend on a particular random result; for example, the range-construction logic can be factored into a directly testable helper or exercised with an inspectable deterministic sampler.

F2 — Accepted Compio TCP/TLS timeouts can panic detached runtime tasks on host Instant overflow

Severity: Medium
Trigger: Operator-provided configuration
Affected component: memberlist-compio

Affected code

  • memberlist-compio/src/driver/options/mod.rs:465-504
    • Exposes arbitrary Duration values through dial_timeout and close_timeout builders.
  • memberlist-compio/src/driver/options/mod.rs:546-608
    • Rejects a zero close_timeout and zero capacities.
    • Does not impose an upper bound on either timeout.
    • Explicitly permits a zero dial_timeout.
  • TCP and TLS constructors invoke this validation, so Duration::MAX is accepted.
  • memberlist-compio/src/driver/stream/mod.rs:2197-2208
    • Passes dial_timeout to compio::time::sleep inside a spawned outbound-dial task.
  • memberlist-compio/src/bridge/mod.rs:454-482
    • Constructs compio::time::sleep(close_timeout) for each pending write in the bridge task.

Existing issue #157 separately reports how close_timeout is applied to active writes. This finding is limited to the unchecked extreme-duration panic and does not duplicate that issue.

Invariant

Any timeout accepted by a public constructor should either:

  • Be safely representable by the selected runtime’s timer implementation, or
  • Produce a typed configuration or operation error.

An accepted timeout should not panic a detached runtime task during timer construction.

Runtime mechanism

The locked compio-runtime version is 0.12.2. Its time.rs:41-43 implements duration-based sleep as:

sleep_until(Instant::now() + duration).await

This uses unchecked std::time::Instant addition.

On the audited macOS host, the following was reproduced:

Instant::now().checked_add(Duration::MAX).is_none()

returns true, while:

Instant::now() + Duration::MAX

panics.

Outbound-dial trace

  1. Configure TCP or TLS with with_dial_timeout(Duration::MAX).
  2. Construction and option validation succeed.
  3. The first outbound Connect operation spawns its dial task.
  4. When the task is polled, compio::time::sleep(Duration::MAX) evaluates the overflowing Instant addition.
  5. The task panics before producing BridgeReady.

Instead of returning a typed configuration or dial error, the outbound task is killed and the surrounding coordination can discover the failure only indirectly.

Write trace

  1. Configure the driver with with_close_timeout(Duration::MAX).
  2. Construction and option validation succeed.
  3. Queue the first reliable write.
  4. The bridge task polls the sleep expression for that pending write.
  5. Timer construction performs the overflowing Instant addition and panics.

The bridge task therefore fails before producing a WriteStatus or typed timeout/configuration error.

Impact

A pathological operator-supplied timeout can panic detached Compio tasks during ordinary dial or reliable-write paths. The failure bypasses the driver’s typed error handling and can leave related coordination to infer task loss indirectly.

The timeout values are local configuration, not remote protocol input.

Root cause

The Compio driver accepts the complete Duration domain, but delegates duration-based sleeping to a runtime implementation that performs unchecked host-Instant addition. The option validator checks zero-value semantics but not timer representability.

Recommended remediation

Apply one consistent policy to both driver-owned timeouts:

  1. Validate dial_timeout and close_timeout using Instant::now().checked_add(duration) during construction and return InvalidOption when the deadline is not representable; or
  2. Stop using Compio’s unchecked duration-based sleep and construct deadlines through a checked, saturating, or “never fires” path with documented semantics.

The same policy should cover TCP and TLS constructors and both dial and write timer paths.

Regression tests

Tests should establish that extreme values either fail fast or remain non-panicking:

  • TCP construction with dial_timeout = Duration::MAX.
  • TLS construction with dial_timeout = Duration::MAX.
  • TCP construction with close_timeout = Duration::MAX.
  • TLS construction with close_timeout = Duration::MAX.
  • Direct observation of outbound-dial timer construction.
  • Direct observation of bridge/write timer construction.
  • Panic detection through catch_unwind or runtime task observation, as appropriate.

The existing focused test transport_new_rejects_zero_close_timeout ran one real test successfully (1 passed, 9 filtered). This confirms that zero close_timeout is validated; the source trace confirms that the maximum value remains accepted.

Cross-driver architecture note

Extreme-duration policy is inconsistent across runtime backends:

  • Locked Compio sleep performs unchecked Instant::now() + duration.
  • Locked Tokio sleep uses checked_add with a far-future fallback.
  • async-io’s Timer::after uses checked_add with Timer::never as its fallback.
  • Portable protocol deadline arithmetic generally uses saturation.

Consequently, the same accepted configuration can remain non-panicking under reactor-backed implementations while panicking under Compio. Driver-owned timeout validation or a shared deadline-construction policy would make behavior explicit and consistent across backends.

Positive controls and rejected candidates

The audit rejected the remaining examined wrap candidates as infeasible or protected:

  • Endpoint probe-sequence allocation can wrap its u32 counter, but it skips zero and every identifier still live in probes, indirect forwards, or the ACK registry.
  • u64 StreamId, ExchangeId, join, broadcast, and metric counters require approximately (2^{64}) operations to wrap, with no practical trigger identified.
  • Broadcast state includes explicit wrap/reset handling.
  • Portable protocol Instant and deadline arithmetic generally uses saturating operations.
  • No consensus-safety violation or durable-data-corruption path was established.

Validation evidence

The retained findings are supported by:

  • Source tracing at commit aeef956bc0bdf0ed21ba386b329dab6aae64b47e.
  • Exact arithmetic at the (2^{64})-nanosecond boundary.
  • Host reproduction showing that Duration::MAX is not addable to the audited macOS Instant and that unchecked addition panics.
  • Source confirmation that TCP and TLS constructors accept the extreme Compio timeout values.
  • Source confirmation of the outbound-dial and bridge/write timer call paths.
  • Locked-runtime source comparison across Compio, Tokio, and async-io.
  • The focused zero-close_timeout validation test result: 1 passed, 9 filtered.
  • Review of existing issues #157 through #170 to delimit already-reported behavior.

No unreported test outcome is implied.

Acceptance criteria

  • Initial stagger range construction preserves the configured interval across (2^{64}\text{ ns} - 1), (2^{64}\text{ ns}), and (2^{64}\text{ ns} + 1).
  • Initial probe, gossip, and push/pull scheduling no longer receives a modulo-narrowed range.
  • Compio applies the same documented representability policy to dial_timeout and close_timeout.
  • TCP and TLS constructors either reject Duration::MAX with a typed InvalidOption error or use timer construction that cannot panic.
  • Outbound-dial and bridge/write timer paths are covered by direct non-panic regression tests.
  • Existing zero-timeout semantics remain intentional and tested.
  • Runtime-backend differences for extreme durations are either eliminated or explicitly documented.

Repository modification status

No repository files were modified during this audit or report preparation.

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