Skip to contentSkip to conversation

rust: list: Add unsafe for container_of

PATCH Superseded Philipp Stanner 2 participants 2 messages Feb 9

Subsystems (2)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 rust: list: Add unsafe for container_of

impl_list_item_mod.rs calls container_of() without unsafe blocks at a couple of places. Since container_of() is an unsafe macro / function, the blocks are strictly necessary.

The problem was so far not visible because the "unsafe-op-in-unsafe-fn" check is a linter rather than a compiler check. Rust suppresses lint checks triggered inside of a macro from another crate. Thus, the error becomes only visible once someone from without the core crate tries to use linked lists:

error[E0133]: call to unsafe function core::ptr::mut_ptr::<impl *mut T>::byte_sub is unsafe and requires unsafe block
--> rust/kernel/lib.rs:252:29

    |
252 |           let container_ptr = field_ptr.byte_sub(offset).cast::<$Container>();
    |                               ^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function
    |
   ::: rust/kernel/drm/jq.rs:98:1
    |
98  | / impl_list_item! {
99  | |     impl ListItem<0> for BasicItem { using ListLinks { self.links }; }
100 | | }
    | |_- in this macro invocation
    |
note: an unsafe function restricts its caller, but its body is safe by default
   --> rust/kernel/list/impl_list_item_mod.rs:216:13
    |
216 |               unsafe fn view_value(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
    |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
   ::: rust/kernel/drm/jq.rs:98:1
    |
98  | / impl_list_item! {
99  | |     impl ListItem<0> for BasicItem { using ListLinks { self.links }; }
100 | | }
    | |_- in this macro invocation

= note: requested on the command line with -D unsafe-op-in-unsafe-fn = note: this error originates in the macro $crate::container_of which comes from the expansion of the macro impl_list_item

Add unsafe blocks to container_of to fix the issue.

Fixes c77f85b347dd ("rust: list: remove OFFSET constants")

---
Changes in v2:
- Tidy up commit message and provide exact reason for the error.
- Add Reviewed-bys.

---
 rust/kernel/list/impl_list_item_mod.rs | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
rust/kernel/list/impl_list_item_mod.rs +6 -6
index 202bc6f97c13..7052095efde5 100644
@@ -217,7 +217,7 @@ unsafe fn view_value(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
                 // SAFETY: `me` originates from the most recent call to `prepare_to_insert`, so it
                 // points at the field `$field` in a value of type `Self`. Thus, reversing that
                 // operation is still in-bounds of the allocation.
-                $crate::container_of!(me, Self, $($field).*)
+                unsafe { $crate::container_of!(me, Self, $($field).*) }
             }
 
             // GUARANTEES:
@@ -242,7 +242,7 @@ unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
                 // SAFETY: `me` originates from the most recent call to `prepare_to_insert`, so it
                 // points at the field `$field` in a value of type `Self`. Thus, reversing that
                 // operation is still in-bounds of the allocation.
-                $crate::container_of!(me, Self, $($field).*)
+                unsafe { $crate::container_of!(me, Self, $($field).*) }
             }
         }
     )*};
@@ -270,9 +270,9 @@ 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) };
 
-                let container = $crate::container_of!(
+                let container = unsafe { $crate::container_of!(
                     links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
-                );
+                ) };
 
                 // SAFETY: By the same reasoning above, `links_field` is a valid pointer.
                 let self_ptr = unsafe {
@@ -319,9 +319,9 @@ 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 {
-                let container = $crate::container_of!(
+                let container = unsafe { $crate::container_of!(
                     links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
-                );
+                ) };
 
                 // SAFETY: By the same reasoning above, `links_field` is a valid pointer.
                 let self_ptr = unsafe {
Signature · 1 line
2.49.0
#1 lore raw
Quoted 2 lines · Philipp Stanner

[PATCH v2] rust: list: Add unsafe for container_of

Isn't this v3?

rust/kernel/list/impl_list_item_mod.rs unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$nu · line 275

Quoted 5 lines
rust/kernel/list/impl_list_item_mod.rs +2 -2
-                let container = $crate::container_of!(
+                let container = unsafe { $crate::container_of!(
                     links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
-                );
+                ) };

This one and another one below need a // SAFETY: comment -- I mentioned it in v2:

https://lore.kernel.org/rust-for-linux/CANiq72md+0Lerj+kqr6QiU6ySR3XjRzmuBiLjkpWWieM72wyeQ@mail.gmail.com/

Thanks for fixing the commit message and resending this!

Cheers,
Miguel