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

Prevent overflow through Arc::downgrade #108708

Merged
merged 1 commit into from
Mar 11, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,16 @@ mod tests;
///
/// Going above this limit will abort your program (although not
/// necessarily) at _exactly_ `MAX_REFCOUNT + 1` references.
/// Trying to go above it might call a `panic` (if not actually going above it).
///
/// This is a global invariant, and also applies when using a compare-exchange loop.
///
/// See comment in `Arc::clone`.
const MAX_REFCOUNT: usize = (isize::MAX) as usize;

/// The error in case either counter reaches above `MAX_REFCOUNT`, and we can `panic` safely.
const INTERNAL_OVERFLOW_ERROR: &str = "Arc counter overflow";

#[cfg(not(sanitize = "thread"))]
macro_rules! acquire {
($x:expr) => {
Expand Down Expand Up @@ -950,6 +958,9 @@ impl<T: ?Sized> Arc<T> {
continue;
}
noamtashma marked this conversation as resolved.
Show resolved Hide resolved

// We can't allow the refcount to increase much past `MAX_REFCOUNT`.
assert!(cur <= MAX_REFCOUNT, "{}", INTERNAL_OVERFLOW_ERROR);

// NOTE: this code currently ignores the possibility of overflow
// into usize::MAX; in general both Rc and Arc need to be adjusted
// to deal with overflow.
Expand Down Expand Up @@ -1373,6 +1384,11 @@ impl<T: ?Sized> Clone for Arc<T> {
// the worst already happened and we actually do overflow the `usize` counter. However, that
// requires the counter to grow from `isize::MAX` to `usize::MAX` between the increment
// above and the `abort` below, which seems exceedingly unlikely.
//
// This is a global invariant, and also applies when using a compare-exchange loop to increment
// counters in other methods.
// Otherwise, the counter could be brought to an almost-overflow using a compare-exchange loop,
// and then overflow using a few `fetch_add`s.
if old_size > MAX_REFCOUNT {
abort();
}
Expand Down Expand Up @@ -2001,9 +2017,7 @@ impl<T: ?Sized> Weak<T> {
return None;
}
// See comments in `Arc::clone` for why we do this (for `mem::forget`).
if n > MAX_REFCOUNT {
abort();
}
assert!(n <= MAX_REFCOUNT, "{}", INTERNAL_OVERFLOW_ERROR);
Some(n + 1)
})
.ok()
Expand Down