On Wed, Aug 12, 2026 at 05:51:22PM +0900, Eliot Courtney wrote:
> It is currently possible to construct a non-`BitmapVec` backed
> `Bitmap` using `Bitmap::from_raw` that is larger than `i32::MAX`, and
> it is not part of the unsafe requirements. Restricting all bitmaps
> (even non-`BitmapVec` backed ones) to a maximum size of `i32::MAX`
> simplifies a few things and matches `BitmapVec::MAX_LEN`.
> 
> Add that requirement to the unsafe requirements on `Bitmap::from_raw`
> and `Bitmap::from_raw_mut`, and to the invariants on `Bitmap`.
> 
> This also fixes u32 casts truncating in `copy_and_extend`, which could
> otherwise lead to OOB writes.
> 
> Fixes: 11eca92a2cae ("rust: add bitmap API.")
> Link: https://lore.kernel.org/[email protected]
> Signed-off-by: Eliot Courtney <[email protected]>

Reviewed-by: Yury Norov <[email protected]>

> ---
>  rust/kernel/bitmap.rs | 68 
> +++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 47 insertions(+), 21 deletions(-)
> 
> diff --git a/rust/kernel/bitmap.rs b/rust/kernel/bitmap.rs
> index a43bfe0ec3dc..fdcfc0409773 100644
> --- a/rust/kernel/bitmap.rs
> +++ b/rust/kernel/bitmap.rs
> @@ -17,24 +17,57 @@
>  /// # Invariants
>  ///
>  /// Must reference a `[c_ulong]` long enough to fit `data.len()` bits.
> +/// Must not be longer than `i32::MAX` bits.
>  #[cfg_attr(CONFIG_64BIT, repr(align(8)))]
>  #[cfg_attr(not(CONFIG_64BIT), repr(align(4)))]
>  pub struct Bitmap {
>      data: [()],
>  }
>  
> +macro_rules! bitmap_assert {
> +    ($cond:expr, $($arg:tt)+) => {
> +        #[cfg(CONFIG_RUST_BITMAP_HARDENED)]
> +        assert!($cond, $($arg)*);
> +    }
> +}
> +
> +macro_rules! bitmap_assert_return {
> +    ($cond:expr, $($arg:tt)+) => {
> +        #[cfg(CONFIG_RUST_BITMAP_HARDENED)]
> +        assert!($cond, $($arg)*);
> +
> +        #[cfg(not(CONFIG_RUST_BITMAP_HARDENED))]
> +        if !($cond) {
> +            pr_err!($($arg)*);
> +            return
> +        }
> +    }
> +}
> +
>  impl Bitmap {
>      /// Borrows a C bitmap.
>      ///
> +    /// # Panics
> +    ///
> +    /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and `nbits` exceeds 
> `i32::MAX`.
> +    ///
>      /// # Safety
>      ///
>      /// * `ptr` holds a non-null address of an initialized array of 
> `unsigned long`
>      ///   that is large enough to hold `nbits` bits.
> +    /// * `nbits` must not exceed `i32::MAX`.
>      /// * the array must not be freed for the lifetime of this [`Bitmap`]
>      /// * concurrent access only happens through atomic operations
>      pub unsafe fn from_raw<'a>(ptr: *const usize, nbits: usize) -> &'a 
> Bitmap {
> +        bitmap_assert!(
> +            nbits <= i32::MAX as usize,
> +            "`nbits` must be <= {}, was {}",
> +            i32::MAX,
> +            nbits
> +        );
>          let data: *const [()] = core::ptr::slice_from_raw_parts(ptr.cast(), 
> nbits);
>          // INVARIANT: `data` references an initialized array that can hold 
> `nbits` bits.
> +        // INVARIANT: the caller guarantees that `nbits` does not exceed 
> `i32::MAX`.
>          // SAFETY:
>          // The caller guarantees that `data` (derived from `ptr` and `nbits`)
>          // points to a valid, initialized, and appropriately sized memory 
> region
> @@ -51,15 +84,27 @@ pub unsafe fn from_raw<'a>(ptr: *const usize, nbits: 
> usize) -> &'a Bitmap {
>  
>      /// Borrows a C bitmap exclusively.
>      ///
> +    /// # Panics
> +    ///
> +    /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and `nbits` exceeds 
> `i32::MAX`.
> +    ///
>      /// # Safety
>      ///
>      /// * `ptr` holds a non-null address of an initialized array of 
> `unsigned long`
>      ///   that is large enough to hold `nbits` bits.
> +    /// * `nbits` must not exceed `i32::MAX`.
>      /// * the array must not be freed for the lifetime of this [`Bitmap`]
>      /// * no concurrent access may happen.
>      pub unsafe fn from_raw_mut<'a>(ptr: *mut usize, nbits: usize) -> &'a mut 
> Bitmap {
> +        bitmap_assert!(
> +            nbits <= i32::MAX as usize,
> +            "`nbits` must be <= {}, was {}",
> +            i32::MAX,
> +            nbits
> +        );
>          let data: *mut [()] = 
> core::ptr::slice_from_raw_parts_mut(ptr.cast(), nbits);
>          // INVARIANT: `data` references an initialized array that can hold 
> `nbits` bits.
> +        // INVARIANT: the caller guarantees that `nbits` does not exceed 
> `i32::MAX`.
>          // SAFETY:
>          // The caller guarantees that `data` (derived from `ptr` and `nbits`)
>          // points to a valid, initialized, and appropriately sized memory 
> region
> @@ -96,26 +141,6 @@ union BitmapRepr {
>      ptr: NonNull<usize>,
>  }
>  
> -macro_rules! bitmap_assert {
> -    ($cond:expr, $($arg:tt)+) => {
> -        #[cfg(CONFIG_RUST_BITMAP_HARDENED)]
> -        assert!($cond, $($arg)*);
> -    }
> -}
> -
> -macro_rules! bitmap_assert_return {
> -    ($cond:expr, $($arg:tt)+) => {
> -        #[cfg(CONFIG_RUST_BITMAP_HARDENED)]
> -        assert!($cond, $($arg)*);
> -
> -        #[cfg(not(CONFIG_RUST_BITMAP_HARDENED))]
> -        if !($cond) {
> -            pr_err!($($arg)*);
> -            return
> -        }
> -    }
> -}
> -
>  /// Represents an owned bitmap.
>  ///
>  /// Wraps underlying C bitmap API. See [`Bitmap`] for available
> @@ -415,7 +440,8 @@ pub fn clear_bit_atomic(&self, index: usize) {
>      #[inline]
>      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.
> +        // SAFETY: access to `self` and `src` is within bounds. Both lengths 
> fit in `u32`
> +        // because a `Bitmap` is at most `i32::MAX` bits, so the casts are 
> lossless.
>          unsafe {
>              bindings::bitmap_copy_and_extend(
>                  self.as_mut_ptr(),
> 
> -- 
> 2.55.0

Reply via email to