All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Andreas Hindborg" <a.hindborg@kernel.org>
Cc: linux-pci@vger.kernel.org, dri-devel@lists.freedesktop.org,
	ojeda@kernel.org
Subject: Re: [PATCH v19 2/8] rust: types: Add Ownable/Owned types
Date: Fri, 26 Jun 2026 12:06:02 +0000	[thread overview]
Message-ID: <20260626120603.1E8971F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260626-unique-ref-v19-2-2607ca88dfdf@kernel.org>

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 <lina+kernel@asahilina.net>

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/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/

> 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/20260626-unique-ref-v19-0-2607ca88dfdf@kernel.org?part=2

  reply	other threads:[~2026-06-26 12:06 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-26 11:53 [PATCH v19 0/8] rust: add `Ownable` trait and `Owned` type Andreas Hindborg
2026-06-26 11:53 ` [PATCH v19 1/8] rust: alloc: add `KBox::into_non_null` Andreas Hindborg
2026-06-26 12:01   ` sashiko-bot
2026-06-26 11:53 ` [PATCH v19 2/8] rust: types: Add Ownable/Owned types Andreas Hindborg
2026-06-26 12:06   ` sashiko-bot [this message]
2026-06-26 11:54 ` [PATCH v19 3/8] rust: implement `ForeignOwnable` for `Owned` Andreas Hindborg
2026-06-26 12:07   ` sashiko-bot
2026-06-26 11:54 ` [PATCH v19 4/8] rust: page: convert to `Ownable` Andreas Hindborg
2026-06-26 12:05   ` sashiko-bot
2026-06-26 11:54 ` [PATCH v19 5/8] rust: rename `AlwaysRefCounted` to `RefCounted` Andreas Hindborg
2026-06-26 12:06   ` sashiko-bot
2026-06-26 11:54 ` [PATCH v19 6/8] rust: Add missing SAFETY documentation for `ARef` example Andreas Hindborg
2026-06-26 12:02   ` sashiko-bot
2026-06-26 11:54 ` [PATCH v19 7/8] rust: Add `OwnableRefCounted` Andreas Hindborg
2026-06-26 12:06   ` sashiko-bot
2026-06-26 11:54 ` [PATCH v19 8/8] rust: page: add `from_raw()` Andreas Hindborg
2026-06-26 12:06   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260626120603.1E8971F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.