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

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

[Philipp Stanner]

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.

For unknown reasons, that problem was so far not visible and only gets
visible once one utilizes the list implementation from within the core
crate:

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.

Cc: stable@vger.kernel.org # v6.17+
Fixes: c77f85b347dd ("rust: list: remove OFFSET constants")
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
Changes in v2:
  - Add unsafes to the list implementation instead of container_of
    itself. (Alice, Miguel)
  - Adjust commit message and Fixes: tag.
  - Add stable-kernel for completeness.
---
 rust/kernel/list/impl_list_item_mod.rs | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index 202bc6f97c13..7052095efde5 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -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 {
-- 
2.49.0

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

[Miguel Ojeda]

On Tue, Nov 18, 2025 at 8:29 AM Philipp Stanner <phasta@kernel.org> wrote:
>
> For unknown reasons, that problem was so far not visible and only gets
> visible once one utilizes the list implementation from within the core
> crate:

What do you mean by "unknown reasons"? The reason is known -- please
refer to my message in the other thread. It should be mentioned in the
log, including the link to the compiler issue.

Also, I assume you meant `kernel` crate, not `core`.

> Cc: stable@vger.kernel.org # v6.17+

No need to mention the kernel version if the Fixes tags implies it. In
fact, it is more confusing, since it looks like there is a version
where it should not be applied to.

> -                let container = $crate::container_of!(
> +                let container = unsafe { $crate::container_of!(
>                      links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
> -                );
> +                ) };

Unsafe blocks require `// SAFETY: ...` comments.

Also, please double-check if this is the formatting that `rustfmt` would use.

Thanks!

Cheers,
Miguel

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

[Philipp Stanner]

On Tue, 2025-11-18 at 09:20 +0100, Miguel Ojeda wrote:
> On Tue, Nov 18, 2025 at 8:29 AM Philipp Stanner <phasta@kernel.org> wrote:
> > 
> > For unknown reasons, that problem was so far not visible and only gets
> > visible once one utilizes the list implementation from within the core
> > crate:
> 
> What do you mean by "unknown reasons"? The reason is known -- please
> refer to my message in the other thread. It should be mentioned in the
> log, including the link to the compiler issue.

OK.

> 
> Also, I assume you meant `kernel` crate, not `core`.
> 
> > Cc: stable@vger.kernel.org # v6.17+
> 
> No need to mention the kernel version if the Fixes tags implies it. In
> fact, it is more confusing, since it looks like there is a version
> where it should not be applied to.

It's absolutely common to provide it. If you feel better without it, I
can omit it, I guess.

> 
> > -                let container = $crate::container_of!(
> > +                let container = unsafe { $crate::container_of!(
> >                      links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
> > -                );
> > +                ) };
> 
> Unsafe blocks require `// SAFETY: ...` comments.

Ah, right. Overlooked that because the other section already has one.

> 
> Also, please double-check if this is the formatting that `rustfmt` would use.

I ran rustfmt.


P.

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

[Miguel Ojeda]

On Tue, Nov 18, 2025 at 9:30 AM Philipp Stanner <phasta@mailbox.org> wrote:
>
> It's absolutely common to provide it. If you feel better without it, I
> can omit it, I guess.

No, it is not "absolutely common" to provide it in a case like this,
and it is not about "feeling better" either.

As I already explained, it is confusing and takes more time to review.
For instance, it made me double-check why you wanted to skip some
versions or why the constraint was there. Please understand that
maintainers will need to check what you wrote there and whether it is
correct.

It is also riskier for yourself, since one can also easily get it wrong.

Those constraints, as the stable kernel rules explain, are about
sending additional instructions. It also explicitly says such tagging
is unnecessary when the Fixes tag is enough.

So, no, please do not add redundant constraints when they are not needed.

> I ran rustfmt.

Yes, but this is a macro -- `rustfmt` is likely not formatting that
code. In formatted code, there are no multiline `unsafe` blocks that
contain code after the opening brace, so it looks off.

Cheers,
Miguel

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

[Philipp Stanner]

On Tue, 2025-11-18 at 10:00 +0100, Miguel Ojeda wrote:
> On Tue, Nov 18, 2025 at 9:30 AM Philipp Stanner <phasta@mailbox.org> wrote:
> > 
> > It's absolutely common to provide it. If you feel better without it, I
> > can omit it, I guess.
> 
> No, it is not "absolutely common" to provide it in a case like this,
> and it is not about "feeling better" either.

It *is* absolutely common, or at least frequent, and you are the first
guy in the entire project I ever heard complaining about it. Maybe it
is often used wrongly or unnecessarily, though.

But no worries, be assured that I will take this detail into account
when working with you.

> 
> > I ran rustfmt.
> 
> Yes, but this is a macro -- `rustfmt` is likely not formatting that
> code. In formatted code, there are no multiline `unsafe` blocks that
> contain code after the opening brace, so it looks off.

So why then do you even suggest running rustfmt? How should I make it
check the formatting?

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

[Miguel Ojeda]

On Tue, Nov 18, 2025 at 10:33 AM Philipp Stanner <phasta@mailbox.org> wrote:
>
> It *is* absolutely common, or at least frequent, and you are the first
> guy in the entire project I ever heard complaining about it. Maybe it
> is often used wrongly or unnecessarily, though.
>
> But no worries, be assured that I will take this detail into account
> when working with you.

It is not, and since you seem to be keen on making an issue out of
this, I took a quick look.

What is actually common is using Cc: stable *without* constraints:
~84% of the cases in 2025.

And that is *before* removing the cases where the constraint is actually needed.

> So why then do you even suggest running rustfmt? How should I make it
> check the formatting?

I didn't suggest running `rustfmt`. I asked to please double-check if
that is the formatting that `rustfmt` *would* use, i.e. how those
blocks are normally formatted elsewhere, precisely because the tool
does not format it here.

Now, it isn't nice to try to frame things as if the issue is with the
other person (with remarks such as "first guy in the entire project"
and "when working with you").

So I will help you no further.

Cheers,
Miguel