rust/kernel/list/impl_list_item_mod.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
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.
Cc: stable@vger.kernel.org
Fixes: c77f85b347dd ("rust: list: remove OFFSET constants")
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Philipp Stanner <phasta@kernel.org>
Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
---
Changes in v4:
- Add unsafe comments. (Miguel)
Changes in v3 (was accidentally called "v2" before):
- Tidy up commit message and provide exact reason for the error.
- Add Reviewed-bys.
---
rust/kernel/list/impl_list_item_mod.rs | 15 +++++++++------
1 file changed, 9 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..9f9c36051cf6 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,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) };
- let container = $crate::container_of!(
+ // SAFETY: By the same reasoning above, `links_field` is a valid pointer.
+ 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 +320,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 {
- let container = $crate::container_of!(
+ // SAFETY: The caller must guarantee that `links_field` is a valid pointer of type
+ // ListLinks.
+ 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
On Mon, Feb 16, 2026 at 2:17 PM Philipp Stanner <phasta@kernel.org> wrote:
>
> 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.
>
> Cc: stable@vger.kernel.org
> Fixes: c77f85b347dd ("rust: list: remove OFFSET constants")
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
(I said it was applied, but for completeness, let me send my usual
message here... The commit is in mainline already.).
Applied to `rust-fixes` -- thanks everyone!
[ As discussed, let's fix the build for those that want to use the
macro within the `kernel` crate now and we can discuss the proper
safety comments afterwards. Thus I removed the ones from the patch.
However, we cannot just avoid the comments with `CLIPPY=1`, so I
provided placeholders for now, like we did in the past. They were
also needed for an `unsafe impl`.
While I am not happy about it, it isn't worse than the current
status (the comments were meant to be there), and at least this
shows what is missing -- our pre-existing "good first issue" [1]
may motivate new contributors to complete them properly.
Finally, I moved one of the existing safety comments one line down
so that Clippy could locate it.
Link: https://github.com/Rust-for-Linux/linux/issues/351 [1]
- Miguel ]
[ Fixed formatting. Reworded to fix the lint suppression
explanation. Indent build error. - Miguel ]
Cheers,
Miguel
On Mon, Feb 16, 2026 at 2:17 PM Philipp Stanner <phasta@kernel.org> wrote:
>
> Add unsafe blocks to container_of to fix the issue.
So I don't think this was tested with `CLIPPY=1`, because there are
other safety comments missing even after this is applied.
Please note that `CLIPPY=1` must remain clean for all Rust code:
https://rust-for-linux.com/contributing#submit-checklist-addendum
I have pushed to `rust-fixes` the patch with placeholders to show what
is missing.
I am not sure if I will keep it there. I am not too happy introducing
`// SAFETY: TODO.`s, but it isn't worse than the status (the comments
were meant to be there), and at least this shows what is missing --
our pre-existing "good first issue" [1] may motivate new contributors
to complete them properly.
Cheers,
Miguel
On Fri, 2026-02-20 at 02:08 +0100, Miguel Ojeda wrote: > On Mon, Feb 16, 2026 at 2:17 PM Philipp Stanner <phasta@kernel.org> wrote: > > > > Add unsafe blocks to container_of to fix the issue. > > So I don't think this was tested with `CLIPPY=1`, because there are > other safety comments missing even after this is applied. > > Please note that `CLIPPY=1` must remain clean for all Rust code: > > https://rust-for-linux.com/contributing#submit-checklist-addendum > > I have pushed to `rust-fixes` the patch with placeholders to show what > is missing. > > I am not sure if I will keep it there. > Feel free to drop it for now, I can go through it properly with the list author to see what the most accurate formulation would be.
On Mon, Feb 23, 2026 at 8:34 AM Philipp Stanner <phasta@mailbox.org> wrote: > > Feel free to drop it for now, I can go through it properly with the > list author to see what the most accurate formulation would be. The commit is in v7.0-rc1 -- please feel free to send the improvements on top of that. Thanks! Cheers, Miguel
On Mon, Feb 16, 2026 at 2:17 PM Philipp Stanner <phasta@kernel.org> wrote:
>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Hmm... Where do these Reviewed-by's come from?
Ah, I see them now -- a copy of this patch was moved into another series:
https://lore.kernel.org/rust-for-linux/20260203081403.68733-3-phasta@kernel.org/
I understand it was an RFC series, and perhaps you didn't intend to
land the patch, so addressing the previous feedback wasn't important
for the patch series.
Nevertheless, a note about it would have been nice -- we weren't Cc'd
for some reason, and there was no notification about the move nor a
link to the previous version.
I see Gary and Alice pointed out feedback that I had already given
before -- that is why it is important to mention that this was not the
first version of the patch or better, mention that there was feedback
yet to be addressed. Otherwise, reviewers may end up mentioning again
the same feedback, just like it happened here.
> + // SAFETY: The caller must guarantee that `links_field` is a valid pointer of type
> + // ListLinks.
> + let container = unsafe { $crate::container_of!(
> + // SAFETY: By the same reasoning above, `links_field` is a valid pointer.
> + let container = unsafe { $crate::container_of!(
I don't think these two comments justify the requirement of
`container_of!`, which is:
/// # Safety
///
/// The pointer passed to this macro, and the pointer returned by
this macro, must both be in
/// bounds of the same allocation.
i.e. even if `links_field` is valid, it could still be UB.
By the way, the "Reviewed-by" tags don't really apply to the `//
SAFETY:` comments, so I would have probably dropped
To make progress to get the build fix done, and to avoid dropping
those tags, I think I will apply the fix without the comments. Then
please feel free to send the safety comments separately, which can be
iterated independently. (In fact, we may want to reorganize a bit the
safety comments within the macro...).
Sounds good to everyone?
(The formatting is also still not right -- this was pointed out by at
least a couple people now. Anyway, I will fix it myself.).
Cheers,
Miguel
© 2016 - 2026 Red Hat, Inc.