Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(swarm): prevent overflow in keep-alive computation #4559

Merged
merged 11 commits into from
Sep 27, 2023
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ libp2p-rendezvous = { version = "0.13.0", path = "protocols/rendezvous" }
libp2p-upnp = { version = "0.1.1", path = "protocols/upnp" }
libp2p-request-response = { version = "0.25.1", path = "protocols/request-response" }
libp2p-server = { version = "0.12.3", path = "misc/server" }
libp2p-swarm = { version = "0.43.4", path = "swarm" }
libp2p-swarm = { version = "0.43.5", path = "swarm" }
libp2p-swarm-derive = { version = "0.33.0", path = "swarm-derive" }
libp2p-swarm-test = { version = "0.2.0", path = "swarm-test" }
libp2p-tcp = { version = "0.40.0", path = "transports/tcp" }
Expand Down
5 changes: 5 additions & 0 deletions swarm/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 0.43.5

- Fix overflow in `KeepAlive` computation that could occur if `SwarmBuilder::idle_connection_timeout` is configured with `u64::MAX`.
See [PR 4559](https://github.com/libp2p/rust-libp2p/pull/4559).

## 0.43.4

- Implement `Debug` for event structs.
Expand Down
2 changes: 1 addition & 1 deletion swarm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name = "libp2p-swarm"
edition = "2021"
rust-version = { workspace = true }
description = "The libp2p swarm"
version = "0.43.4"
version = "0.43.5"
authors = ["Parity Technologies <admin@parity.io>"]
license = "MIT"
repository = "https://github.com/libp2p/rust-libp2p"
Expand Down
43 changes: 35 additions & 8 deletions swarm/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,15 +361,16 @@ where
}
}
(_, KeepAlive::Until(earliest_shutdown)) => {
if let Some(requested_keep_alive) =
earliest_shutdown.checked_duration_since(Instant::now())
{
let effective_keep_alive = max(requested_keep_alive, *idle_timeout);
let now = Instant::now();

if let Some(requested) = earliest_shutdown.checked_duration_since(now) {
let effective_keep_alive = max(requested, *idle_timeout);

let safe_keep_alive = checked_add_fraction(now, effective_keep_alive);

// Important: We store the _original_ `Instant` given by the `ConnectionHandler` in the `Later` instance to ensure we can compare it in the above branch.
// This is quite subtle but will hopefully become simpler soon once `KeepAlive::Until` is fully deprecated. See <https://github.com/libp2p/rust-libp2p/issues/3844>/
*shutdown =
Shutdown::Later(Delay::new(effective_keep_alive), earliest_shutdown)
*shutdown = Shutdown::Later(Delay::new(safe_keep_alive), earliest_shutdown)
}
}
(_, KeepAlive::No) if idle_timeout == &Duration::ZERO => {
Expand All @@ -379,8 +380,10 @@ where
// Do nothing, i.e. let the shutdown timer continue to tick.
}
(_, KeepAlive::No) => {
let deadline = Instant::now() + *idle_timeout;
*shutdown = Shutdown::Later(Delay::new(*idle_timeout), deadline);
let now = Instant::now();
let safe_keep_alive = checked_add_fraction(now, *idle_timeout);

*shutdown = Shutdown::Later(Delay::new(safe_keep_alive), now + safe_keep_alive);
}
(_, KeepAlive::Yes) => *shutdown = Shutdown::None,
};
Expand Down Expand Up @@ -479,6 +482,20 @@ fn gather_supported_protocols(handler: &impl ConnectionHandler) -> HashSet<Strea
.collect()
}

/// Repeatedly halves and adds the [`Duration`] to the [`Instant`] until [`Instant::checked_add`] succeeds.
///
/// [`Instant`] depends on the underlying platform and has a limit of which points in time it can represent.
/// The [`Duration`] computed by the this function may not be the longest possible that we can add to `now` but it will work.
fn checked_add_fraction(start: Instant, mut duration: Duration) -> Duration {
while start.checked_add(duration).is_none() {
log::debug!("{start:?} + {duration:?} cannot be presented, halving duration");

duration /= 2;
}

duration
}

/// Borrowed information about an incoming connection currently being negotiated.
#[derive(Debug, Copy, Clone)]
pub(crate) struct IncomingInfo<'a> {
Expand Down Expand Up @@ -957,6 +974,16 @@ mod tests {
));
}

#[test]
fn checked_add_fraction_can_add_u64_max() {
let _ = env_logger::try_init();
let start = Instant::now();

let duration = checked_add_fraction(start, Duration::from_secs(u64::MAX));

assert!(start.checked_add(duration).is_some())
}

struct KeepAliveUntilConnectionHandler {
until: Instant,
}
Expand Down
Loading