public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] rust: list: fix incomplete SAFETY comments in list implementation
@ 2026-04-03 22:08 Christian Benton
  2026-04-03 22:08 ` [PATCH 1/2] rust: list: fix SAFETY comment in List::remove Christian Benton
  2026-04-03 22:08 ` [PATCH 2/2] rust: list: fix SAFETY comments in impl_list_item_mod Christian Benton
  0 siblings, 2 replies; 5+ messages in thread
From: Christian Benton @ 2026-04-03 22:08 UTC (permalink / raw)
  To: ojeda; +Cc: rust-for-linux, linux-kernel, aliceryhl, lossin, Christian Benton

Four SAFETY comments in the linked list implementation were left as
TODO. This series fills them in with proper safety proofs explaining
why each unsafe operation is sound.

Christian Benton (2):
  rust: list: fix SAFETY comment in List::remove
  rust: list: fix SAFETY comments in impl_list_item_mod

 rust/kernel/list.rs                    |  4 +++-
 rust/kernel/list/impl_list_item_mod.rs | 17 ++++++++++++++---
 2 files changed, 17 insertions(+), 4 deletions(-)

-- 
2.53.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] rust: list: fix SAFETY comment in List::remove
  2026-04-03 22:08 [PATCH 0/2] rust: list: fix incomplete SAFETY comments in list implementation Christian Benton
@ 2026-04-03 22:08 ` Christian Benton
  2026-04-07  8:15   ` Alice Ryhl
  2026-04-03 22:08 ` [PATCH 2/2] rust: list: fix SAFETY comments in impl_list_item_mod Christian Benton
  1 sibling, 1 reply; 5+ messages in thread
From: Christian Benton @ 2026-04-03 22:08 UTC (permalink / raw)
  To: ojeda; +Cc: rust-for-linux, linux-kernel, aliceryhl, lossin, Christian Benton

The SAFETY comment for the call to ListLinks::fields in List::remove
was left as TODO. Fill it in: the call is safe because T::view_links
returns a reference to the ListLinks field of item, and references are
always valid and non-dangling.

Signed-off-by: Christian Benton <t1bur0n.kernel.org@protonmail.ch>
---
 rust/kernel/list.rs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index 406e3a028..b5705af43 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -599,7 +599,9 @@ pub fn pop_front(&mut self) -> Option<ListArc<T, ID>> {
     ///
     /// `item` must not be in a different linked list (with the same id).
     pub unsafe fn remove(&mut self, item: &T) -> Option<ListArc<T, ID>> {
-        // SAFETY: TODO.
+        // SAFETY: `T::view_links` returns a reference to the `ListLinks` field of `item`.
+        // References are always valid and non-dangling, so converting to a raw pointer
+        // via `ListLinks::fields` is sound.
         let mut item = unsafe { ListLinks::fields(T::view_links(item)) };
         // SAFETY: The user provided a reference, and reference are never dangling.
         //
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] rust: list: fix SAFETY comments in impl_list_item_mod
  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-03 22:08 ` Christian Benton
  2026-04-07  8:18   ` Alice Ryhl
  1 sibling, 1 reply; 5+ messages in thread
From: Christian Benton @ 2026-04-03 22:08 UTC (permalink / raw)
  To: ojeda; +Cc: rust-for-linux, linux-kernel, aliceryhl, lossin, Christian Benton

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.
         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



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] rust: list: fix SAFETY comment in List::remove
  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
  0 siblings, 0 replies; 5+ messages in thread
From: Alice Ryhl @ 2026-04-07  8:15 UTC (permalink / raw)
  To: Christian Benton; +Cc: ojeda, rust-for-linux, linux-kernel, lossin

On Fri, Apr 03, 2026 at 10:08:15PM +0000, Christian Benton wrote:
> The SAFETY comment for the call to ListLinks::fields in List::remove
> was left as TODO. Fill it in: the call is safe because T::view_links
> returns a reference to the ListLinks field of item, and references are
> always valid and non-dangling.
> 
> Signed-off-by: Christian Benton <t1bur0n.kernel.org@protonmail.ch>

Thanks. I agree that `item` being a reference is the only thing needed
for this to be sound, as reference implies that the pointer is not
dangling.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] rust: list: fix SAFETY comments in impl_list_item_mod
  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
  0 siblings, 0 replies; 5+ messages in thread
From: Alice Ryhl @ 2026-04-07  8:18 UTC (permalink / raw)
  To: Christian Benton; +Cc: ojeda, rust-for-linux, linux-kernel, lossin

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
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-07  8:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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-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 is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox