Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] rust: id_pool: panic in find_unused_id() on small or zero capacity pools --
commit 018a4fbd0a161c3060c7581d4c4fd80bc70dcd41 Author: Eliot Courtney <[email protected]> rust: id_pool: do not round capacity up to BitmapVec::MAX_INLINE_LEN This commit stops rounding small capacities up to BitmapVec::MAX_INLINE_LEN in IdPool::with_capacity() so that capacities smaller than MAX_INLINE_LEN, including 0, can be used directly. It also updates IdPool::grow_request() to grow to at least BitmapVec::MAX_INLINE_LEN. > diff --git a/rust/kernel/id_pool.rs b/rust/kernel/id_pool.rs > index 384753fe0e44a..7bb8bb89297f0 100644 > --- a/rust/kernel/id_pool.rs > +++ b/rust/kernel/id_pool.rs > @@ -106,13 +106,8 @@ pub fn new() -> Self { > } > > /// Constructs a new [`IdPool`] with space for a specific number of bits. > - /// > - /// A capacity below [`MAX_INLINE_LEN`] is adjusted to > [`MAX_INLINE_LEN`]. > - /// > - /// [`MAX_INLINE_LEN`]: BitmapVec::MAX_INLINE_LEN > #[inline] > pub fn with_capacity(num_ids: usize, flags: Flags) -> Result<Self, > AllocError> { > - let num_ids = usize::max(num_ids, BitmapVec::MAX_INLINE_LEN); > let map = BitmapVec::new(num_ids, flags)?; > Ok(Self { map }) > } [Severity: Medium] With the capacity floor removed, can calling IdPool::find_unused_id() on a pool created with capacity 0 (or any capacity <= offset) trigger a panic when CONFIG_RUST_BITMAP_HARDENED=y is enabled? In rust/kernel/id_pool.rs, IdPool::find_unused_id() passes offset directly to Bitmap::next_zero_bit() without checking whether offset >= self.capacity(): pub fn find_unused_id(&mut self, offset: usize) -> Option<UnusedId<'_>> { // INVARIANT: `next_zero_bit()` returns None or an integer less than `map.len()` Some(UnusedId { id: self.map.next_zero_bit(offset)?, pool: self, }) } In rust/kernel/bitmap.rs, Bitmap::next_zero_bit() asserts that start is strictly less than self.len(): pub fn next_zero_bit(&self, start: usize) -> Option<usize> { bitmap_assert!( start < self.len(), "`start` must be < {} was {}", self.len(), start ); When a pool is constructed via IdPool::with_capacity(0, flags) and a caller invokes pool.find_unused_id(0) before growing the pool, self.len() is 0 and start < self.len() fails, panicking under CONFIG_RUST_BITMAP_HARDENED=y instead of returning None. The same happens if a caller creates a small pool (for example with capacity 1) and calls find_unused_id(1). [ ... ] > > > > > > > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
