[PATCH v2] rust: list: fix SAFETY comments in impl_list_item_mod

Christian Benton posted 1 patch 2 months, 1 week ago
rust/kernel/list/impl_list_item_mod.rs | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
[PATCH v2] rust: list: fix SAFETY comments in impl_list_item_mod
Posted by Christian Benton 2 months, 1 week ago
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.

v2: Fix incorrect SAFETY comment per Alice Ryhl's review
  - The cast is valid because ListLinksSelfPtr is repr(C) and ListLinks
    is its first field, not because they share the same memory layout.

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

diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index 5a3eac9f3cf0..48d4da349b56 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -86,7 +86,12 @@ 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 `repr(C)` and its first field is `ListLinks`, so the
+        // pointer to `ListLinksSelfPtr` can be cast to a pointer to its first field.
+        // 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 +279,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 +334,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