Skip to content

Commit e0ea5f8

Browse files
committed
spsc: fix potential overflows
Thanks Jannic for the find: #652 (comment) If N == usize::MAX, there is the possibility of a panic in len() If N >= usize::MAX, then in the iterator code, self.index + self.head could overflow The operations are now slightly more complex and a bit slower, but thanks to compiler optimization they don't introduce branches, only conditional instructions (cmov, csel, it). All added tests fail without the fix
1 parent 0950363 commit e0ea5f8

File tree

2 files changed

+89
-8
lines changed

2 files changed

+89
-8
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ of panicking drop implementations.
1414
- Added `pop_front_if` and `pop_back_if` to `Deque`
1515
- Made `Vec::from_array` const.
1616
- Fixed long division being instroduced by the const-erasure in spsc
17+
- spsc: Fix integer overflow in iterators when N > usize::MAX/2 and the queue loops.
18+
- spsc: Fix integer overflow leading to a panic in `len` when N == usize::MAX and debug assertions are enabled.
1719

1820
## [v0.9.2] 2025-11-12
1921

src/spsc.rs

Lines changed: 87 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,10 @@ impl<T, S: Storage> QueueInner<T, S> {
176176

177177
#[inline]
178178
fn increment(&self, val: usize) -> usize {
179-
let val = val + 1;
179+
// We know that self.n() <= usize::MAX
180+
// So this can only overflow if N == usize::MAX
181+
// and in this case the overflow will be equivalent to the modulo N operation
182+
let val = val.wrapping_add(1);
180183
if val >= self.n() {
181184
val - self.n()
182185
} else {
@@ -628,8 +631,11 @@ impl<'a, T> Iterator for Iter<'a, T> {
628631
if self.index < self.len {
629632
let head = self.rb.head.load(Ordering::Relaxed);
630633

631-
let i = head + self.index;
632-
let i = if i >= self.rb.n() { i - self.rb.n() } else { i };
634+
let i = match head.checked_add(self.index) {
635+
Some(i) if i >= self.rb.n() => i - self.rb.n(),
636+
Some(i) => i,
637+
None => head.wrapping_add(self.index).wrapping_sub(self.rb.n()),
638+
};
633639
self.index += 1;
634640

635641
Some(unsafe { &*(self.rb.buffer.borrow().get_unchecked(i).get() as *const T) })
@@ -646,8 +652,11 @@ impl<'a, T> Iterator for IterMut<'a, T> {
646652
if self.index < self.len {
647653
let head = self.rb.head.load(Ordering::Relaxed);
648654

649-
let i = head + self.index;
650-
let i = if i >= self.rb.n() { i - self.rb.n() } else { i };
655+
let i = match head.checked_add(self.index) {
656+
Some(i) if i >= self.rb.n() => i - self.rb.n(),
657+
Some(i) => i,
658+
None => head.wrapping_add(self.index).wrapping_sub(self.rb.n()),
659+
};
651660
self.index += 1;
652661

653662
Some(unsafe { &mut *self.rb.buffer.borrow().get_unchecked(i).get().cast::<T>() })
@@ -663,8 +672,11 @@ impl<T> DoubleEndedIterator for Iter<'_, T> {
663672
let head = self.rb.head.load(Ordering::Relaxed);
664673

665674
// self.len > 0, since it's larger than self.index > 0
666-
let i = head + self.len - 1;
667-
let i = if i >= self.rb.n() { i - self.rb.n() } else { i };
675+
let i = match head.checked_add(self.len - 1) {
676+
Some(i) if i >= self.rb.n() => i - self.rb.n(),
677+
Some(i) => i,
678+
None => head.wrapping_add(self.len - 1).wrapping_sub(self.rb.n()),
679+
};
668680
self.len -= 1;
669681
Some(unsafe { &*(self.rb.buffer.borrow().get_unchecked(i).get() as *const T) })
670682
} else {
@@ -678,6 +690,7 @@ impl<T> DoubleEndedIterator for IterMut<'_, T> {
678690
if self.index < self.len {
679691
let head = self.rb.head.load(Ordering::Relaxed);
680692

693+
// TODO: fix usize overflow
681694
// self.len > 0, since it's larger than self.index > 0
682695
let i = head + self.len - 1;
683696
let i = if i >= self.rb.n() { i - self.rb.n() } else { i };
@@ -888,7 +901,12 @@ impl<T> Producer<'_, T> {
888901

889902
#[cfg(test)]
890903
mod tests {
891-
use std::hash::{Hash, Hasher};
904+
use std::{
905+
cell::UnsafeCell,
906+
hash::{Hash, Hasher},
907+
mem::MaybeUninit,
908+
sync::atomic::AtomicUsize,
909+
};
892910

893911
use super::{Consumer, Producer, Queue};
894912

@@ -1310,4 +1328,65 @@ mod tests {
13101328
};
13111329
assert_eq!(hash1, hash2);
13121330
}
1331+
1332+
// Test for some integer overflow bugs. See
1333+
// https://github.com/rust-embedded/heapless/pull/652#discussion_r3046630717
1334+
// for more info
1335+
#[test]
1336+
fn test_len_overflow() {
1337+
let mut queue = Queue::<(), { usize::MAX }> {
1338+
head: AtomicUsize::new(usize::MAX),
1339+
tail: AtomicUsize::new(2),
1340+
buffer: [const { UnsafeCell::new(MaybeUninit::new(())) }; usize::MAX],
1341+
};
1342+
queue.enqueue(()).unwrap();
1343+
queue.enqueue(()).unwrap();
1344+
1345+
let collected: Vec<_> = queue.iter().collect();
1346+
assert_eq!(&collected, &[&(); 4]);
1347+
}
1348+
#[test]
1349+
fn test_usize_overflow_iter() {
1350+
let queue = Queue::<(), { usize::MAX - 1 }> {
1351+
head: AtomicUsize::new(usize::MAX - 3),
1352+
tail: AtomicUsize::new(2),
1353+
buffer: [const { UnsafeCell::new(MaybeUninit::new(())) }; usize::MAX - 1],
1354+
};
1355+
1356+
let collected: Vec<_> = queue.iter().collect();
1357+
assert_eq!(&collected, &[&(); 4]);
1358+
}
1359+
#[test]
1360+
fn test_usize_overflow_iter_mut() {
1361+
let mut queue = Queue::<(), { usize::MAX - 1 }> {
1362+
head: AtomicUsize::new(usize::MAX - 3),
1363+
tail: AtomicUsize::new(2),
1364+
buffer: [const { UnsafeCell::new(MaybeUninit::new(())) }; usize::MAX - 1],
1365+
};
1366+
1367+
let collected: Vec<_> = queue.iter_mut().collect();
1368+
assert_eq!(&collected, &[&(); 4]);
1369+
}
1370+
#[test]
1371+
fn test_usize_overflow_iter_rev() {
1372+
let queue = Queue::<(), { usize::MAX - 1 }> {
1373+
head: AtomicUsize::new(usize::MAX - 3),
1374+
tail: AtomicUsize::new(2),
1375+
buffer: [const { UnsafeCell::new(MaybeUninit::new(())) }; usize::MAX - 1],
1376+
};
1377+
1378+
let collected: Vec<_> = queue.iter().rev().collect();
1379+
assert_eq!(&collected, &[&(); 4]);
1380+
}
1381+
#[test]
1382+
fn test_usize_overflow_iter_mut_rev() {
1383+
let mut queue = Queue::<(), { usize::MAX - 1 }> {
1384+
head: AtomicUsize::new(usize::MAX - 3),
1385+
tail: AtomicUsize::new(2),
1386+
buffer: [const { UnsafeCell::new(MaybeUninit::new(())) }; usize::MAX - 1],
1387+
};
1388+
1389+
let collected: Vec<_> = queue.iter_mut().rev().collect();
1390+
assert_eq!(&collected, &[&(); 4]);
1391+
}
13131392
}

0 commit comments

Comments
 (0)