Re: [PATCH v4 3/5] rust: bitmap: add contiguous area operations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The overflow check `align_mask.checked_add(self.len())?.checked_add(nbits)?` incorrectly rejects valid `next_zero_area_off` queries on 32-bit architectures when `self.len()` and `nbits` are large.
- [Medium] The assertion `start < self.len()` in `next_zero_area_off` panics if `start == self.len()`, which breaks iterators and empty bitmaps.
--

--- Patch [3]: [PATCH v4 3/5] rust: bitmap: add contiguous area operations ---
commit cd8a17b1e3284bd61e474e6a67facb0e46c2ec8a
Author: Eliot Courtney <ecourtney@nvidia.com>

rust: bitmap: add contiguous area operations

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. Tests demonstrating the edge cases are
also included.

> diff --git a/rust/kernel/bitmap.rs b/rust/kernel/bitmap.rs
> index 0d481d761f2ac..a2557e9c5cfe0 100644
> --- a/rust/kernel/bitmap.rs
> +++ b/rust/kernel/bitmap.rs

[ ... ]

> @@ -504,6 +504,138 @@ 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: usize,
> +        align: Alignment,
> +        align_offset: usize,
> +    ) -> Option<usize> {
> +        bitmap_assert!(
> +            start < self.len(),

[Severity: Medium]
Does this assertion break iterators and empty bitmaps when
CONFIG_RUST_BITMAP_HARDENED is enabled?

If a caller iteratively searches for zero areas (e.g., start = area_index +
nbits) and the last found area ends exactly at the bitmap boundary, the next
iteration will query with start == self.len(). Also, querying an empty bitmap
(self.len() == 0) with start = 0 would trigger this panic.

The underlying C function bitmap_find_next_zero_area_off() safely processes
start >= size by returning size, which the Rust code gracefully transforms to
None.

Could we use start <= self.len() here instead, similar to how the set() and
clear() functions allow end <= self.len()?

> +            "`start` must be < {}, was {}",
> +            self.len(),
> +            start
> +        );
> +
> +        let nr = u32::try_from(nbits).ok()?;
> +
> +        // The C alignment and end arithmetic must not overflow, or it can read out of bounds.
> +        // Overflow is only possible on 32-bit.
> +        let align_mask = align.as_usize() - 1;
> +        align_mask.checked_add(self.len())?.checked_add(nbits)?;

[Severity: Medium]
Is this overflow check overly strict for 32-bit architectures?

If a caller requests a large contiguous area in a large bitmap, such as
self.len() = 0x7FFFFFFF and nbits = 0x7FFFFFFF, the sum of align_mask +
self.len() + nbits can exceed u32::MAX. This would cause the checked_add()
chain to fail and incorrectly return None instead of executing the search.

Since we only need to prevent the C function's end = index + nr calculation
from wrapping around maliciously, and we know nbits <= self.len() makes nr at
most 0x7FFFFFFF, any wrap-around of index + nr on 32-bit would mathematically
result in an end value strictly less than index. The C code handles end < index
safely by returning index, which Rust then correctly rejects since
index < self.len() is false.

Would a simpler check like "if nbits > self.len() { return None; }" be
sufficient to prevent malicious wrap-around while preserving valid searches
for large boundaries?

> +
> +        // SAFETY: `bitmap_find_next_zero_area_off` is safe to use with an out of bounds `start`
> +        // value and, given the overflow check above, never reads beyond `self.len()` bits.
> +        let index = unsafe {
> +            bindings::bitmap_find_next_zero_area_off(
> +                self.as_ptr().cast_mut(),

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-chid-v4-0-c9f206fdcb97@nvidia.com?part=3