| 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
| ||
|
Message-ID: <CAJ-ks9=WhEyMexy47gWRCBLmje8PUoaJKeNbjS53dfnQzPAPAQ@mail.gmail.com>
Date: Mon, 29 Dec 2025 11:29:41 -0500
From: Tamir Duberstein <tamird@...il.com>
To: Andreas Hindborg <a.hindborg@...nel.org>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <lossin@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, Daniel Gomez <da.gomez@...nel.org>,
rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 10/10] rust: xarray: fix false positive lockdep warnings
On Wed, Dec 3, 2025 at 5:28 PM Andreas Hindborg <a.hindborg@...nel.org> wrote:
>
> Replace the `xa_init_flags` helper with direct initialization of XArray
> structures using `__spin_lock_init`. This allows each XArray to have
> its own lock class key for lockdep, preventing false positive warnings
> about lock ordering violations.
>
> Add a `new_xarray!` macro that automatically generates a unique lock
> class key for each XArray instantiation site. The macro accepts an
> optional name parameter and uses the `optional_name!` and
> `static_lock_class!` macros to generate appropriate names and lock
> classes.
>
> Update the `XArray::new` method signature to accept explicit name and
> lock class key parameters. This enables proper lockdep tracking while
> maintaining flexibility for advanced use cases.
>
> Remove `rust_helper_xa_init_flags` as it is now dead code.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@...nel.org>
> ---
> rust/helpers/xarray.c | 5 ---
> rust/kernel/xarray.rs | 74 +++++++++++++++++++++++++++++++++------------
> rust/kernel/xarray/entry.rs | 55 ++++++++++++++++++---------------
> 3 files changed, 86 insertions(+), 48 deletions(-)
>
> diff --git a/rust/helpers/xarray.c b/rust/helpers/xarray.c
> index 425a6cc494734..2852f63388eea 100644
> --- a/rust/helpers/xarray.c
> +++ b/rust/helpers/xarray.c
> @@ -7,11 +7,6 @@ int rust_helper_xa_err(void *entry)
> return xa_err(entry);
> }
>
> -void rust_helper_xa_init_flags(struct xarray *xa, gfp_t flags)
> -{
> - return xa_init_flags(xa, flags);
> -}
> -
> int rust_helper_xa_trylock(struct xarray *xa)
> {
> return xa_trylock(xa);
> diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs
> index a405f2b6fdcad..c393d4c01af2a 100644
> --- a/rust/kernel/xarray.rs
> +++ b/rust/kernel/xarray.rs
> @@ -29,6 +29,8 @@
> Result, //
> },
> ffi::c_void,
> + str::CStr,
> + sync::LockClassKey,
> types::{
> ForeignOwnable,
> NotThreadSafe,
> @@ -48,6 +50,19 @@
>
> mod preload;
>
> +/// Creates a [`XArray`] initialiser with the given name and a newly-created lock class.
> +///
> +/// It uses the name if one is given, otherwise it generates one based on the file name and line
> +/// number.
> +#[macro_export]
> +macro_rules! new_xarray {
> + ($kind:expr $(, $name:literal)? $(,)?) => {
> + $crate::xarray::XArray::new(
> + $kind, $crate::optional_name!($($name)?), $crate::static_lock_class!())
> + };
> +}
> +pub use new_xarray;
> +
> /// An array which efficiently maps sparse integer indices to owned objects.
> ///
> /// This is similar to a [`crate::alloc::kvec::Vec<Option<T>>`], but more efficient when there are
> @@ -62,9 +77,10 @@
> ///
> /// ```rust
> /// use kernel::alloc::KBox;
> -/// use kernel::xarray::{AllocKind, XArray};
> +/// use kernel::xarray::{new_xarray, AllocKind, XArray};
> ///
> -/// let xa = KBox::pin_init(XArray::new(AllocKind::Alloc1), GFP_KERNEL)?;
> +/// let xa: Pin<KBox<XArray<KBox<u32>>>> =
> +/// KBox::pin_init(new_xarray!(AllocKind::Alloc1), GFP_KERNEL)?;
> ///
> /// let dead = KBox::new(0xdead, GFP_KERNEL)?;
> /// let beef = KBox::new(0xbeef, GFP_KERNEL)?;
> @@ -124,7 +140,11 @@ pub enum AllocKind {
>
> impl<T: ForeignOwnable> XArray<T> {
> /// Creates a new initializer for this type.
Should this be #[doc(hidden)] now?
> - pub fn new(kind: AllocKind) -> impl PinInit<Self> {
> + pub fn new(
> + kind: AllocKind,
> + name: &'static CStr,
> + key: Pin<&'static LockClassKey>,
> + ) -> impl PinInit<Self> {
> let flags = match kind {
> AllocKind::Alloc => bindings::XA_FLAGS_ALLOC,
> AllocKind::Alloc1 => bindings::XA_FLAGS_ALLOC1,
> @@ -133,8 +153,14 @@ pub fn new(kind: AllocKind) -> impl PinInit<Self> {
> // SAFETY: `xa` is valid while the closure is called.
> //
> // INVARIANT: `xa` is initialized here to an empty, valid [`bindings::xarray`].
> - xa <- Opaque::ffi_init(|xa| unsafe {
> - bindings::xa_init_flags(xa, flags)
> + xa <- Opaque::ffi_init(|xa: *mut bindings::xarray| unsafe {
> + bindings::__spin_lock_init(
> + &raw mut (*xa).xa_lock,
> + name.as_ptr().cast(),
> + key.as_ptr(),
> + );
> + (*xa).xa_flags = flags;
> + (*xa).xa_head = null_mut();
> }),
> _p: PhantomData,
> })
> @@ -236,8 +262,12 @@ fn load(&self, index: usize) -> Option<NonNull<c_void>> {
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{alloc::{flags::GFP_KERNEL, kbox::KBox}, xarray::{AllocKind, XArray}};
> - /// let xa = KBox::pin_init(XArray::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{
> + /// # alloc::{flags::GFP_KERNEL, kbox::KBox},
> + /// # xarray::{new_xarray, AllocKind, XArray},
> + /// # };
> + /// let xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> ///
> /// let mut guard = xa.lock();
> /// assert_eq!(guard.contains_index(42), false);
> @@ -272,8 +302,9 @@ pub fn get_mut(&mut self, index: usize) -> Option<T::BorrowedMut<'_>> {
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> - /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> + /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> /// let mut guard = xa.lock();
> ///
> /// assert_eq!(guard.contains_index(42), false);
> @@ -309,8 +340,9 @@ fn load_next(&self, index: usize) -> Option<(usize, NonNull<c_void>)> {
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{prelude::*, xarray::{AllocKind, XArray}};
> - /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray}};
> + /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> /// let mut guard = xa.lock();
> ///
> /// guard.store(10, KBox::new(10u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -339,8 +371,9 @@ pub fn find_next(&self, index: usize) -> Option<(usize, T::Borrowed<'_>)> {
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{prelude::*, xarray::{AllocKind, XArray}};
> - /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray}};
> + /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> /// let mut guard = xa.lock();
> ///
> /// guard.store(10, KBox::new(10u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -366,8 +399,9 @@ pub fn find_next_mut(&mut self, index: usize) -> Option<(usize, T::BorrowedMut<'
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{prelude::*, xarray::{AllocKind, XArray}};
> - /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray}};
> + /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> /// let mut guard = xa.lock();
> ///
> /// guard.store(10, KBox::new(10u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -401,8 +435,9 @@ pub fn find_next_entry<'b>(&'b mut self, index: usize) -> Option<OccupiedEntry<'
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{prelude::*, xarray::{AllocKind, XArray}};
> - /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray}};
> + /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> /// let mut guard = xa.lock();
> ///
> /// guard.store(100, KBox::new(42u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -505,8 +540,9 @@ pub fn store(
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{prelude::*, xarray::{AllocKind, XArray}};
> - /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray}};
> + /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> /// let mut guard = xa.lock();
> ///
> /// assert_eq!(guard.get(42), None);
> diff --git a/rust/kernel/xarray/entry.rs b/rust/kernel/xarray/entry.rs
> index 2d6ef4781f47d..6e370252309fc 100644
> --- a/rust/kernel/xarray/entry.rs
> +++ b/rust/kernel/xarray/entry.rs
> @@ -26,8 +26,9 @@ impl<T: ForeignOwnable> Entry<'_, '_, T> {
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> - /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> + /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> /// let mut guard = xa.lock();
> ///
> /// let entry = guard.get_entry(42);
> @@ -100,17 +101,17 @@ fn insert_internal(
> ///
> /// Returns a reference to the newly inserted value.
> ///
> - /// - This method will fail if the nodes on the path to the index
> - /// represented by this entry are not present in the XArray and no memory
> - /// is available via the `preload` argument.
> + /// - This method will fail if the nodes on the path to the index represented by this entry are
> + /// not present in the XArray and no memory is available via the `preload` argument.
> /// - This method will not drop the XArray lock.
> ///
> ///
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> - /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> + /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> /// let mut guard = xa.lock();
> ///
> /// assert_eq!(guard.get(42), None);
> @@ -139,16 +140,16 @@ pub fn insert(
>
> /// Inserts a value and returns an occupied entry representing the newly inserted value.
> ///
> - /// - This method will fail if the nodes on the path to the index
> - /// represented by this entry are not present in the XArray and no memory
> - /// is available via the `preload` argument.
> + /// - This method will fail if the nodes on the path to the index represented by this entry are
> + /// not present in the XArray and no memory is available via the `preload` argument.
> /// - This method will not drop the XArray lock.
> ///
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> - /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> + /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> /// let mut guard = xa.lock();
> ///
> /// assert_eq!(guard.get(42), None);
> @@ -182,8 +183,9 @@ pub fn insert_entry(
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> - /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> + /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> /// let mut guard = xa.lock();
> ///
> /// assert_eq!(guard.get(42), None);
> @@ -221,8 +223,9 @@ pub(crate) fn new(guard: &'b mut Guard<'a, T>, index: usize, ptr: NonNull<c_void
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> - /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> + /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> /// let mut guard = xa.lock();
> ///
> /// guard.store(42, KBox::new(0x1337u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -266,8 +269,9 @@ pub fn remove(mut self) -> T {
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> - /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> + /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> /// let mut guard = xa.lock();
> ///
> /// guard.store(42, KBox::new(0x1337u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -287,8 +291,9 @@ pub fn index(&self) -> usize {
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> - /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> + /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> /// let mut guard = xa.lock();
> ///
> /// guard.store(42, KBox::new(0x1337u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -334,8 +339,9 @@ pub fn insert(&mut self, value: T) -> T {
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> - /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> + /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> /// let mut guard = xa.lock();
> ///
> /// guard.store(42, KBox::new(0x1337u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -361,8 +367,9 @@ pub fn into_mut(self) -> T::BorrowedMut<'b> {
> /// # Examples
> ///
> /// ```
> - /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> - /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> + /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> + /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> + /// KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
> /// let mut guard = xa.lock();
> ///
> /// guard.store(42, KBox::new(100u32, GFP_KERNEL)?, GFP_KERNEL)?;
>
> --
> 2.51.2
>
>
Acked-by: Tamir Duberstein <tamird@...il.com>
Powered by blists - more mailing lists