[PATCH 0/2] Take ARCH_KMALLOC_MINALIGN into account for build-time XArray check

[PATCH 0/2] Take ARCH_KMALLOC_MINALIGN into account for build-time XArray check

[Alice Ryhl]

The Rust bindings for XArray include a build-time check to ensure that
you can only use the XArray with pointers that are 4-byte aligned.
Because of that, there is currently a build failure if you attempt to
create an XArray<KBox<T>> where T is a 1-byte or 2-byte aligned type.
However, this error is incorrect as KBox<_> is guaranteed to be a
pointer that comes from kmalloc, and kmalloc always produces pointers
that are at least 4-byte aligned.

To fix this, we augment the compile-time logic that computes the
alignment of KBox<_> to take the minimum alignment of its allocator into
account.

This series is based on top of rust-next.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Alice Ryhl (2):
      rust: alloc: specify the minimum alignment of each allocator
      rust: alloc: take the allocator into account for FOREIGN_ALIGN

 rust/kernel/alloc.rs           |  8 ++++++++
 rust/kernel/alloc/allocator.rs |  8 ++++++++
 rust/kernel/alloc/kbox.rs      | 15 +++++++++++----
 rust/kernel/sync/arc.rs        |  6 +++---
 4 files changed, 30 insertions(+), 7 deletions(-)
---
base-commit: a68a6bef0e75fb9e5aea1399d8538f4e3584dab1
change-id: 20250715-align-min-allocator-b31aee53cbda

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

[PATCH 1/2] rust: alloc: specify the minimum alignment of each allocator

[Alice Ryhl]

The kernel's allocators sometimes provide a higher alignment than the
end-user requested, so add a new constant on the Allocator trait to let
the allocator specify what its minimum guaranteed alignment is.

This allows the ForeignOwnable trait to provide a more accurate value of
FOREIGN_ALIGN when using a pointer type such as Box, which will be
useful with certain collections such as XArray that store its own data
in the low bits of pointers.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/alloc.rs           | 8 ++++++++
 rust/kernel/alloc/allocator.rs | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
index a2c49e5494d334bfde67328464dafcdb31052947..c12753a5fb1c7423a4063553674b537a775c860e 100644
--- a/rust/kernel/alloc.rs
+++ b/rust/kernel/alloc.rs
@@ -137,6 +137,14 @@ pub mod flags {
 /// - Implementers must ensure that all trait functions abide by the guarantees documented in the
 ///   `# Guarantees` sections.
 pub unsafe trait Allocator {
+    /// The minimum alignment satisfied by all allocations from this allocator.
+    ///
+    /// # Guarantees
+    ///
+    /// Any pointer allocated by this allocator must be aligned to `MIN_ALIGN` even if the
+    /// requested layout has a smaller alignment.
+    const MIN_ALIGN: usize;
+
     /// Allocate memory based on `layout` and `flags`.
     ///
     /// On success, returns a buffer represented as `NonNull<[u8]>` that satisfies the layout
diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
index aa2dfa9dca4c309e5a9eafc7da6a8a9bd7b54b11..25fc9f9ae3b4e471a08d77130b374bd1397f7384 100644
--- a/rust/kernel/alloc/allocator.rs
+++ b/rust/kernel/alloc/allocator.rs
@@ -17,6 +17,8 @@
 use crate::bindings;
 use crate::pr_warn;
 
+const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN as usize;
+
 /// The contiguous kernel allocator.
 ///
 /// `Kmalloc` is typically used for physically contiguous allocations up to page size, but also
@@ -128,6 +130,8 @@ unsafe fn call(
 // - passing a pointer to a valid memory allocation is OK,
 // - `realloc` satisfies the guarantees, since `ReallocFunc::call` has the same.
 unsafe impl Allocator for Kmalloc {
+    const MIN_ALIGN: usize = ARCH_KMALLOC_MINALIGN;
+
     #[inline]
     unsafe fn realloc(
         ptr: Option<NonNull<u8>>,
@@ -145,6 +149,8 @@ unsafe fn realloc(
 // - passing a pointer to a valid memory allocation is OK,
 // - `realloc` satisfies the guarantees, since `ReallocFunc::call` has the same.
 unsafe impl Allocator for Vmalloc {
+    const MIN_ALIGN: usize = kernel::page::PAGE_SIZE;
+
     #[inline]
     unsafe fn realloc(
         ptr: Option<NonNull<u8>>,
@@ -169,6 +175,8 @@ unsafe fn realloc(
 // - passing a pointer to a valid memory allocation is OK,
 // - `realloc` satisfies the guarantees, since `ReallocFunc::call` has the same.
 unsafe impl Allocator for KVmalloc {
+    const MIN_ALIGN: usize = ARCH_KMALLOC_MINALIGN;
+
     #[inline]
     unsafe fn realloc(
         ptr: Option<NonNull<u8>>,

-- 
2.50.0.727.gbf7dc18ff4-goog

Re: [PATCH 1/2] rust: alloc: specify the minimum alignment of each allocator

[Danilo Krummrich]

On Tue Jul 15, 2025 at 3:46 PM CEST, Alice Ryhl wrote:
> diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
> index a2c49e5494d334bfde67328464dafcdb31052947..c12753a5fb1c7423a4063553674b537a775c860e 100644
> --- a/rust/kernel/alloc.rs
> +++ b/rust/kernel/alloc.rs
> @@ -137,6 +137,14 @@ pub mod flags {
>  /// - Implementers must ensure that all trait functions abide by the guarantees documented in the
>  ///   `# Guarantees` sections.
>  pub unsafe trait Allocator {
> +    /// The minimum alignment satisfied by all allocations from this allocator.
> +    ///
> +    /// # Guarantees
> +    ///
> +    /// Any pointer allocated by this allocator must be aligned to `MIN_ALIGN` even if the
> +    /// requested layout has a smaller alignment.

I'd say "is guaranteed to be aligned to" instead, "must be" reads like a
requirement.

Speaking of which, I think this also needs to be expressed as a safety
requirement of the Allocator trait itself, which the specific allocator
implementations need to justify.

> +    const MIN_ALIGN: usize;
> +
>      /// Allocate memory based on `layout` and `flags`.
>      ///
>      /// On success, returns a buffer represented as `NonNull<[u8]>` that satisfies the layout
> diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
> index aa2dfa9dca4c309e5a9eafc7da6a8a9bd7b54b11..25fc9f9ae3b4e471a08d77130b374bd1397f7384 100644
> --- a/rust/kernel/alloc/allocator.rs
> +++ b/rust/kernel/alloc/allocator.rs
> @@ -17,6 +17,8 @@
>  use crate::bindings;
>  use crate::pr_warn;
>  
> +const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN as usize;
> +
>  /// The contiguous kernel allocator.
>  ///
>  /// `Kmalloc` is typically used for physically contiguous allocations up to page size, but also
> @@ -128,6 +130,8 @@ unsafe fn call(
>  // - passing a pointer to a valid memory allocation is OK,
>  // - `realloc` satisfies the guarantees, since `ReallocFunc::call` has the same.
>  unsafe impl Allocator for Kmalloc {
> +    const MIN_ALIGN: usize = ARCH_KMALLOC_MINALIGN;
> +
>      #[inline]
>      unsafe fn realloc(
>          ptr: Option<NonNull<u8>>,
> @@ -145,6 +149,8 @@ unsafe fn realloc(
>  // - passing a pointer to a valid memory allocation is OK,
>  // - `realloc` satisfies the guarantees, since `ReallocFunc::call` has the same.
>  unsafe impl Allocator for Vmalloc {
> +    const MIN_ALIGN: usize = kernel::page::PAGE_SIZE;
> +
>      #[inline]
>      unsafe fn realloc(
>          ptr: Option<NonNull<u8>>,
> @@ -169,6 +175,8 @@ unsafe fn realloc(
>  // - passing a pointer to a valid memory allocation is OK,
>  // - `realloc` satisfies the guarantees, since `ReallocFunc::call` has the same.
>  unsafe impl Allocator for KVmalloc {
> +    const MIN_ALIGN: usize = ARCH_KMALLOC_MINALIGN;
> +
>      #[inline]
>      unsafe fn realloc(
>          ptr: Option<NonNull<u8>>,

Re: [PATCH 1/2] rust: alloc: specify the minimum alignment of each allocator

[Alice Ryhl]

On Tue, Jul 15, 2025 at 4:05 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Tue Jul 15, 2025 at 3:46 PM CEST, Alice Ryhl wrote:
> > diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
> > index a2c49e5494d334bfde67328464dafcdb31052947..c12753a5fb1c7423a4063553674b537a775c860e 100644
> > --- a/rust/kernel/alloc.rs
> > +++ b/rust/kernel/alloc.rs
> > @@ -137,6 +137,14 @@ pub mod flags {
> >  /// - Implementers must ensure that all trait functions abide by the guarantees documented in the
> >  ///   `# Guarantees` sections.
> >  pub unsafe trait Allocator {
> > +    /// The minimum alignment satisfied by all allocations from this allocator.
> > +    ///
> > +    /// # Guarantees
> > +    ///
> > +    /// Any pointer allocated by this allocator must be aligned to `MIN_ALIGN` even if the
> > +    /// requested layout has a smaller alignment.
>
> I'd say "is guaranteed to be aligned to" instead, "must be" reads like a
> requirement.

Yes I agree that sounds better.

> Speaking of which, I think this also needs to be expressed as a safety
> requirement of the Allocator trait itself, which the specific allocator
> implementations need to justify.

The trait safety requirements already says that the implementation
must provide the guarantee listed on each item in the trait.

Alice

Re: [PATCH 1/2] rust: alloc: specify the minimum alignment of each allocator

[Danilo Krummrich]

On Tue Jul 15, 2025 at 4:35 PM CEST, Alice Ryhl wrote:
> On Tue, Jul 15, 2025 at 4:05 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Tue Jul 15, 2025 at 3:46 PM CEST, Alice Ryhl wrote:
>> > diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
>> > index a2c49e5494d334bfde67328464dafcdb31052947..c12753a5fb1c7423a4063553674b537a775c860e 100644
>> > --- a/rust/kernel/alloc.rs
>> > +++ b/rust/kernel/alloc.rs
>> > @@ -137,6 +137,14 @@ pub mod flags {
>> >  /// - Implementers must ensure that all trait functions abide by the guarantees documented in the
>> >  ///   `# Guarantees` sections.
>> >  pub unsafe trait Allocator {
>> > +    /// The minimum alignment satisfied by all allocations from this allocator.
>> > +    ///
>> > +    /// # Guarantees
>> > +    ///
>> > +    /// Any pointer allocated by this allocator must be aligned to `MIN_ALIGN` even if the
>> > +    /// requested layout has a smaller alignment.
>>
>> I'd say "is guaranteed to be aligned to" instead, "must be" reads like a
>> requirement.
>
> Yes I agree that sounds better.
>
>> Speaking of which, I think this also needs to be expressed as a safety
>> requirement of the Allocator trait itself, which the specific allocator
>> implementations need to justify.
>
> The trait safety requirements already says that the implementation
> must provide the guarantee listed on each item in the trait.

Oh, indeed, that's fine then. :)

Re: [PATCH 1/2] rust: alloc: specify the minimum alignment of each allocator

[Benno Lossin]

On Tue Jul 15, 2025 at 3:46 PM CEST, Alice Ryhl wrote:
> The kernel's allocators sometimes provide a higher alignment than the
> end-user requested, so add a new constant on the Allocator trait to let
> the allocator specify what its minimum guaranteed alignment is.
>
> This allows the ForeignOwnable trait to provide a more accurate value of
> FOREIGN_ALIGN when using a pointer type such as Box, which will be
> useful with certain collections such as XArray that store its own data
> in the low bits of pointers.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

With the wording changed according to Danilo's suggestion:

Reviewed-by: Benno Lossin <lossin@kernel.org>

---
Cheers,
Benno

> ---
>  rust/kernel/alloc.rs           | 8 ++++++++
>  rust/kernel/alloc/allocator.rs | 8 ++++++++
>  2 files changed, 16 insertions(+)

[PATCH 2/2] rust: alloc: take the allocator into account for FOREIGN_ALIGN

[Alice Ryhl]

When converting a Box<T> into a void pointer, the allocator might
guarantee a higher alignment than the type itself does, and in that case
it is guaranteed that the void pointer has that higher alignment.

This is quite useful when combined with the XArray, which you can only
create using a ForeignOwnable whose FOREIGN_ALIGN is at least 4. This
means that you can now always use a Box<T> with the XArray no matter the
alignment of T.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/alloc/kbox.rs | 15 +++++++++++----
 rust/kernel/sync/arc.rs   |  6 +++---
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
index bffe72f44cb33a265018e67d92d9f0abe82f8e22..fd3f1e0b9c3b3437fb50d8f1b28c92bc7cefd565 100644
--- a/rust/kernel/alloc/kbox.rs
+++ b/rust/kernel/alloc/kbox.rs
@@ -400,12 +400,19 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
 }
 
 // SAFETY: The pointer returned by `into_foreign` comes from a well aligned
-// pointer to `T`.
+// pointer to `T` allocated by `A`.
 unsafe impl<T: 'static, A> ForeignOwnable for Box<T, A>
 where
     A: Allocator,
 {
-    const FOREIGN_ALIGN: usize = core::mem::align_of::<T>();
+    const FOREIGN_ALIGN: usize = {
+        let mut align = core::mem::align_of::<T>();
+        if align < A::MIN_ALIGN {
+            align = A::MIN_ALIGN;
+        }
+        align
+    };
+
     type Borrowed<'a> = &'a T;
     type BorrowedMut<'a> = &'a mut T;
 
@@ -434,12 +441,12 @@ unsafe fn borrow_mut<'a>(ptr: *mut c_void) -> &'a mut T {
 }
 
 // SAFETY: The pointer returned by `into_foreign` comes from a well aligned
-// pointer to `T`.
+// pointer to `T` allocated by `A`.
 unsafe impl<T: 'static, A> ForeignOwnable for Pin<Box<T, A>>
 where
     A: Allocator,
 {
-    const FOREIGN_ALIGN: usize = core::mem::align_of::<T>();
+    const FOREIGN_ALIGN: usize = <Box<T, A> as ForeignOwnable>::FOREIGN_ALIGN;
     type Borrowed<'a> = Pin<&'a T>;
     type BorrowedMut<'a> = Pin<&'a mut T>;
 
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 63a66761d0c7d752e09ce7372bc230661b2f7c6d..74121cf935f364c16799b5c31cc88714dfd6b702 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -373,10 +373,10 @@ pub fn into_unique_or_drop(self) -> Option<Pin<UniqueArc<T>>> {
     }
 }
 
-// SAFETY: The pointer returned by `into_foreign` comes from a well aligned
-// pointer to `ArcInner<T>`.
+// SAFETY: The pointer returned by `into_foreign` was originally allocated as an
+// `KBox<ArcInner<T>>`, so that type is what determines the alignment.
 unsafe impl<T: 'static> ForeignOwnable for Arc<T> {
-    const FOREIGN_ALIGN: usize = core::mem::align_of::<ArcInner<T>>();
+    const FOREIGN_ALIGN: usize = <KBox<ArcInner<T>> as ForeignOwnable>::FOREIGN_ALIGN;
 
     type Borrowed<'a> = ArcBorrow<'a, T>;
     type BorrowedMut<'a> = Self::Borrowed<'a>;

-- 
2.50.0.727.gbf7dc18ff4-goog

Re: [PATCH 2/2] rust: alloc: take the allocator into account for FOREIGN_ALIGN

[Danilo Krummrich]

On Tue Jul 15, 2025 at 3:46 PM CEST, Alice Ryhl wrote:
> diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
> index bffe72f44cb33a265018e67d92d9f0abe82f8e22..fd3f1e0b9c3b3437fb50d8f1b28c92bc7cefd565 100644
> --- a/rust/kernel/alloc/kbox.rs
> +++ b/rust/kernel/alloc/kbox.rs
> @@ -400,12 +400,19 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
>  }
>  
>  // SAFETY: The pointer returned by `into_foreign` comes from a well aligned
> -// pointer to `T`.
> +// pointer to `T` allocated by `A`.
>  unsafe impl<T: 'static, A> ForeignOwnable for Box<T, A>
>  where
>      A: Allocator,
>  {
> -    const FOREIGN_ALIGN: usize = core::mem::align_of::<T>();
> +    const FOREIGN_ALIGN: usize = {
> +        let mut align = core::mem::align_of::<T>();
> +        if align < A::MIN_ALIGN {
> +            align = A::MIN_ALIGN;
> +        }
> +        align
> +    };

Pretty unfortunate that core::cmp::max() can't be used from const context. :(

What do you think about

	const FOREIGN_ALIGN: usize =
	    if core::mem::align_of::<T>() < A::MIN_ALIGN {
	        A::MIN_ALIGN
	    } else {
	        core::mem::align_of::<T>()
	    };

instead? I think that reads a bit better.

Re: [PATCH 2/2] rust: alloc: take the allocator into account for FOREIGN_ALIGN

[Alice Ryhl]

On Tue, Jul 15, 2025 at 4:19 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Tue Jul 15, 2025 at 3:46 PM CEST, Alice Ryhl wrote:
> > diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
> > index bffe72f44cb33a265018e67d92d9f0abe82f8e22..fd3f1e0b9c3b3437fb50d8f1b28c92bc7cefd565 100644
> > --- a/rust/kernel/alloc/kbox.rs
> > +++ b/rust/kernel/alloc/kbox.rs
> > @@ -400,12 +400,19 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
> >  }
> >
> >  // SAFETY: The pointer returned by `into_foreign` comes from a well aligned
> > -// pointer to `T`.
> > +// pointer to `T` allocated by `A`.
> >  unsafe impl<T: 'static, A> ForeignOwnable for Box<T, A>
> >  where
> >      A: Allocator,
> >  {
> > -    const FOREIGN_ALIGN: usize = core::mem::align_of::<T>();
> > +    const FOREIGN_ALIGN: usize = {
> > +        let mut align = core::mem::align_of::<T>();
> > +        if align < A::MIN_ALIGN {
> > +            align = A::MIN_ALIGN;
> > +        }
> > +        align
> > +    };
>
> Pretty unfortunate that core::cmp::max() can't be used from const context. :(
>
> What do you think about
>
>         const FOREIGN_ALIGN: usize =
>             if core::mem::align_of::<T>() < A::MIN_ALIGN {
>                 A::MIN_ALIGN
>             } else {
>                 core::mem::align_of::<T>()
>             };
>
> instead? I think that reads a bit better.

I don't mind doing that instead.

Alice

Re: [PATCH 2/2] rust: alloc: take the allocator into account for FOREIGN_ALIGN

[Benno Lossin]

On Tue Jul 15, 2025 at 3:46 PM CEST, Alice Ryhl wrote:
> When converting a Box<T> into a void pointer, the allocator might
> guarantee a higher alignment than the type itself does, and in that case
> it is guaranteed that the void pointer has that higher alignment.
>
> This is quite useful when combined with the XArray, which you can only
> create using a ForeignOwnable whose FOREIGN_ALIGN is at least 4. This
> means that you can now always use a Box<T> with the XArray no matter the
> alignment of T.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Hey this is cool!

Reviewed-by: Benno Lossin <lossin@kernel.org>

One question below.

> ---
>  rust/kernel/alloc/kbox.rs | 15 +++++++++++----
>  rust/kernel/sync/arc.rs   |  6 +++---
>  2 files changed, 14 insertions(+), 7 deletions(-)

> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 63a66761d0c7d752e09ce7372bc230661b2f7c6d..74121cf935f364c16799b5c31cc88714dfd6b702 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -373,10 +373,10 @@ pub fn into_unique_or_drop(self) -> Option<Pin<UniqueArc<T>>> {
>      }
>  }
>  
> -// SAFETY: The pointer returned by `into_foreign` comes from a well aligned
> -// pointer to `ArcInner<T>`.
> +// SAFETY: The pointer returned by `into_foreign` was originally allocated as an
> +// `KBox<ArcInner<T>>`, so that type is what determines the alignment.
>  unsafe impl<T: 'static> ForeignOwnable for Arc<T> {
> -    const FOREIGN_ALIGN: usize = core::mem::align_of::<ArcInner<T>>();
> +    const FOREIGN_ALIGN: usize = <KBox<ArcInner<T>> as ForeignOwnable>::FOREIGN_ALIGN;

Do we at some point also want to give people the option to use vmalloc
for `Arc`?

---
Cheers,
Benno

>  
>      type Borrowed<'a> = ArcBorrow<'a, T>;
>      type BorrowedMut<'a> = Self::Borrowed<'a>;

Re: [PATCH 2/2] rust: alloc: take the allocator into account for FOREIGN_ALIGN

[Danilo Krummrich]

On Tue Jul 15, 2025 at 6:00 PM CEST, Benno Lossin wrote:
> Do we at some point also want to give people the option to use vmalloc
> for `Arc`?

I'm not sure how useful that is. Typically, larger allocations (e.g. buffers)
are members of smaller reference counted objects that manage their lifetime,
etc.