From: Alice Ryhl <aliceryhl@google.com>
To: Christian Benton <t1bur0n.kernel.org@protonmail.ch>
Cc: ojeda@kernel.org, rust-for-linux@vger.kernel.org,
linux-kernel@vger.kernel.org, lossin@kernel.org
Subject: Re: [PATCH 2/2] rust: list: fix SAFETY comments in impl_list_item_mod
Date: Tue, 7 Apr 2026 08:18:50 +0000 [thread overview]
Message-ID: <adS96lmBZBcsX5mN@google.com> (raw)
In-Reply-To: <20260403220751.15374-3-t1bur0n.kernel.org@protonmail.ch>
On Fri, Apr 03, 2026 at 10:08:24PM +0000, Christian Benton wrote:
> Three SAFETY comments were left as TODO in impl_list_item_mod.rs.
> Fill them in:
>
> - impl_has_list_links_self_ptr!: the HasListLinks impl is safe because
> raw_get_list_links only compiles if the field has type ListLinksSelfPtr,
> which the type system enforces statically.
>
> - prepare_to_insert: the container_of! call is safe because links_field
> is valid from view_links and Self: HasSelfPtr guarantees links_field
> points to the inner field of a ListLinksSelfPtr.
>
> - view_value: the container_of! call is safe because the caller of
> prepare_to_insert promised to retain the ListArc, and Self: HasSelfPtr
> guarantees links_field points to the inner field of a ListLinksSelfPtr.
>
> Signed-off-by: Christian Benton <t1bur0n.kernel.org@protonmail.ch>
> ---
> rust/kernel/list/impl_list_item_mod.rs | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
> index 5a3eac9f3..2d1f4723a 100644
> --- a/rust/kernel/list/impl_list_item_mod.rs
> +++ b/rust/kernel/list/impl_list_item_mod.rs
> @@ -86,7 +86,11 @@ macro_rules! impl_has_list_links_self_ptr {
> // right type.
> unsafe impl$(<$($generics)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
>
> - // SAFETY: TODO.
> + // SAFETY: The implementation of `raw_get_list_links` returns a pointer to the
> + // `ListLinks` field inside `ListLinksSelfPtr`. This cast is valid because
> + // `ListLinksSelfPtr` is a wrapper around `ListLinks` and shares the same memory
> + // layout. The macro only compiles if the field has type `ListLinksSelfPtr`, which
> + // the type system enforces statically.
No, `ListLinksSelfPtr` doesn't share the same memory layout as
`ListLinks`. It has an extra field. The cast is okay because of the
repr(C) annotation.
> unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
> #[inline]
> unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
> @@ -274,7 +278,10 @@ unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$nu
> // SAFETY: The caller promises that `me` points at a valid value of type `Self`.
> let links_field = unsafe { <Self as $crate::list::ListItem<$num>>::view_links(me) };
>
> - // SAFETY: TODO.
> + // SAFETY: `links_field` is valid because `view_links` returned it from a valid
> + // `me` pointer as promised by the caller. `links_field` points to the `inner`
> + // field of a `ListLinksSelfPtr` because `Self: HasSelfPtr` guarantees that the
> + // `ListLinks` field is always inside a `ListLinksSelfPtr`.
> let container = unsafe {
> $crate::container_of!(
> links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
> @@ -326,7 +333,11 @@ unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
> // `ListArc` containing `Self` until the next call to `post_remove`. The value cannot
> // be destroyed while a `ListArc` reference exists.
> unsafe fn view_value(links_field: *mut $crate::list::ListLinks<$num>) -> *const Self {
> - // SAFETY: TODO.
> + // SAFETY: `links_field` is valid and points to a live value because the caller
> + // of `prepare_to_insert` promised to retain ownership of the `ListArc`, and the
> + // value cannot be destroyed while a `ListArc` exists. `links_field` points to
> + // the `inner` field of a `ListLinksSelfPtr` because `Self: HasSelfPtr`
> + // guarantees this.
> let container = unsafe {
> $crate::container_of!(
> links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
> --
> 2.53.0
>
>
prev parent reply other threads:[~2026-04-07 8:18 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-03 22:08 [PATCH 0/2] rust: list: fix incomplete SAFETY comments in list implementation Christian Benton
2026-04-03 22:08 ` [PATCH 1/2] rust: list: fix SAFETY comment in List::remove Christian Benton
2026-04-07 8:15 ` Alice Ryhl
2026-04-07 11:56 ` Gary Guo
2026-04-24 11:29 ` Philipp Stanner
2026-04-27 16:53 ` Christian Benton
2026-04-27 23:08 ` Gary Guo
2026-04-28 7:27 ` Alice Ryhl
2026-04-28 10:39 ` Gary Guo
2026-04-03 22:08 ` [PATCH 2/2] rust: list: fix SAFETY comments in impl_list_item_mod Christian Benton
2026-04-07 8:18 ` Alice Ryhl [this message]
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=adS96lmBZBcsX5mN@google.com \
--to=aliceryhl@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=t1bur0n.kernel.org@protonmail.ch \
/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.