Re: [PATCH v7 05/10] rust: bitmap: add contiguous area operations
"Alexandre Courbot" <[email protected]>
| Newsgroups | dev.linux.lists.nova-gpu,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel,org.kernel.vger.rust-for-linux |
|---|---|
| Message-ID | <[email protected]> |
On Mon Aug 17, 2026 at 4:04 PM JST, Eliot Courtney wrote: > Add bindings for area operations on bitmaps. Each one is > made safe by adding some extra checks compared to the underlying C code > (for example, checking bounds) and with additional checks to catch > likely erroneous usage if `CONFIG_RUST_BITMAP_HARDENED` is on. > > Add tests demonstrating the edge cases. > > Signed-off-by: Eliot Courtney <[email protected]> > --- > rust/kernel/bitmap.rs | 242 +++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 240 insertions(+), 2 deletions(-) > > diff --git a/rust/kernel/bitmap.rs b/rust/kernel/bitmap.rs > index fdcfc0409773..a4997022ff0f 100644 > --- a/rust/kernel/bitmap.rs > +++ b/rust/kernel/bitmap.rs > @@ -10,7 +10,11 @@ > use crate::bindings; > #[cfg(not(CONFIG_RUST_BITMAP_HARDENED))] > use crate::pr_err; > -use core::ptr::NonNull; > +use crate::ptr::Alignment; > +use core::{ > + num::NonZero, > + ptr::NonNull, // > +}; > > /// Represents a C bitmap. Wraps underlying C bitmap API. > /// > @@ -523,13 +527,160 @@ pub fn next_zero_bit(&self, start: usize) -> Option<usize> { > Some(index) > } > } > + > + /// Finds a contiguous area of `nbits` zero bits at or after `start`, where the area plus > + /// `align_offset` is aligned to `align`. > + /// > + /// Returns the bit index of the start of the area, or [`None`] if no such area fitting in > + /// the bitmap exists. > + /// > + /// The returned index plus `align_offset` is a multiple of `align`. > + /// > + /// # Panics > + /// > + /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and `start` is out of bounds. > + #[inline] > + pub fn next_zero_area_off( > + &self, > + start: usize, > + nbits: NonZero<usize>, > + align: Alignment, > + align_offset: usize, > + ) -> Option<usize> { > + bitmap_assert!( > + start < self.len(), > + "`start` must be < {}, was {}", > + self.len(), > + start > + ); Do we need to potentially panic here if `start >= self.len()`? The question "is there an area of `nbits` bits after my bounds" can be answered by "there is `None`" without semantically sounding weird; and this test doesn't cover `start + nbits >= self.len()`, which should logically also be considered to be consistent. It seems like the C API also tolerates this, so as this is not a safety issue I guess the Rust one should do the same? If anything I'd say we should remove these tests from `next_bit`/`next_zero_bit` as well. Mutating methods should definitely keep that check, but for querying this looks like a legitimate way to use the API. > + > + let nr = u32::try_from(nbits.get()).ok()?; > + let align_mask = align.as_usize() - 1; > + > + // The C alignment and end arithmetic must not overflow, or it can read out of bounds. > + // Overflow is only possible on 32-bit. > + #[cfg(not(CONFIG_64BIT))] > + align_mask > + .checked_add(self.len())? > + .checked_add(nbits.get())?; Is it ok to not consider `align_offset` here? The C code adds it, and the result could overflow on large values, even on 64-bit.