Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The unsafe constructor `Owned::from_raw` and the extractor `Owned::into_raw` fail to require or document the necessary pinning constraints, allowing safe code to violate `Pin` guarantees. --
commit a0693c87c8e48e6a9ddc21b1871047bae4f76436 Author: Asahi Lina <[email protected]> rust: types: Add Ownable/Owned types This commit introduces the Ownable trait and Owned pointer type for handling unique ownership of C-allocated objects in Rust. It delegates resource management to the custom drop logic of the underlying type. Link: https://lore.kernel.org/all/[email protected]/ > diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs > new file mode 100644 > index 0000000000000..7fe9ec3e55126 > --- /dev/null > +++ b/rust/kernel/owned.rs [ ... ] > + /// Creates a new instance of [`Owned`]. > + /// > + /// This function takes over ownership of the underlying object. > + /// > + /// # Safety > + /// > + /// Callers must ensure that: > + /// - `ptr` points to a valid instance of `T`. > + /// - Until `T::release` is called, the returned `Owned<T>` exclusively > owns the underlying `T`. > + #[inline] > + pub unsafe fn from_raw(ptr: NonNull<T>) -> Self { [Severity: High] Does this safety documentation need to require the caller to provide a logically pinned allocation? Since Owned<T> guarantees its contents are pinned and provides as_pin_mut(), a caller could pass an unpinned pointer (like a stack variable) to from_raw(), fully complying with these documented preconditions. This would then allow safe code to obtain a Pin<&mut T> and potentially violate Pin invariants later. > + // INVARIANT: By function safety requirement we satisfy the first > invariant of `Self`. > + // We treat `T` as pinned from now on. > + Self { ptr } > + } > + > + /// Consumes the [`Owned`], returning a raw pointer. > + /// > + /// This function does not drop the underlying `T`. When this function > returns, ownership of the > + /// underlying `T` is with the caller. > + #[inline] > + pub fn into_raw(me: Self) -> NonNull<T> { [Severity: High] Should the documentation here warn the caller that the returned pointer inherits the Pin guarantee and must not be moved? Without this, a developer could safely extract the pointer via into_raw() and move the memory, causing undefined behavior by violating the Pin guarantees established while it was Owned. > + ManuallyDrop::new(me).ptr > + } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
