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]>
---
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
