Re: [PATCH v19 6/8] rust: Add missing SAFETY documentation for `ARef` example

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added SAFETY comments in the rustdoc example incorrectly justify an unsound trait implementation that violates the safety invariants of `RefCounted`.
--

commit 8c6c2bf519778f423165d5e3c916d3965bebb093
Author: Oliver Mangold <oliver.mangold@pm.me>

rust: Add missing SAFETY documentation for ARef example

This commit replaces TODO safety comments in the rustdoc example for
ARef::into_raw with explanations of why the unsafe operations are permitted.

> diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
> index ea5a16b8163a6..f26ca39b84d0d 100644
> --- a/rust/kernel/sync/aref.rs
> +++ b/rust/kernel/sync/aref.rs
[ ... ]
> @@ -142,7 +142,9 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>      ///
>      /// struct Empty {}
>      ///
> -    /// # // SAFETY: TODO.
> +    /// // SAFETY: The `RefCounted` implementation for `Empty` does not count references and never
> +    /// // frees the underlying object. Thus we can act as owning an increment on the refcount for
> +    /// // the object that we pass to the newly created `ARef`.
>      /// unsafe impl RefCounted for Empty {
>      ///     fn inc_ref(&self) {}
>      ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}

[Severity: Medium]
Does this explanation violate the safety invariants of the RefCounted trait?

The RefCounted trait strictly requires that increments to the reference count
keep the object alive, and that all instances are reference-counted.

By using a no-op inc_ref and dec_ref, this example demonstrates an
unsound implementation of the trait, which might lead developers to
introduce use-after-free bugs if they copy this pattern into production
code.

>      /// }
>      ///
>      /// let mut data = Empty {};
>      /// let ptr = NonNull::<Empty>::new(&mut data).unwrap();
> -    /// # // SAFETY: TODO.
> +    /// // SAFETY: We keep `data` around longer than the `ARef`.
>      /// let data_ref: ARef<Empty> = unsafe { ARef::from_raw(ptr) };

[Severity: Medium]
Is it safe to rely on stack-variable lifetimes here to bypass the normal
reference counting expectations?

ARef::from_raw and the RefCounted trait expect the object's lifetime to be
handled inherently via reference counting. Relying on keeping a stack
variable around longer than the ARef bypasses ARef::from_raw safety
preconditions.

Could this documentation lead others to incorrectly apply ARef to stack
variables?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-unique-ref-v19-0-2607ca88dfdf@kernel.org?part=6