Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Integer truncation in `Bitmap::copy_and_extend` leading to out-of-bounds memset in C. -- commit 19918f0914fec684f788000700e6deb8bdb878e0 Author: Eliot Courtney <[email protected]> rust: bitmap: add contiguous area operations This patch adds bindings and helpers for area operations on bitmaps. It makes each operation safe by adding extra bounds checks compared to the underlying C code. Additional checks are included to catch likely erroneous usage when CONFIG_RUST_BITMAP_HARDENED is enabled. > diff --git a/rust/kernel/bitmap.rs b/rust/kernel/bitmap.rs > index a43bfe0ec3dc2..f7290fa439d6f 100644 > --- a/rust/kernel/bitmap.rs > +++ b/rust/kernel/bitmap.rs [ ... ] > @@ -497,6 +497,129 @@ pub fn next_zero_bit(&self, start: usize) -> > Option<usize> { > Some(index) > } > } [ ... ] > + #[inline] > + pub fn set_area(&mut self, start: usize, nbits: usize) { > + bitmap_assert_return!( > + start > + .checked_add(nbits) > + .is_some_and(|end| end <= self.len() && end <= i32::MAX as > usize), [Severity: High] This isn't a bug introduced by this patch, but given your note about bounds checking and 32-bit limits, I noticed a pre-existing issue in Bitmap::copy_and_extend() that might need similar treatment. rust/kernel/bitmap.rs:Bitmap::copy_and_extend() { ... pub fn copy_and_extend(&mut self, src: &Bitmap) { let len = core::cmp::min(src.len(), self.len()); // SAFETY: access to self and src is within bounds. unsafe { bindings::bitmap_copy_and_extend( self.as_mut_ptr(), src.as_ptr(), len as u32, self.len() as u32, ) }; } } Can this lead to an out-of-bounds memset in C? A Bitmap can be created from a raw pointer using Bitmap::from_raw() without u32::MAX length restrictions. If self.len() exceeds u32::MAX on 64-bit systems, the self.len() as u32 cast will silently truncate the length. If src.len() is smaller than the full length but larger than the truncated length, count will be greater than size when passed to the C function bitmap_copy_and_extend(). The C function calculates the byte delta for memset as: bitmap_size(size) - copy * sizeof(long) With a truncated size, this would subtract a larger byte count from a smaller byte count, underflowing to a huge size_t value. Could this cause a massive out-of-bounds write and kernel panic? > + "Area `start..start + nbits` ({}..{}) must be within bounds {}", > + start, > + start.saturating_add(nbits), > + self.len() > + ); > + // SAFETY: The area `start..start + nbits` is within bounds. > + unsafe { bindings::bitmap_set(self.as_mut_ptr(), start as u32, nbits > as u32) }; > + } [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
