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 {
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
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.
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
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?
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
linux-stable-mirror@lists.linaro.org
-
Miguel Ojeda -
Philipp Stanner -
Philipp Stanner