[PATCH] rust: types: add FOREIGN_ALIGN to ForeignOwnable

[PATCH] rust: types: add FOREIGN_ALIGN to ForeignOwnable

[Andreas Hindborg]

The current implementation of `ForeignOwnable` is leaking the type of the
opaque pointer to consumers of the API. This allows consumers of the opaque
pointer to rely on the information that can be extracted from the pointer
type.

To prevent this, change the API to the version suggested by Maira
Canal (link below): Remove `ForeignOwnable::PointedTo` in favor of a
constant, which specifies the alignment of the pointers returned by
`into_foreign`.

Suggested-by: Alice Ryhl <aliceryhl@google.com>
Suggested-by: Maíra Canal <mcanal@igalia.com>
Link: https://lore.kernel.org/r/20240309235927.168915-3-mcanal@igalia.com
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/alloc/kbox.rs | 40 ++++++++++++++++++++++------------------
 rust/kernel/miscdevice.rs | 10 +++++-----
 rust/kernel/pci.rs        |  2 +-
 rust/kernel/platform.rs   |  2 +-
 rust/kernel/sync/arc.rs   | 23 ++++++++++++-----------
 rust/kernel/types.rs      | 46 +++++++++++++++++++++-------------------------
 rust/kernel/xarray.rs     |  8 ++++----
 7 files changed, 66 insertions(+), 65 deletions(-)

diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
index c386ff771d50..97f45bc4d74f 100644
--- a/rust/kernel/alloc/kbox.rs
+++ b/rust/kernel/alloc/kbox.rs
@@ -398,70 +398,74 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
     }
 }
 
-// SAFETY: The `into_foreign` function returns a pointer that is well-aligned.
+// SAFETY: The pointer returned by `into_foreign` comes from a well aligned
+// pointer to `T`.
 unsafe impl<T: 'static, A> ForeignOwnable for Box<T, A>
 where
     A: Allocator,
 {
-    type PointedTo = T;
+    const FOREIGN_ALIGN: usize = core::mem::align_of::<T>();
     type Borrowed<'a> = &'a T;
     type BorrowedMut<'a> = &'a mut T;
 
-    fn into_foreign(self) -> *mut Self::PointedTo {
-        Box::into_raw(self)
+    fn into_foreign(self) -> *mut crate::ffi::c_void {
+        Box::into_raw(self).cast()
     }
 
-    unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self {
+    unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self {
         // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous
         // call to `Self::into_foreign`.
-        unsafe { Box::from_raw(ptr) }
+        unsafe { Box::from_raw(ptr.cast()) }
     }
 
-    unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> &'a T {
+    unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> &'a T {
         // SAFETY: The safety requirements of this method ensure that the object remains alive and
         // immutable for the duration of 'a.
-        unsafe { &*ptr }
+        unsafe { &*ptr.cast() }
     }
 
-    unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> &'a mut T {
+    unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> &'a mut T {
+        let ptr = ptr.cast();
         // SAFETY: The safety requirements of this method ensure that the pointer is valid and that
         // nothing else will access the value for the duration of 'a.
         unsafe { &mut *ptr }
     }
 }
 
-// SAFETY: The `into_foreign` function returns a pointer that is well-aligned.
+// SAFETY: The pointer returned by `into_foreign` comes from a well aligned
+// pointer to `T`.
 unsafe impl<T: 'static, A> ForeignOwnable for Pin<Box<T, A>>
 where
     A: Allocator,
 {
-    type PointedTo = T;
+    const FOREIGN_ALIGN: usize = core::mem::align_of::<T>();
     type Borrowed<'a> = Pin<&'a T>;
     type BorrowedMut<'a> = Pin<&'a mut T>;
 
-    fn into_foreign(self) -> *mut Self::PointedTo {
+    fn into_foreign(self) -> *mut crate::ffi::c_void {
         // SAFETY: We are still treating the box as pinned.
-        Box::into_raw(unsafe { Pin::into_inner_unchecked(self) })
+        Box::into_raw(unsafe { Pin::into_inner_unchecked(self) }).cast()
     }
 
-    unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self {
+    unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self {
         // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous
         // call to `Self::into_foreign`.
-        unsafe { Pin::new_unchecked(Box::from_raw(ptr)) }
+        unsafe { Pin::new_unchecked(Box::from_raw(ptr.cast())) }
     }
 
-    unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> Pin<&'a T> {
+    unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> Pin<&'a T> {
         // SAFETY: The safety requirements for this function ensure that the object is still alive,
         // so it is safe to dereference the raw pointer.
         // The safety requirements of `from_foreign` also ensure that the object remains alive for
         // the lifetime of the returned value.
-        let r = unsafe { &*ptr };
+        let r = unsafe { &*ptr.cast() };
 
         // SAFETY: This pointer originates from a `Pin<Box<T>>`.
         unsafe { Pin::new_unchecked(r) }
     }
 
-    unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> Pin<&'a mut T> {
+    unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> Pin<&'a mut T> {
+        let ptr = ptr.cast();
         // SAFETY: The safety requirements for this function ensure that the object is still alive,
         // so it is safe to dereference the raw pointer.
         // The safety requirements of `from_foreign` also ensure that the object remains alive for
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index f33c13c3ff97..9b46c3f7ac65 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -217,7 +217,7 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
         // type.
         //
         // SAFETY: The open call of a file can access the private data.
-        unsafe { (*raw_file).private_data = ptr.into_foreign().cast() };
+        unsafe { (*raw_file).private_data = ptr.into_foreign() };
 
         0
     }
@@ -228,7 +228,7 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
     /// must be associated with a `MiscDeviceRegistration<T>`.
     unsafe extern "C" fn release(_inode: *mut bindings::inode, file: *mut bindings::file) -> c_int {
         // SAFETY: The release call of a file owns the private data.
-        let private = unsafe { (*file).private_data }.cast();
+        let private = unsafe { (*file).private_data };
         // SAFETY: The release call of a file owns the private data.
         let ptr = unsafe { <T::Ptr as ForeignOwnable>::from_foreign(private) };
 
@@ -272,7 +272,7 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
     /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
     unsafe extern "C" fn ioctl(file: *mut bindings::file, cmd: c_uint, arg: c_ulong) -> c_long {
         // SAFETY: The ioctl call of a file can access the private data.
-        let private = unsafe { (*file).private_data }.cast();
+        let private = unsafe { (*file).private_data };
         // SAFETY: Ioctl calls can borrow the private data of the file.
         let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
 
@@ -297,7 +297,7 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
         arg: c_ulong,
     ) -> c_long {
         // SAFETY: The compat ioctl call of a file can access the private data.
-        let private = unsafe { (*file).private_data }.cast();
+        let private = unsafe { (*file).private_data };
         // SAFETY: Ioctl calls can borrow the private data of the file.
         let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
 
@@ -318,7 +318,7 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
     /// - `seq_file` must be a valid `struct seq_file` that we can write to.
     unsafe extern "C" fn show_fdinfo(seq_file: *mut bindings::seq_file, file: *mut bindings::file) {
         // SAFETY: The release call of a file owns the private data.
-        let private = unsafe { (*file).private_data }.cast();
+        let private = unsafe { (*file).private_data };
         // SAFETY: Ioctl calls can borrow the private data of the file.
         let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
         // SAFETY:
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 8435f8132e38..0b4b52804250 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -89,7 +89,7 @@ extern "C" fn probe_callback(
     extern "C" fn remove_callback(pdev: *mut bindings::pci_dev) {
         // SAFETY: The PCI bus only ever calls the remove callback with a valid pointer to a
         // `struct pci_dev`.
-        let ptr = unsafe { bindings::pci_get_drvdata(pdev) }.cast();
+        let ptr = unsafe { bindings::pci_get_drvdata(pdev) };
 
         // SAFETY: `remove_callback` is only ever called after a successful call to
         // `probe_callback`, hence it's guaranteed that `ptr` points to a valid and initialized
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 5b21fa517e55..4e37c5ab014d 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -79,7 +79,7 @@ extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> kernel::ff
 
     extern "C" fn remove_callback(pdev: *mut bindings::platform_device) {
         // SAFETY: `pdev` is a valid pointer to a `struct platform_device`.
-        let ptr = unsafe { bindings::platform_get_drvdata(pdev) }.cast();
+        let ptr = unsafe { bindings::platform_get_drvdata(pdev) };
 
         // SAFETY: `remove_callback` is only ever called after a successful call to
         // `probe_callback`, hence it's guaranteed that `ptr` points to a valid and initialized
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index c7af0aa48a0a..6603079b05af 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -140,10 +140,9 @@ pub struct Arc<T: ?Sized> {
     _p: PhantomData<ArcInner<T>>,
 }
 
-#[doc(hidden)]
 #[pin_data]
 #[repr(C)]
-pub struct ArcInner<T: ?Sized> {
+struct ArcInner<T: ?Sized> {
     refcount: Opaque<bindings::refcount_t>,
     data: T,
 }
@@ -372,20 +371,22 @@ pub fn into_unique_or_drop(self) -> Option<Pin<UniqueArc<T>>> {
     }
 }
 
-// SAFETY: The `into_foreign` function returns a pointer that is well-aligned.
+// SAFETY: The pointer returned by `into_foreign` comes from a well aligned
+// pointer to `ArcInner<T>`.
 unsafe impl<T: 'static> ForeignOwnable for Arc<T> {
-    type PointedTo = ArcInner<T>;
+    const FOREIGN_ALIGN: usize = core::mem::align_of::<ArcInner<T>>();
+
     type Borrowed<'a> = ArcBorrow<'a, T>;
     type BorrowedMut<'a> = Self::Borrowed<'a>;
 
-    fn into_foreign(self) -> *mut Self::PointedTo {
-        ManuallyDrop::new(self).ptr.as_ptr()
+    fn into_foreign(self) -> *mut crate::ffi::c_void {
+        ManuallyDrop::new(self).ptr.as_ptr().cast()
     }
 
-    unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self {
+    unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self {
         // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous
         // call to `Self::into_foreign`.
-        let inner = unsafe { NonNull::new_unchecked(ptr) };
+        let inner = unsafe { NonNull::new_unchecked(ptr.cast::<ArcInner<T>>()) };
 
         // SAFETY: By the safety requirement of this function, we know that `ptr` came from
         // a previous call to `Arc::into_foreign`, which guarantees that `ptr` is valid and
@@ -393,17 +394,17 @@ unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self {
         unsafe { Self::from_inner(inner) }
     }
 
-    unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> ArcBorrow<'a, T> {
+    unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> ArcBorrow<'a, T> {
         // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous
         // call to `Self::into_foreign`.
-        let inner = unsafe { NonNull::new_unchecked(ptr) };
+        let inner = unsafe { NonNull::new_unchecked(ptr.cast::<ArcInner<T>>()) };
 
         // SAFETY: The safety requirements of `from_foreign` ensure that the object remains alive
         // for the lifetime of the returned value.
         unsafe { ArcBorrow::new(inner) }
     }
 
-    unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> ArcBorrow<'a, T> {
+    unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> ArcBorrow<'a, T> {
         // SAFETY: The safety requirements for `borrow_mut` are a superset of the safety
         // requirements for `borrow`.
         unsafe { Self::borrow(ptr) }
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 22985b6f6982..025c619a2195 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -21,15 +21,11 @@
 ///
 /// # Safety
 ///
-/// Implementers must ensure that [`into_foreign`] returns a pointer which meets the alignment
-/// requirements of [`PointedTo`].
-///
-/// [`into_foreign`]: Self::into_foreign
-/// [`PointedTo`]: Self::PointedTo
+/// Implementers must ensure that [`Self::into_foreign`] return pointers with alignment that is an
+/// integer multiple of [`Self::FOREIGN_ALIGN`].
 pub unsafe trait ForeignOwnable: Sized {
-    /// Type used when the value is foreign-owned. In practical terms only defines the alignment of
-    /// the pointer.
-    type PointedTo;
+    /// The alignment of pointers returned by `into_foreign`.
+    const FOREIGN_ALIGN: usize;
 
     /// Type used to immutably borrow a value that is currently foreign-owned.
     type Borrowed<'a>;
@@ -39,18 +35,17 @@ pub unsafe trait ForeignOwnable: Sized {
 
     /// Converts a Rust-owned object to a foreign-owned one.
     ///
-    /// # Guarantees
-    ///
-    /// The return value is guaranteed to be well-aligned, but there are no other guarantees for
-    /// this pointer. For example, it might be null, dangling, or point to uninitialized memory.
-    /// Using it in any way except for [`ForeignOwnable::from_foreign`], [`ForeignOwnable::borrow`],
-    /// [`ForeignOwnable::try_from_foreign`] can result in undefined behavior.
+    /// The foreign representation is a pointer to void. The minimum alignment of the returned
+    /// pointer is [`Self::FOREIGN_ALIGN`]. There are no other guarantees for this pointer. For
+    /// example, it might be invalid, dangling or pointing to uninitialized memory. Using it in any
+    /// way except for [`from_foreign`], [`try_from_foreign`], [`borrow`], or [`borrow_mut`] can
+    /// result in undefined behavior.
     ///
     /// [`from_foreign`]: Self::from_foreign
     /// [`try_from_foreign`]: Self::try_from_foreign
     /// [`borrow`]: Self::borrow
     /// [`borrow_mut`]: Self::borrow_mut
-    fn into_foreign(self) -> *mut Self::PointedTo;
+    fn into_foreign(self) -> *mut crate::ffi::c_void;
 
     /// Converts a foreign-owned object back to a Rust-owned one.
     ///
@@ -60,7 +55,7 @@ pub unsafe trait ForeignOwnable: Sized {
     /// must not be passed to `from_foreign` more than once.
     ///
     /// [`into_foreign`]: Self::into_foreign
-    unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self;
+    unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self;
 
     /// Tries to convert a foreign-owned object back to a Rust-owned one.
     ///
@@ -72,7 +67,7 @@ pub unsafe trait ForeignOwnable: Sized {
     /// `ptr` must either be null or satisfy the safety requirements for [`from_foreign`].
     ///
     /// [`from_foreign`]: Self::from_foreign
-    unsafe fn try_from_foreign(ptr: *mut Self::PointedTo) -> Option<Self> {
+    unsafe fn try_from_foreign(ptr: *mut crate::ffi::c_void) -> Option<Self> {
         if ptr.is_null() {
             None
         } else {
@@ -95,7 +90,7 @@ unsafe fn try_from_foreign(ptr: *mut Self::PointedTo) -> Option<Self> {
     ///
     /// [`into_foreign`]: Self::into_foreign
     /// [`from_foreign`]: Self::from_foreign
-    unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> Self::Borrowed<'a>;
+    unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> Self::Borrowed<'a>;
 
     /// Borrows a foreign-owned object mutably.
     ///
@@ -123,23 +118,24 @@ unsafe fn try_from_foreign(ptr: *mut Self::PointedTo) -> Option<Self> {
     /// [`from_foreign`]: Self::from_foreign
     /// [`borrow`]: Self::borrow
     /// [`Arc`]: crate::sync::Arc
-    unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> Self::BorrowedMut<'a>;
+    unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> Self::BorrowedMut<'a>;
 }
 
-// SAFETY: The `into_foreign` function returns a pointer that is dangling, but well-aligned.
+// SAFETY: The pointer returned by `into_foreign` comes from a well aligned
+// pointer to `()`.
 unsafe impl ForeignOwnable for () {
-    type PointedTo = ();
+    const FOREIGN_ALIGN: usize = core::mem::align_of::<()>();
     type Borrowed<'a> = ();
     type BorrowedMut<'a> = ();
 
-    fn into_foreign(self) -> *mut Self::PointedTo {
+    fn into_foreign(self) -> *mut crate::ffi::c_void {
         core::ptr::NonNull::dangling().as_ptr()
     }
 
-    unsafe fn from_foreign(_: *mut Self::PointedTo) -> Self {}
+    unsafe fn from_foreign(_: *mut crate::ffi::c_void) -> Self {}
 
-    unsafe fn borrow<'a>(_: *mut Self::PointedTo) -> Self::Borrowed<'a> {}
-    unsafe fn borrow_mut<'a>(_: *mut Self::PointedTo) -> Self::BorrowedMut<'a> {}
+    unsafe fn borrow<'a>(_: *mut crate::ffi::c_void) -> Self::Borrowed<'a> {}
+    unsafe fn borrow_mut<'a>(_: *mut crate::ffi::c_void) -> Self::BorrowedMut<'a> {}
 }
 
 /// Runs a cleanup function/closure when dropped.
diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs
index 75719e7bb491..35f4357fc03a 100644
--- a/rust/kernel/xarray.rs
+++ b/rust/kernel/xarray.rs
@@ -9,7 +9,7 @@
     error::{Error, Result},
     types::{ForeignOwnable, NotThreadSafe, Opaque},
 };
-use core::{iter, marker::PhantomData, mem, pin::Pin, ptr::NonNull};
+use core::{iter, marker::PhantomData, pin::Pin, ptr::NonNull};
 use pin_init::{pin_data, pin_init, pinned_drop, PinInit};
 
 /// An array which efficiently maps sparse integer indices to owned objects.
@@ -101,7 +101,7 @@ pub fn new(kind: AllocKind) -> impl PinInit<Self> {
         })
     }
 
-    fn iter(&self) -> impl Iterator<Item = NonNull<T::PointedTo>> + '_ {
+    fn iter(&self) -> impl Iterator<Item = NonNull<crate::ffi::c_void>> + '_ {
         let mut index = 0;
 
         // SAFETY: `self.xa` is always valid by the type invariant.
@@ -179,7 +179,7 @@ fn from(value: StoreError<T>) -> Self {
 impl<'a, T: ForeignOwnable> Guard<'a, T> {
     fn load<F, U>(&self, index: usize, f: F) -> Option<U>
     where
-        F: FnOnce(NonNull<T::PointedTo>) -> U,
+        F: FnOnce(NonNull<crate::ffi::c_void>) -> U,
     {
         // SAFETY: `self.xa.xa` is always valid by the type invariant.
         let ptr = unsafe { bindings::xa_load(self.xa.xa.get(), index) };
@@ -230,7 +230,7 @@ pub fn store(
         gfp: alloc::Flags,
     ) -> Result<Option<T>, StoreError<T>> {
         build_assert!(
-            mem::align_of::<T::PointedTo>() >= 4,
+            T::FOREIGN_ALIGN >= 4,
             "pointers stored in XArray must be 4-byte aligned"
         );
         let new = value.into_foreign();

---
base-commit: ec7714e4947909190ffb3041a03311a975350fe0
change-id: 20250605-pointed-to-6170ae01520f

Best regards,
-- 
Andreas Hindborg <a.hindborg@kernel.org>

Re: [PATCH] rust: types: add FOREIGN_ALIGN to ForeignOwnable

[Danilo Krummrich]

On 6/5/25 9:55 PM, Andreas Hindborg wrote:
> The current implementation of `ForeignOwnable` is leaking the type of the
> opaque pointer to consumers of the API. This allows consumers of the opaque
> pointer to rely on the information that can be extracted from the pointer
> type.
> 
> To prevent this, change the API to the version suggested by Maira
> Canal (link below): Remove `ForeignOwnable::PointedTo` in favor of a
> constant, which specifies the alignment of the pointers returned by
> `into_foreign`.
> 
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Suggested-by: Maíra Canal <mcanal@igalia.com>
> Link: https://lore.kernel.org/r/20240309235927.168915-3-mcanal@igalia.com
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

I think having the pointer type be `crate::ffi::c_void` was much more convenient
for users of the API anyways. :)

	Acked-by: Danilo Krummrich <dakr@kernel.org>

Re: [PATCH] rust: types: add FOREIGN_ALIGN to ForeignOwnable

[Benno Lossin]

The title should probably also mention that it removes `PointedTo`. 

On Thu Jun 5, 2025 at 9:55 PM CEST, Andreas Hindborg wrote:
> The current implementation of `ForeignOwnable` is leaking the type of the
> opaque pointer to consumers of the API. This allows consumers of the opaque
> pointer to rely on the information that can be extracted from the pointer
> type.
>
> To prevent this, change the API to the version suggested by Maira
> Canal (link below): Remove `ForeignOwnable::PointedTo` in favor of a
> constant, which specifies the alignment of the pointers returned by
> `into_foreign`.
>
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Suggested-by: Maíra Canal <mcanal@igalia.com>
> Link: https://lore.kernel.org/r/20240309235927.168915-3-mcanal@igalia.com
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

A couple nits and documentation review below, with those fixed:

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

> ---
>  rust/kernel/alloc/kbox.rs | 40 ++++++++++++++++++++++------------------
>  rust/kernel/miscdevice.rs | 10 +++++-----
>  rust/kernel/pci.rs        |  2 +-
>  rust/kernel/platform.rs   |  2 +-
>  rust/kernel/sync/arc.rs   | 23 ++++++++++++-----------
>  rust/kernel/types.rs      | 46 +++++++++++++++++++++-------------------------
>  rust/kernel/xarray.rs     |  8 ++++----
>  7 files changed, 66 insertions(+), 65 deletions(-)
>
> diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
> index c386ff771d50..97f45bc4d74f 100644
> --- a/rust/kernel/alloc/kbox.rs
> +++ b/rust/kernel/alloc/kbox.rs
> @@ -398,70 +398,74 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
>      }
>  }
>  
> -// SAFETY: The `into_foreign` function returns a pointer that is well-aligned.
> +// SAFETY: The pointer returned by `into_foreign` comes from a well aligned
> +// pointer to `T`.
>  unsafe impl<T: 'static, A> ForeignOwnable for Box<T, A>
>  where
>      A: Allocator,
>  {
> -    type PointedTo = T;
> +    const FOREIGN_ALIGN: usize = core::mem::align_of::<T>();
>      type Borrowed<'a> = &'a T;
>      type BorrowedMut<'a> = &'a mut T;
>  
> -    fn into_foreign(self) -> *mut Self::PointedTo {
> -        Box::into_raw(self)
> +    fn into_foreign(self) -> *mut crate::ffi::c_void {

How about we import the prelude, then you can just write `*mut c_void`
everywhere instead of having to write `crate::ffi` all the time.

> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index c7af0aa48a0a..6603079b05af 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -140,10 +140,9 @@ pub struct Arc<T: ?Sized> {
>      _p: PhantomData<ArcInner<T>>,
>  }
>  
> -#[doc(hidden)]
>  #[pin_data]
>  #[repr(C)]
> -pub struct ArcInner<T: ?Sized> {
> +struct ArcInner<T: ?Sized> {

I agree with this change, but let's mention it in the commit message.

>      refcount: Opaque<bindings::refcount_t>,
>      data: T,
>  }

> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 22985b6f6982..025c619a2195 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -21,15 +21,11 @@
>  ///
>  /// # Safety
>  ///
> -/// Implementers must ensure that [`into_foreign`] returns a pointer which meets the alignment
> -/// requirements of [`PointedTo`].
> -///
> -/// [`into_foreign`]: Self::into_foreign
> -/// [`PointedTo`]: Self::PointedTo
> +/// Implementers must ensure that [`Self::into_foreign`] return pointers with alignment that is an

s/return/returns/

> +/// integer multiple of [`Self::FOREIGN_ALIGN`].

I would just write "returns pointers aligned to [`Self::FOREIGN_ALIGN`]".

>  pub unsafe trait ForeignOwnable: Sized {
> -    /// Type used when the value is foreign-owned. In practical terms only defines the alignment of
> -    /// the pointer.
> -    type PointedTo;
> +    /// The alignment of pointers returned by `into_foreign`.
> +    const FOREIGN_ALIGN: usize;
>  
>      /// Type used to immutably borrow a value that is currently foreign-owned.
>      type Borrowed<'a>;
> @@ -39,18 +35,17 @@ pub unsafe trait ForeignOwnable: Sized {
>  
>      /// Converts a Rust-owned object to a foreign-owned one.
>      ///
> -    /// # Guarantees

Why remove this section? I think we should streamline it, (make it use
bullet points, shorten the sentences etc). We can keep the paragraph you
wrote below as normal docs.

> -    ///
> -    /// The return value is guaranteed to be well-aligned, but there are no other guarantees for
> -    /// this pointer. For example, it might be null, dangling, or point to uninitialized memory.
> -    /// Using it in any way except for [`ForeignOwnable::from_foreign`], [`ForeignOwnable::borrow`],
> -    /// [`ForeignOwnable::try_from_foreign`] can result in undefined behavior.
> +    /// The foreign representation is a pointer to void. The minimum alignment of the returned
> +    /// pointer is [`Self::FOREIGN_ALIGN`]. There are no other guarantees for this pointer. For
> +    /// example, it might be invalid, dangling or pointing to uninitialized memory. Using it in any
> +    /// way except for [`from_foreign`], [`try_from_foreign`], [`borrow`], or [`borrow_mut`] can
> +    /// result in undefined behavior.
>      ///
>      /// [`from_foreign`]: Self::from_foreign
>      /// [`try_from_foreign`]: Self::try_from_foreign
>      /// [`borrow`]: Self::borrow
>      /// [`borrow_mut`]: Self::borrow_mut
> -    fn into_foreign(self) -> *mut Self::PointedTo;
> +    fn into_foreign(self) -> *mut crate::ffi::c_void;

Re: [PATCH] rust: types: add FOREIGN_ALIGN to ForeignOwnable

[Andreas Hindborg]

Hi Benno,

"Benno Lossin" <lossin@kernel.org> writes:

> The title should probably also mention that it removes `PointedTo`.
>
> On Thu Jun 5, 2025 at 9:55 PM CEST, Andreas Hindborg wrote:
>> The current implementation of `ForeignOwnable` is leaking the type of the
>> opaque pointer to consumers of the API. This allows consumers of the opaque
>> pointer to rely on the information that can be extracted from the pointer
>> type.
>>
>> To prevent this, change the API to the version suggested by Maira
>> Canal (link below): Remove `ForeignOwnable::PointedTo` in favor of a
>> constant, which specifies the alignment of the pointers returned by
>> `into_foreign`.
>>
>> Suggested-by: Alice Ryhl <aliceryhl@google.com>
>> Suggested-by: Maíra Canal <mcanal@igalia.com>
>> Link: https://lore.kernel.org/r/20240309235927.168915-3-mcanal@igalia.com
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> A couple nits and documentation review below, with those fixed:
>
> Reviewed-by: Benno Lossin <lossin@kernel.org>
>
>> ---
>>  rust/kernel/alloc/kbox.rs | 40 ++++++++++++++++++++++------------------
>>  rust/kernel/miscdevice.rs | 10 +++++-----
>>  rust/kernel/pci.rs        |  2 +-
>>  rust/kernel/platform.rs   |  2 +-
>>  rust/kernel/sync/arc.rs   | 23 ++++++++++++-----------
>>  rust/kernel/types.rs      | 46 +++++++++++++++++++++-------------------------
>>  rust/kernel/xarray.rs     |  8 ++++----
>>  7 files changed, 66 insertions(+), 65 deletions(-)
>>
>> diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
>> index c386ff771d50..97f45bc4d74f 100644
>> --- a/rust/kernel/alloc/kbox.rs
>> +++ b/rust/kernel/alloc/kbox.rs
>> @@ -398,70 +398,74 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
>>      }
>>  }
>>
>> -// SAFETY: The `into_foreign` function returns a pointer that is well-aligned.
>> +// SAFETY: The pointer returned by `into_foreign` comes from a well aligned
>> +// pointer to `T`.
>>  unsafe impl<T: 'static, A> ForeignOwnable for Box<T, A>
>>  where
>>      A: Allocator,
>>  {
>> -    type PointedTo = T;
>> +    const FOREIGN_ALIGN: usize = core::mem::align_of::<T>();
>>      type Borrowed<'a> = &'a T;
>>      type BorrowedMut<'a> = &'a mut T;
>>
>> -    fn into_foreign(self) -> *mut Self::PointedTo {
>> -        Box::into_raw(self)
>> +    fn into_foreign(self) -> *mut crate::ffi::c_void {
>
> How about we import the prelude, then you can just write `*mut c_void`
> everywhere instead of having to write `crate::ffi` all the time.

OK.

>
>> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
>> index c7af0aa48a0a..6603079b05af 100644
>> --- a/rust/kernel/sync/arc.rs
>> +++ b/rust/kernel/sync/arc.rs
>> @@ -140,10 +140,9 @@ pub struct Arc<T: ?Sized> {
>>      _p: PhantomData<ArcInner<T>>,
>>  }
>>
>> -#[doc(hidden)]
>>  #[pin_data]
>>  #[repr(C)]
>> -pub struct ArcInner<T: ?Sized> {
>> +struct ArcInner<T: ?Sized> {
>
> I agree with this change, but let's mention it in the commit message.

Right.

>
>>      refcount: Opaque<bindings::refcount_t>,
>>      data: T,
>>  }
>
>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> index 22985b6f6982..025c619a2195 100644
>> --- a/rust/kernel/types.rs
>> +++ b/rust/kernel/types.rs
>> @@ -21,15 +21,11 @@
>>  ///
>>  /// # Safety
>>  ///
>> -/// Implementers must ensure that [`into_foreign`] returns a pointer which meets the alignment
>> -/// requirements of [`PointedTo`].
>> -///
>> -/// [`into_foreign`]: Self::into_foreign
>> -/// [`PointedTo`]: Self::PointedTo
>> +/// Implementers must ensure that [`Self::into_foreign`] return pointers with alignment that is an
>
> s/return/returns/

Thanks.

>
>> +/// integer multiple of [`Self::FOREIGN_ALIGN`].
>
> I would just write "returns pointers aligned to [`Self::FOREIGN_ALIGN`]".

OK.

>
>>  pub unsafe trait ForeignOwnable: Sized {
>> -    /// Type used when the value is foreign-owned. In practical terms only defines the alignment of
>> -    /// the pointer.
>> -    type PointedTo;
>> +    /// The alignment of pointers returned by `into_foreign`.
>> +    const FOREIGN_ALIGN: usize;
>>
>>      /// Type used to immutably borrow a value that is currently foreign-owned.
>>      type Borrowed<'a>;
>> @@ -39,18 +35,17 @@ pub unsafe trait ForeignOwnable: Sized {
>>
>>      /// Converts a Rust-owned object to a foreign-owned one.
>>      ///
>> -    /// # Guarantees
>
> Why remove this section? I think we should streamline it, (make it use
> bullet points, shorten the sentences etc). We can keep the paragraph you
> wrote below as normal docs.

Not sure exactly what you are going for here. How is this:


  Converts a Rust-owned object to a foreign-owned one.

  The foreign representation is a pointer to void.

  # Guarantees

  - Minimum alignment of returned pointer is [`Self::FOREIGN_ALIGN`].

  There are no other guarantees for this pointer. For example, it might be invalid, dangling
  or pointing to uninitialized memory. Using it in any way except for [`from_foreign`],
  [`try_from_foreign`], [`borrow`], or [`borrow_mut`] can result in undefined behavior.



Best regards,
Andreas Hindborg

Re: [PATCH] rust: types: add FOREIGN_ALIGN to ForeignOwnable

[Benno Lossin]

On Tue Jun 10, 2025 at 11:27 AM CEST, Andreas Hindborg wrote:
> Hi Benno,
>
> "Benno Lossin" <lossin@kernel.org> writes:
>
>> The title should probably also mention that it removes `PointedTo`.

Just making sure that you saw this.

>> On Thu Jun 5, 2025 at 9:55 PM CEST, Andreas Hindborg wrote:
>>>  pub unsafe trait ForeignOwnable: Sized {
>>> -    /// Type used when the value is foreign-owned. In practical terms only defines the alignment of
>>> -    /// the pointer.
>>> -    type PointedTo;
>>> +    /// The alignment of pointers returned by `into_foreign`.
>>> +    const FOREIGN_ALIGN: usize;
>>>
>>>      /// Type used to immutably borrow a value that is currently foreign-owned.
>>>      type Borrowed<'a>;
>>> @@ -39,18 +35,17 @@ pub unsafe trait ForeignOwnable: Sized {
>>>
>>>      /// Converts a Rust-owned object to a foreign-owned one.
>>>      ///
>>> -    /// # Guarantees
>>
>> Why remove this section? I think we should streamline it, (make it use
>> bullet points, shorten the sentences etc). We can keep the paragraph you
>> wrote below as normal docs.
>
> Not sure exactly what you are going for here. How is this:
>
>
>   Converts a Rust-owned object to a foreign-owned one.
>
>   The foreign representation is a pointer to void.
>
>   # Guarantees
>
>   - Minimum alignment of returned pointer is [`Self::FOREIGN_ALIGN`].
>
>   There are no other guarantees for this pointer. For example, it might be invalid, dangling
>   or pointing to uninitialized memory. Using it in any way except for [`from_foreign`],
>   [`try_from_foreign`], [`borrow`], or [`borrow_mut`] can result in undefined behavior.

Maybe even move this paragraph above the `Guarantees` section and change
the beginning of it to "Aside from the guarantees listed below, there
are no other..."?

Otherwise looks good!

---
Cheers,
Benno

Re: [PATCH] rust: types: add FOREIGN_ALIGN to ForeignOwnable

[Andreas Hindborg]

"Benno Lossin" <lossin@kernel.org> writes:

> On Tue Jun 10, 2025 at 11:27 AM CEST, Andreas Hindborg wrote:
>> Hi Benno,
>>
>> "Benno Lossin" <lossin@kernel.org> writes:
>>
>>> The title should probably also mention that it removes `PointedTo`.
>
> Just making sure that you saw this.
>
>>> On Thu Jun 5, 2025 at 9:55 PM CEST, Andreas Hindborg wrote:
>>>>  pub unsafe trait ForeignOwnable: Sized {
>>>> -    /// Type used when the value is foreign-owned. In practical terms only defines the alignment of
>>>> -    /// the pointer.
>>>> -    type PointedTo;
>>>> +    /// The alignment of pointers returned by `into_foreign`.
>>>> +    const FOREIGN_ALIGN: usize;
>>>>
>>>>      /// Type used to immutably borrow a value that is currently foreign-owned.
>>>>      type Borrowed<'a>;
>>>> @@ -39,18 +35,17 @@ pub unsafe trait ForeignOwnable: Sized {
>>>>
>>>>      /// Converts a Rust-owned object to a foreign-owned one.
>>>>      ///
>>>> -    /// # Guarantees
>>>
>>> Why remove this section? I think we should streamline it, (make it use
>>> bullet points, shorten the sentences etc). We can keep the paragraph you
>>> wrote below as normal docs.
>>
>> Not sure exactly what you are going for here. How is this:
>>
>>
>>   Converts a Rust-owned object to a foreign-owned one.
>>
>>   The foreign representation is a pointer to void.
>>
>>   # Guarantees
>>
>>   - Minimum alignment of returned pointer is [`Self::FOREIGN_ALIGN`].
>>
>>   There are no other guarantees for this pointer. For example, it might be invalid, dangling
>>   or pointing to uninitialized memory. Using it in any way except for [`from_foreign`],
>>   [`try_from_foreign`], [`borrow`], or [`borrow_mut`] can result in undefined behavior.
>
> Maybe even move this paragraph above the `Guarantees` section and change
> the beginning of it to "Aside from the guarantees listed below, there
> are no other..."?

Alright.


Best regards,
Andreas Hindborg

Re: [PATCH] rust: types: add FOREIGN_ALIGN to ForeignOwnable

[Alice Ryhl]

On Thu, Jun 5, 2025 at 10:00 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> The current implementation of `ForeignOwnable` is leaking the type of the
> opaque pointer to consumers of the API. This allows consumers of the opaque
> pointer to rely on the information that can be extracted from the pointer
> type.
>
> To prevent this, change the API to the version suggested by Maira
> Canal (link below): Remove `ForeignOwnable::PointedTo` in favor of a
> constant, which specifies the alignment of the pointers returned by
> `into_foreign`.
>
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Suggested-by: Maíra Canal <mcanal@igalia.com>
> Link: https://lore.kernel.org/r/20240309235927.168915-3-mcanal@igalia.com
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

One nit below. With that and things other folks mentioned fixed, you may add:

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 22985b6f6982..025c619a2195 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -21,15 +21,11 @@
>  ///
>  /// # Safety
>  ///
> -/// Implementers must ensure that [`into_foreign`] returns a pointer which meets the alignment
> -/// requirements of [`PointedTo`].
> -///
> -/// [`into_foreign`]: Self::into_foreign
> -/// [`PointedTo`]: Self::PointedTo
> +/// Implementers must ensure that [`Self::into_foreign`] return pointers with alignment that is an
> +/// integer multiple of [`Self::FOREIGN_ALIGN`].

We should require non-null:

Implementers must ensure that [`Self::into_foreign`] returns pointers
that are non-null and with alignment that is an integer multiple of
[`Self::FOREIGN_ALIGN`].

Alice

Re: [PATCH] rust: types: add FOREIGN_ALIGN to ForeignOwnable

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Thu, Jun 5, 2025 at 10:00 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> The current implementation of `ForeignOwnable` is leaking the type of the
>> opaque pointer to consumers of the API. This allows consumers of the opaque
>> pointer to rely on the information that can be extracted from the pointer
>> type.
>>
>> To prevent this, change the API to the version suggested by Maira
>> Canal (link below): Remove `ForeignOwnable::PointedTo` in favor of a
>> constant, which specifies the alignment of the pointers returned by
>> `into_foreign`.
>>
>> Suggested-by: Alice Ryhl <aliceryhl@google.com>
>> Suggested-by: Maíra Canal <mcanal@igalia.com>
>> Link: https://lore.kernel.org/r/20240309235927.168915-3-mcanal@igalia.com
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> One nit below. With that and things other folks mentioned fixed, you may add:
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>
>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> index 22985b6f6982..025c619a2195 100644
>> --- a/rust/kernel/types.rs
>> +++ b/rust/kernel/types.rs
>> @@ -21,15 +21,11 @@
>>  ///
>>  /// # Safety
>>  ///
>> -/// Implementers must ensure that [`into_foreign`] returns a pointer which meets the alignment
>> -/// requirements of [`PointedTo`].
>> -///
>> -/// [`into_foreign`]: Self::into_foreign
>> -/// [`PointedTo`]: Self::PointedTo
>> +/// Implementers must ensure that [`Self::into_foreign`] return pointers with alignment that is an
>> +/// integer multiple of [`Self::FOREIGN_ALIGN`].
>
> We should require non-null:

What is your rationale for this?


Best regards,
Andreas Hindborg

Re: [PATCH] rust: types: add FOREIGN_ALIGN to ForeignOwnable

[Alice Ryhl]

On Wed, Jun 11, 2025 at 12:46 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> "Alice Ryhl" <aliceryhl@google.com> writes:
>
> > On Thu, Jun 5, 2025 at 10:00 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
> >>
> >> The current implementation of `ForeignOwnable` is leaking the type of the
> >> opaque pointer to consumers of the API. This allows consumers of the opaque
> >> pointer to rely on the information that can be extracted from the pointer
> >> type.
> >>
> >> To prevent this, change the API to the version suggested by Maira
> >> Canal (link below): Remove `ForeignOwnable::PointedTo` in favor of a
> >> constant, which specifies the alignment of the pointers returned by
> >> `into_foreign`.
> >>
> >> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> >> Suggested-by: Maíra Canal <mcanal@igalia.com>
> >> Link: https://lore.kernel.org/r/20240309235927.168915-3-mcanal@igalia.com
> >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> >
> > One nit below. With that and things other folks mentioned fixed, you may add:
> >
> > Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> >
> >> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> >> index 22985b6f6982..025c619a2195 100644
> >> --- a/rust/kernel/types.rs
> >> +++ b/rust/kernel/types.rs
> >> @@ -21,15 +21,11 @@
> >>  ///
> >>  /// # Safety
> >>  ///
> >> -/// Implementers must ensure that [`into_foreign`] returns a pointer which meets the alignment
> >> -/// requirements of [`PointedTo`].
> >> -///
> >> -/// [`into_foreign`]: Self::into_foreign
> >> -/// [`PointedTo`]: Self::PointedTo
> >> +/// Implementers must ensure that [`Self::into_foreign`] return pointers with alignment that is an
> >> +/// integer multiple of [`Self::FOREIGN_ALIGN`].
> >
> > We should require non-null:
>
> What is your rationale for this?

The rationale is that the implementation of XArray assumes that the
pointers are non-null. If we allow null pointers, we will need to fix
the XArray.

Alice

Re: [PATCH] rust: types: add FOREIGN_ALIGN to ForeignOwnable

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Wed, Jun 11, 2025 at 12:46 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> "Alice Ryhl" <aliceryhl@google.com> writes:
>>
>> > On Thu, Jun 5, 2025 at 10:00 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>> >>
>> >> The current implementation of `ForeignOwnable` is leaking the type of the
>> >> opaque pointer to consumers of the API. This allows consumers of the opaque
>> >> pointer to rely on the information that can be extracted from the pointer
>> >> type.
>> >>
>> >> To prevent this, change the API to the version suggested by Maira
>> >> Canal (link below): Remove `ForeignOwnable::PointedTo` in favor of a
>> >> constant, which specifies the alignment of the pointers returned by
>> >> `into_foreign`.
>> >>
>> >> Suggested-by: Alice Ryhl <aliceryhl@google.com>
>> >> Suggested-by: Maíra Canal <mcanal@igalia.com>
>> >> Link: https://lore.kernel.org/r/20240309235927.168915-3-mcanal@igaliacom
>> >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> >
>> > One nit below. With that and things other folks mentioned fixed, you may add:
>> >
>> > Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>> >
>> >> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> >> index 22985b6f6982..025c619a2195 100644
>> >> --- a/rust/kernel/types.rs
>> >> +++ b/rust/kernel/types.rs
>> >> @@ -21,15 +21,11 @@
>> >>  ///
>> >>  /// # Safety
>> >>  ///
>> >> -/// Implementers must ensure that [`into_foreign`] returns a pointer which meets the alignment
>> >> -/// requirements of [`PointedTo`].
>> >> -///
>> >> -/// [`into_foreign`]: Self::into_foreign
>> >> -/// [`PointedTo`]: Self::PointedTo
>> >> +/// Implementers must ensure that [`Self::into_foreign`] return pointers with alignment that is an
>> >> +/// integer multiple of [`Self::FOREIGN_ALIGN`].
>> >
>> > We should require non-null:
>>
>> What is your rationale for this?
>
> The rationale is that the implementation of XArray assumes that the
> pointers are non-null. If we allow null pointers, we will need to fix
> the XArray.

OK, thanks. Also, `try_from_foreign` as Benno highlighted.


Best regards,
Andreas Hindborg