Skip to content

BBRv3 inflight_at_loss panics: unchecked u64 subtraction underflows on loss (bbr3/mod.rs:626) #768

Description

@kixelated

Summary

Bbr3::inflight_at_loss performs an unchecked u64 subtraction that underflows when the loss already exceeds the loss threshold at the previous packet. In a debug build (overflow checks on) this panics and takes down the connection's runtime worker; in release it silently wraps to a huge u64, producing a garbage inflight_at_loss and, downstream, a corrupted inflight/cwnd estimate.

Panic

thread 'tokio-rt-worker' panicked at noq-proto-1.1.0/src/congestion/bbr3/mod.rs:626:33:
attempt to subtract with overflow

Location and cause

noq-proto/src/congestion/bbr3/mod.rs (line 626 on main today, identical in the published 1.1.0):

fn inflight_at_loss(&mut self, packet_size: u64) -> u64 {
    if let Some(rate_sample) = self.rs {
        let inflight_prev = rate_sample.tx_in_flight.saturating_sub(packet_size);
        let inflight_prev_threshold = LOSS_THRESH * inflight_prev as f64;
        let lost_prev = rate_sample.lost.saturating_sub(packet_size);
        let compared_loss = (inflight_prev_threshold.round() as u64) - lost_prev; // <-- underflows
        let lost_prefix = compared_loss as f64 / (1.0 - LOSS_THRESH);
        let inflight_at_loss = inflight_prev + lost_prefix as u64;
        return inflight_at_loss;
    }
    0
}

Every neighboring term uses saturating_sub, but compared_loss uses a plain -. When lost_prev > LOSS_THRESH * inflight_prev (i.e. the bytes lost already exceed the loss-threshold fraction of the prior inflight), (inflight_prev_threshold.round() as u64) - lost_prev underflows.

This mirrors BBRInflightAtLoss() in draft-ietf-ccwg-bbr-05 §5.5.10.2, where the expression BBRLossThresh * inflight_prev - lost_prev is computed in real arithmetic and is legitimately allowed to go negative (yielding a lost_prefix < 0 and inflight < inflight_prev). Evaluating it in u64 loses that sign and underflows instead.

When it triggers

inflight_at_loss runs during loss handling, so any connection that declares loss can hit it. It reproduces readily under a bufferbloated / lossy bottleneck (a droptail queue overflowing counts as loss even with no configured random loss). A short, loss-free transfer will not hit it, which is why it only shows up under sustained load.

Reproduction context

  • noq-proto 1.1.0, debug build (overflow-checks = true).
  • macOS aarch64.
  • Hit via a QUIC server (a MoQ relay using the noq backend) with BBRv3 selected, serving a ~2 Mbit media flow across a dummynet-shaped 3 Mbit link with a deep (~600 ms) droptail queue and ~30 ms RTT. The relay's tokio-rt-worker panicked and aborted a few seconds into sustained delivery.

Suggested fix

Minimal: clamp the subtraction.

let compared_loss = (inflight_prev_threshold.round() as u64).saturating_sub(lost_prev);

That floors inflight_at_loss at inflight_prev, which is a reasonable "loss already past threshold" result. If you want to match the draft's real-valued formula faithfully (allowing inflight_at_loss < inflight_prev), compute the numerator in f64/i64 and clamp the final result to >= 0 rather than clamping the intermediate.

Happy to send a PR for either approach.

(written by Opus 4.8)

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

    No type

    Projects

    • Status
      🚑 Needs Triage

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions