linux-hardening - Re: [PATCH v9 3/5] rust: add bitmap API.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aDi-edjArUXl7Mq6@Mac.home>
Date: Thu, 29 May 2025 13:07:21 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Pekka Ristola <pekkarr@...tonmail.com>
Cc: Burak Emir <bqe@...gle.com>, Yury Norov <yury.norov@...il.com>,
	Kees Cook <kees@...nel.org>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	"Gustavo A . R . Silva" <gustavoars@...nel.org>,
	rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-hardening@...r.kernel.org
Subject: Re: [PATCH v9 3/5] rust: add bitmap API.

On Thu, May 29, 2025 at 07:42:09PM +0000, Pekka Ristola wrote:
[...]
> > +    }
> > +
> > +    /// Copy `src` into this [`Bitmap`] and set any remaining bits to zero.
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// ```
> > +    /// use kernel::alloc::{AllocError, flags::GFP_KERNEL};
> > +    /// use kernel::bitmap::Bitmap;
> > +    ///
> > +    /// let mut long_bitmap = Bitmap::new(256, GFP_KERNEL)?;
> > +    //
> > +    /// assert_eq!(None, long_bitmap.last_bit());
> > +    //
> > +    /// let mut short_bitmap = Bitmap::new(16, GFP_KERNEL)?;
> > +    //
> > +    /// short_bitmap.set_bit(7);
> > +    /// long_bitmap.copy_and_extend(&short_bitmap);
> > +    /// assert_eq!(Some(7), long_bitmap.last_bit());
> > +    ///
> > +    /// # Ok::<(), AllocError>(())
> > +    /// ```
> > +    #[inline]
> > +    pub fn copy_and_extend(&mut self, src: &Bitmap) {
> > +        let len = core::cmp::min(src.nbits, self.len());
> > +        // SAFETY: access to `self` and `src` is within bounds.
> > +        unsafe {
> > +            bindings::bitmap_copy_and_extend(
> > +                self.as_mut_ptr(),
> > +                src.as_ptr(),
> > +                len as u32,
> > +                self.len() as u32,
> > +            )
> 
> Would this cause a data race if `src` is concurrently (atomically)
> modified? The C function seems to use a plain `memcpy` which is not atomic.
> 

We need some better documentation on the effect of kernel C's
`memcpy()`-like functions regarding data races, but in general a kernel
C's `memcpy` can be treated as a volatile one (or per-byte atomic), so
it won't cause data race.

> > +        };
> > +    }
> > +
> > +    /// Finds last set bit.
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// ```
> > +    /// use kernel::alloc::{AllocError, flags::GFP_KERNEL};
> > +    /// use kernel::bitmap::Bitmap;
> > +    ///
> > +    /// let bitmap = Bitmap::new(64, GFP_KERNEL)?;
> > +    ///
> > +    /// match bitmap.last_bit() {
> > +    ///     Some(idx) => {
> > +    ///         pr_info!("The last bit has index {idx}.\n");
> > +    ///     }
> > +    ///     None => {
> > +    ///         pr_info!("All bits in this bitmap are 0.\n");
> > +    ///     }
> > +    /// }
> > +    /// # Ok::<(), AllocError>(())
> > +    /// ```
> > +    #[inline]
> > +    pub fn last_bit(&self) -> Option<usize> {
> > +        // SAFETY: `_find_next_bit` access is within bounds due to invariant.
> > +        let index = unsafe { bindings::_find_last_bit(self.as_ptr(), self.len()) };
> 
> The C function uses non-atomic reads, so this might cause data races too.
> 

Similar here.

Regards,
Boqun

> > +        if index >= self.len() {
> > +            None
> > +        } else {
> > +            Some(index)
> > +        }
> > +    }
> 
> Pekka
>