Skip to content

TOCTOU between has_slot check and acquire in priority-4 enqueue allows active_total to exceed max_concurrent #425

Description

@forkwright

Finding

The interactive (priority-4) fast-path in enqueue checks has_slot under one lock acquisition, drops the lock, then re-acquires it to call acquire. The slot state can change in the gap. Two concurrent priority-4 enqueue calls can both observe an available slot while the first lock is held, both release, then both re-acquire and both call acquire, pushing active_total past max_concurrent. A concurrent on_download_completed in the same window has the same effect.

Evidence

crates/syntaxis/src/lib.rs:393:

let slot_available = {
    let inner = self.inner.lock().await;
    inner.allocator.has_slot(&item)
}; // lock released here (line 395)

if slot_available {
    let mut inner = self.inner.lock().await; // re-acquired at line 401
    inner.allocator.acquire(&item);

The lock guarding the allocator is dropped between the has_slot check (line 393) and the acquire call (line 401), so the check is not atomic with the acquisition it gates. Both line 393 (has_slot observation) and line 395 (lock drop) are the window boundaries.

Why this matters

The slot allocator is the only mechanism bounding concurrent downloads. Exceeding max_concurrent silently starts extra downloads, breaching the rate limit that protects tracker relationships and local disk/network/battery budget. Under concurrent interactive requests, max_concurrent_downloads is violated, which can saturate bandwidth and oversubscribe tracker connections. An uncontrolled burst of simultaneous connections is precisely the anomalous traffic spike that distinguishes the device from a quiet baseline and can trip tracker abuse heuristics.

Desired correction

Collapse the check and the acquire into a single critical section: hold the lock, read has_slot, and if true call acquire before releasing. Merge the second inner.lock() block at line 401 into the first at line 394 so no window exists between observation and acquisition. Done when: a concurrent stress test driving many simultaneous priority-4 enqueue calls confirms active_total never exceeds max_concurrent.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions