Allows access to a value in an `Arc` that is currently held as a raw
pointer due to use of `Arc::into_raw`, without destroying or otherwise
consuming that raw pointer.
This is a dependency of the linked list that Rust Binder uses. The
linked list uses this method when iterating over the linked list.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/sync/arc.rs | 76 +++++++++++++++++++++++++++++++++++++------------
1 file changed, 58 insertions(+), 18 deletions(-)
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 7d4c4bf58388..53addb8876c2 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -137,6 +137,39 @@ struct ArcInner<T: ?Sized> {
data: T,
}
+impl<T: ?Sized> ArcInner<T> {
+ /// Converts a pointer to the contents of an [`Arc`] into a pointer to the [`ArcInner`].
+ ///
+ /// # Safety
+ ///
+ /// `ptr` must have been returned by a previous call to [`Arc::into_raw`], and the `Arc` must
+ /// not yet have been destroyed.
+ unsafe fn container_of(ptr: *const T) -> NonNull<ArcInner<T>> {
+ let refcount_layout = Layout::new::<bindings::refcount_t>();
+ // SAFETY: The caller guarantees that the pointer is valid.
+ let val_layout = Layout::for_value(unsafe { &*ptr });
+ // SAFETY: We're computing the layout of a real struct that existed when compiling this
+ // binary, so its layout is not so large that it can trigger arithmetic overflow.
+ let val_offset = unsafe { refcount_layout.extend(val_layout).unwrap_unchecked().1 };
+
+ // Pointer casts leave the metadata unchanged. This is okay because the metadata of `T` and
+ // `ArcInner<T>` is the same since `ArcInner` is a struct with `T` as its last field.
+ //
+ // This is documented at:
+ // <https://doc.rust-lang.org/std/ptr/trait.Pointee.html>.
+ let ptr = ptr as *const ArcInner<T>;
+
+ // SAFETY: The pointer is in-bounds of an allocation both before and after offsetting the
+ // pointer, since it originates from a previous call to `Arc::into_raw` on an `Arc` that is
+ // still valid.
+ let ptr = unsafe { ptr.byte_sub(val_offset) };
+
+ // SAFETY: The pointer can't be null since you can't have an `ArcInner<T>` value at the null
+ // address.
+ unsafe { NonNull::new_unchecked(ptr.cast_mut()) }
+ }
+}
+
// This is to allow [`Arc`] (and variants) to be used as the type of `self`.
impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
@@ -232,27 +265,13 @@ pub fn into_raw(self) -> *const T {
/// `ptr` must have been returned by a previous call to [`Arc::into_raw`]. Additionally, it
/// must not be called more than once for each previous call to [`Arc::into_raw`].
pub unsafe fn from_raw(ptr: *const T) -> Self {
- let refcount_layout = Layout::new::<bindings::refcount_t>();
- // SAFETY: The caller guarantees that the pointer is valid.
- let val_layout = Layout::for_value(unsafe { &*ptr });
- // SAFETY: We're computing the layout of a real struct that existed when compiling this
- // binary, so its layout is not so large that it can trigger arithmetic overflow.
- let val_offset = unsafe { refcount_layout.extend(val_layout).unwrap_unchecked().1 };
-
- // Pointer casts leave the metadata unchanged. This is okay because the metadata of `T` and
- // `ArcInner<T>` is the same since `ArcInner` is a struct with `T` as its last field.
- //
- // This is documented at:
- // <https://doc.rust-lang.org/std/ptr/trait.Pointee.html>.
- let ptr = ptr as *const ArcInner<T>;
-
- // SAFETY: The pointer is in-bounds of an allocation both before and after offsetting the
- // pointer, since it originates from a previous call to `Arc::into_raw` and is still valid.
- let ptr = unsafe { ptr.byte_sub(val_offset) };
+ // SAFETY: The caller promises that this pointer originates from a call to `into_raw` on an
+ // `Arc` that is still valid.
+ let ptr = unsafe { ArcInner::container_of(ptr) };
// SAFETY: By the safety requirements we know that `ptr` came from `Arc::into_raw`, so the
// reference count held then will be owned by the new `Arc` object.
- unsafe { Self::from_inner(NonNull::new_unchecked(ptr.cast_mut())) }
+ unsafe { Self::from_inner(ptr) }
}
/// Returns an [`ArcBorrow`] from the given [`Arc`].
@@ -453,6 +472,27 @@ unsafe fn new(inner: NonNull<ArcInner<T>>) -> Self {
_p: PhantomData,
}
}
+
+ /// Creates an [`ArcBorrow`] to an [`Arc`] that has previously been deconstructed with
+ /// [`Arc::into_raw`].
+ ///
+ /// # Safety
+ ///
+ /// * The provided pointer must originate from a call to [`Arc::into_raw`].
+ /// * For the duration of the lifetime annotated on this `ArcBorrow`, the reference count must
+ /// not hit zero.
+ /// * For the duration of the lifetime annotated on this `ArcBorrow`, there must not be a
+ /// [`UniqueArc`] reference to this value.
+ pub unsafe fn from_raw(ptr: *const T) -> Self {
+ // SAFETY: The caller promises that this pointer originates from a call to `into_raw` on an
+ // `Arc` that is still valid.
+ let ptr = unsafe { ArcInner::container_of(ptr) };
+
+ // SAFETY: The caller promises that the value remains valid since the reference count must
+ // not hit zero, and no mutable reference will be created since that would involve a
+ // `UniqueArc`.
+ unsafe { Self::new(ptr) }
+ }
}
impl<T: ?Sized> From<ArcBorrow<'_, T>> for Arc<T> {
--
2.44.0.rc1.240.g4c46232300-googOn 2/28/24 14:00, Alice Ryhl wrote:
> Allows access to a value in an `Arc` that is currently held as a raw
> pointer due to use of `Arc::into_raw`, without destroying or otherwise
> consuming that raw pointer.
>
> This is a dependency of the linked list that Rust Binder uses. The
> linked list uses this method when iterating over the linked list.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/kernel/sync/arc.rs | 76 +++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 58 insertions(+), 18 deletions(-)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 7d4c4bf58388..53addb8876c2 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -137,6 +137,39 @@ struct ArcInner<T: ?Sized> {
> data: T,
> }
>
> +impl<T: ?Sized> ArcInner<T> {
> + /// Converts a pointer to the contents of an [`Arc`] into a pointer to the [`ArcInner`].
> + ///
> + /// # Safety
> + ///
> + /// `ptr` must have been returned by a previous call to [`Arc::into_raw`], and the `Arc` must
> + /// not yet have been destroyed.
> + unsafe fn container_of(ptr: *const T) -> NonNull<ArcInner<T>> {
> + let refcount_layout = Layout::new::<bindings::refcount_t>();
> + // SAFETY: The caller guarantees that the pointer is valid.
> + let val_layout = Layout::for_value(unsafe { &*ptr });
> + // SAFETY: We're computing the layout of a real struct that existed when compiling this
> + // binary, so its layout is not so large that it can trigger arithmetic overflow.
> + let val_offset = unsafe { refcount_layout.extend(val_layout).unwrap_unchecked().1 };
> +
> + // Pointer casts leave the metadata unchanged. This is okay because the metadata of `T` and
> + // `ArcInner<T>` is the same since `ArcInner` is a struct with `T` as its last field.
> + //
> + // This is documented at:
> + // <https://doc.rust-lang.org/std/ptr/trait.Pointee.html>.
> + let ptr = ptr as *const ArcInner<T>;
> +
> + // SAFETY: The pointer is in-bounds of an allocation both before and after offsetting the
> + // pointer, since it originates from a previous call to `Arc::into_raw` on an `Arc` that is
> + // still valid.
> + let ptr = unsafe { ptr.byte_sub(val_offset) };
> +
> + // SAFETY: The pointer can't be null since you can't have an `ArcInner<T>` value at the null
> + // address.
> + unsafe { NonNull::new_unchecked(ptr.cast_mut()) }
> + }
> +}
> +
> // This is to allow [`Arc`] (and variants) to be used as the type of `self`.
> impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
>
> @@ -232,27 +265,13 @@ pub fn into_raw(self) -> *const T {
> /// `ptr` must have been returned by a previous call to [`Arc::into_raw`]. Additionally, it
> /// must not be called more than once for each previous call to [`Arc::into_raw`].
> pub unsafe fn from_raw(ptr: *const T) -> Self {
> - let refcount_layout = Layout::new::<bindings::refcount_t>();
> - // SAFETY: The caller guarantees that the pointer is valid.
> - let val_layout = Layout::for_value(unsafe { &*ptr });
> - // SAFETY: We're computing the layout of a real struct that existed when compiling this
> - // binary, so its layout is not so large that it can trigger arithmetic overflow.
> - let val_offset = unsafe { refcount_layout.extend(val_layout).unwrap_unchecked().1 };
> -
> - // Pointer casts leave the metadata unchanged. This is okay because the metadata of `T` and
> - // `ArcInner<T>` is the same since `ArcInner` is a struct with `T` as its last field.
> - //
> - // This is documented at:
> - // <https://doc.rust-lang.org/std/ptr/trait.Pointee.html>.
> - let ptr = ptr as *const ArcInner<T>;
> -
> - // SAFETY: The pointer is in-bounds of an allocation both before and after offsetting the
> - // pointer, since it originates from a previous call to `Arc::into_raw` and is still valid.
> - let ptr = unsafe { ptr.byte_sub(val_offset) };
> + // SAFETY: The caller promises that this pointer originates from a call to `into_raw` on an
> + // `Arc` that is still valid.
> + let ptr = unsafe { ArcInner::container_of(ptr) };
>
> // SAFETY: By the safety requirements we know that `ptr` came from `Arc::into_raw`, so the
> // reference count held then will be owned by the new `Arc` object.
> - unsafe { Self::from_inner(NonNull::new_unchecked(ptr.cast_mut())) }
> + unsafe { Self::from_inner(ptr) }
> }
>
> /// Returns an [`ArcBorrow`] from the given [`Arc`].
> @@ -453,6 +472,27 @@ unsafe fn new(inner: NonNull<ArcInner<T>>) -> Self {
> _p: PhantomData,
> }
> }
> +
> + /// Creates an [`ArcBorrow`] to an [`Arc`] that has previously been deconstructed with
> + /// [`Arc::into_raw`].
> + ///
> + /// # Safety
> + ///
> + /// * The provided pointer must originate from a call to [`Arc::into_raw`].
> + /// * For the duration of the lifetime annotated on this `ArcBorrow`, the reference count must
> + /// not hit zero.
> + /// * For the duration of the lifetime annotated on this `ArcBorrow`, there must not be a
> + /// [`UniqueArc`] reference to this value.
I am a bit confused, this feels to me like it should be guaranteed by
`UniqueArc` and not by this function. Currently there is not even a way
of getting a `*const T` from a `UniqueArc`.
So I think we can remove this requirement and instead have the
requirement for creating `UniqueArc` that not only the refcount is
exactly 1, but also that no `ArcBorrow` exists.
--
Cheers,
Benno
> + pub unsafe fn from_raw(ptr: *const T) -> Self {
> + // SAFETY: The caller promises that this pointer originates from a call to `into_raw` on an
> + // `Arc` that is still valid.
> + let ptr = unsafe { ArcInner::container_of(ptr) };
> +
> + // SAFETY: The caller promises that the value remains valid since the reference count must
> + // not hit zero, and no mutable reference will be created since that would involve a
> + // `UniqueArc`.
> + unsafe { Self::new(ptr) }
> + }
> }
>
> impl<T: ?Sized> From<ArcBorrow<'_, T>> for Arc<T> {
>
> --
> 2.44.0.rc1.240.g4c46232300-goog
>
On Sat, Mar 9, 2024 at 1:56 PM Benno Lossin <benno.lossin@proton.me> wrote: > > + /// Creates an [`ArcBorrow`] to an [`Arc`] that has previously been deconstructed with > > + /// [`Arc::into_raw`]. > > + /// > > + /// # Safety > > + /// > > + /// * The provided pointer must originate from a call to [`Arc::into_raw`]. > > + /// * For the duration of the lifetime annotated on this `ArcBorrow`, the reference count must > > + /// not hit zero. > > + /// * For the duration of the lifetime annotated on this `ArcBorrow`, there must not be a > > + /// [`UniqueArc`] reference to this value. > > I am a bit confused, this feels to me like it should be guaranteed by > `UniqueArc` and not by this function. Currently there is not even a way > of getting a `*const T` from a `UniqueArc`. > So I think we can remove this requirement and instead have the > requirement for creating `UniqueArc` that not only the refcount is > exactly 1, but also that no `ArcBorrow` exists. If you combine this with `into_unique_or_drop` that is introduced in the next patch of this series, then you could perform these operations: * Arc::into_raw * ArcBorrow::from_raw * Arc::from_raw * Arc::into_unique_or_drop * And then use the ArcBorrow If we drop the final safety requirement from `ArcBorrow::from_raw`, then the above would be allowed. The refcount does not hit zero at any point during these operations. The only unsafe functions are Arc::into_raw, Arc::from_raw, and ArcBorrow::from_raw, so this safety requirement must go on one of them. It seems to me that, out of these, ArcBorrow::from_raw is the most appropriate choice. Thoughts? Alice
On 3/11/24 09:58, Alice Ryhl wrote: > On Sat, Mar 9, 2024 at 1:56 PM Benno Lossin <benno.lossin@proton.me> wrote: >>> + /// Creates an [`ArcBorrow`] to an [`Arc`] that has previously been deconstructed with >>> + /// [`Arc::into_raw`]. >>> + /// >>> + /// # Safety >>> + /// >>> + /// * The provided pointer must originate from a call to [`Arc::into_raw`]. >>> + /// * For the duration of the lifetime annotated on this `ArcBorrow`, the reference count must >>> + /// not hit zero. >>> + /// * For the duration of the lifetime annotated on this `ArcBorrow`, there must not be a >>> + /// [`UniqueArc`] reference to this value. >> >> I am a bit confused, this feels to me like it should be guaranteed by >> `UniqueArc` and not by this function. Currently there is not even a way >> of getting a `*const T` from a `UniqueArc`. >> So I think we can remove this requirement and instead have the >> requirement for creating `UniqueArc` that not only the refcount is >> exactly 1, but also that no `ArcBorrow` exists. > > If you combine this with `into_unique_or_drop` that is introduced in > the next patch of this series, then you could perform these > operations: > > * Arc::into_raw > * ArcBorrow::from_raw > * Arc::from_raw > * Arc::into_unique_or_drop > * And then use the ArcBorrow > > If we drop the final safety requirement from `ArcBorrow::from_raw`, > then the above would be allowed. The refcount does not hit zero at any > point during these operations. The only unsafe functions are > Arc::into_raw, Arc::from_raw, and ArcBorrow::from_raw, so this safety > requirement must go on one of them. It seems to me that, out of these, > ArcBorrow::from_raw is the most appropriate choice. > > Thoughts? I see, it is a bit unfortunate that we have to put the constraint onto `ArcBorrow::from_raw`, but I also do not see a better place. Thus: Reviewed-by: Benno Lossin <benno.lossin@proton.me> -- Cheers, Benno
© 2016 - 2026 Red Hat, Inc.