Re: [PATCH v5 2/5] rust: bitmap: restrict bitmap length to at most i32::MAX

Yury Norov <[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 <anzNKIX3V_zf2O8C@yury>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.