Re: [PATCH v20 2/8] rust: types: Add Ownable/Owned types
| Newsgroups | org.freedesktop.lists.dri-devel,org.kernel.vger.linux-pci |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The safety documentation for `Owned::from_raw` fails to require the caller to uphold the pinning invariant. -- --- Patch [2]: [PATCH v20 2/8] rust: types: Add Ownable/Owned types --- commit 82ccf756b593f4c76a8a6694855add1d65c27d53 Author: Asahi Lina <[email protected]> rust: types: Add Ownable/Owned types This commit introduces Ownable and Owned types for unique ownership of objects with custom drop logic. It provides a mechanism for C-allocated objects to be owned by Rust without generic allocators, delegating resource management to the type itself. 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: Medium] Should the safety documentation for from_raw() explicitly require the caller to uphold the pinning invariant? Since as_pin_mut() returns a Pin<&mut T> and the internal comments note that T is treated as pinned from now on, it seems the caller must guarantee this. If a caller provides an unpinned allocation, retrieves the pointer later with into_raw(), and safely moves the object, it could violate the pinning assumptions established here. Could this lead to memory corruption if the pinned object is self-referential or registered elsewhere? > + // INVARIANT: By function safety requirement we satisfy the first invariant of `Self`. > + // We treat `T` as pinned from now on. > + Self { ptr } > + } -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2