Skip to content

Commit

Permalink
Fix possible arithmetic overflow in libp2p-kad. (libp2p#1291)
Browse files Browse the repository at this point in the history
When the number of active queries exceeds the (internal)
JOBS_MAX_QUERIES limit, which is only supposed to bound
the number of concurrent queries relating to background
jobs, an arithmetic overflow occurs. This is fixed by
using saturating subtraction.
  • Loading branch information
romanb authored and tomaka committed Nov 6, 2019
1 parent b549a6f commit 0980f24
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
2 changes: 1 addition & 1 deletion protocols/kad/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1313,7 +1313,7 @@ where
let now = Instant::now();

// Calculate the available capacity for queries triggered by background jobs.
let mut jobs_query_capacity = JOBS_MAX_QUERIES - self.queries.size();
let mut jobs_query_capacity = JOBS_MAX_QUERIES.saturating_sub(self.queries.size());

// Run the periodic provider announcement job.
if let Some(mut job) = self.add_provider_job.take() {
Expand Down
31 changes: 31 additions & 0 deletions protocols/kad/src/behaviour/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -623,3 +623,34 @@ fn add_provider() {
QuickCheck::new().tests(3).quickcheck(prop as fn(_,_))
}

/// User code should be able to start queries beyond the internal
/// query limit for background jobs. Originally this even produced an
/// arithmetic overflow, see https://github.com/libp2p/rust-libp2p/issues/1290.
#[test]
fn exceed_jobs_max_queries() {
let (_, mut swarms) = build_nodes(1);
let num = JOBS_MAX_QUERIES + 1;
for _ in 0 .. num {
swarms[0].bootstrap();
}

assert_eq!(swarms[0].queries.size(), num);

current_thread::run(
future::poll_fn(move || {
for _ in 0 .. num {
// There are no other nodes, so the queries finish instantly.
if let Ok(Async::Ready(Some(e))) = swarms[0].poll() {
if let KademliaEvent::BootstrapResult(r) = e {
assert!(r.is_ok(), "Unexpected error")
} else {
panic!("Unexpected event: {:?}", e)
}
} else {
panic!("Expected event")
}
}
Ok(Async::Ready(()))
}))
}

0 comments on commit 0980f24

Please sign in to comment.