linux/rust/kernel/list/impl_list_item_mod.rs
Philipp Stanner 97b281d7ed rust: list: Add unsafe blocks for container_of and safety comments
impl_list_item_mod.rs calls container_of! without unsafe blocks at a
couple of places. Since container_of! is unsafe, the blocks are strictly
necessary.

The problem was so far not visible because the "unsafe-op-in-unsafe-fn"
check is a lint rather than a hard compiler error, and Rust suppresses
lints triggered inside of a macro from another crate.

Thus, the error becomes only visible once someone from within the kernel
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`

Therefore, add unsafe blocks to container_of! calls to fix the issue.

[ 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 ]

Cc: stable@vger.kernel.org
Fixes: c77f85b347 ("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>
Link: https://patch.msgid.link/20260216131613.45344-3-phasta@kernel.org
[ Fixed formatting. Reworded to fix the lint suppression
  explanation. Indent build error. - Miguel ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
2026-02-20 01:59:51 +01:00

358 lines
16 KiB
Rust

// SPDX-License-Identifier: GPL-2.0
// Copyright (C) 2024 Google LLC.
//! Helpers for implementing list traits safely.
/// Declares that this type has a [`ListLinks<ID>`] field.
///
/// This trait is only used to help implement [`ListItem`] safely. If [`ListItem`] is implemented
/// manually, then this trait is not needed. Use the [`impl_has_list_links!`] macro to implement
/// this trait.
///
/// # Safety
///
/// The methods on this trait must have exactly the behavior that the definitions given below have.
///
/// [`ListLinks<ID>`]: crate::list::ListLinks
/// [`ListItem`]: crate::list::ListItem
pub unsafe trait HasListLinks<const ID: u64 = 0> {
/// Returns a pointer to the [`ListLinks<ID>`] field.
///
/// # Safety
///
/// The provided pointer must point at a valid struct of type `Self`.
///
/// [`ListLinks<ID>`]: crate::list::ListLinks
unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut crate::list::ListLinks<ID>;
}
/// Implements the [`HasListLinks`] trait for the given type.
#[macro_export]
macro_rules! impl_has_list_links {
($(impl$({$($generics:tt)*})?
HasListLinks$(<$id:tt>)?
for $self:ty
{ self$(.$field:ident)* }
)*) => {$(
// SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
// right type.
unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
#[inline]
unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
// Statically ensure that `$(.field)*` doesn't follow any pointers.
//
// Cannot be `const` because `$self` may contain generics and E0401 says constants
// "can't use {`Self`,generic parameters} from outer item".
if false { let _: usize = ::core::mem::offset_of!(Self, $($field).*); }
// SAFETY: The caller promises that the pointer is not dangling. We know that this
// expression doesn't follow any pointers, as the `offset_of!` invocation above
// would otherwise not compile.
unsafe { ::core::ptr::addr_of_mut!((*ptr)$(.$field)*) }
}
}
)*};
}
pub use impl_has_list_links;
/// Declares that the [`ListLinks<ID>`] field in this struct is inside a
/// [`ListLinksSelfPtr<T, ID>`].
///
/// # Safety
///
/// The [`ListLinks<ID>`] field of this struct at [`HasListLinks<ID>::raw_get_list_links`] must be
/// inside a [`ListLinksSelfPtr<T, ID>`].
///
/// [`ListLinks<ID>`]: crate::list::ListLinks
/// [`ListLinksSelfPtr<T, ID>`]: crate::list::ListLinksSelfPtr
pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
where
Self: HasListLinks<ID>,
{
}
/// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type.
#[macro_export]
macro_rules! impl_has_list_links_self_ptr {
($(impl$({$($generics:tt)*})?
HasSelfPtr<$item_type:ty $(, $id:tt)?>
for $self:ty
{ self$(.$field:ident)* }
)*) => {$(
// SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
// right type.
unsafe impl$(<$($generics)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
// SAFETY: TODO.
unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
#[inline]
unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
let ptr: *mut $crate::list::ListLinksSelfPtr<$item_type $(, $id)?> =
// SAFETY: The caller promises that the pointer is not dangling.
unsafe { ::core::ptr::addr_of_mut!((*ptr)$(.$field)*) };
ptr.cast()
}
}
)*};
}
pub use impl_has_list_links_self_ptr;
/// Implements the [`ListItem`] trait for the given type.
///
/// Requires that the type implements [`HasListLinks`]. Use the [`impl_has_list_links!`] macro to
/// implement that trait.
///
/// [`ListItem`]: crate::list::ListItem
///
/// # Examples
///
/// ```
/// #[pin_data]
/// struct SimpleListItem {
/// value: u32,
/// #[pin]
/// links: kernel::list::ListLinks,
/// }
///
/// kernel::list::impl_list_arc_safe! {
/// impl ListArcSafe<0> for SimpleListItem { untracked; }
/// }
///
/// kernel::list::impl_list_item! {
/// impl ListItem<0> for SimpleListItem { using ListLinks { self.links }; }
/// }
///
/// struct ListLinksHolder {
/// inner: kernel::list::ListLinks,
/// }
///
/// #[pin_data]
/// struct ComplexListItem<T, U> {
/// value: Result<T, U>,
/// #[pin]
/// links: ListLinksHolder,
/// }
///
/// kernel::list::impl_list_arc_safe! {
/// impl{T, U} ListArcSafe<0> for ComplexListItem<T, U> { untracked; }
/// }
///
/// kernel::list::impl_list_item! {
/// impl{T, U} ListItem<0> for ComplexListItem<T, U> { using ListLinks { self.links.inner }; }
/// }
/// ```
///
/// ```
/// #[pin_data]
/// struct SimpleListItem {
/// value: u32,
/// #[pin]
/// links: kernel::list::ListLinksSelfPtr<SimpleListItem>,
/// }
///
/// kernel::list::impl_list_arc_safe! {
/// impl ListArcSafe<0> for SimpleListItem { untracked; }
/// }
///
/// kernel::list::impl_list_item! {
/// impl ListItem<0> for SimpleListItem { using ListLinksSelfPtr { self.links }; }
/// }
///
/// struct ListLinksSelfPtrHolder<T, U> {
/// inner: kernel::list::ListLinksSelfPtr<ComplexListItem<T, U>>,
/// }
///
/// #[pin_data]
/// struct ComplexListItem<T, U> {
/// value: Result<T, U>,
/// #[pin]
/// links: ListLinksSelfPtrHolder<T, U>,
/// }
///
/// kernel::list::impl_list_arc_safe! {
/// impl{T, U} ListArcSafe<0> for ComplexListItem<T, U> { untracked; }
/// }
///
/// kernel::list::impl_list_item! {
/// impl{T, U} ListItem<0> for ComplexListItem<T, U> {
/// using ListLinksSelfPtr { self.links.inner };
/// }
/// }
/// ```
#[macro_export]
macro_rules! impl_list_item {
(
$(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty {
using ListLinks { self$(.$field:ident)* };
})*
) => {$(
$crate::list::impl_has_list_links! {
impl$({$($generics)*})? HasListLinks<$num> for $self { self$(.$field)* }
}
// SAFETY: See GUARANTEES comment on each method.
unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
// GUARANTEES:
// * This returns the same pointer as `prepare_to_insert` because `prepare_to_insert`
// is implemented in terms of `view_links`.
// * By the type invariants of `ListLinks`, the `ListLinks` has two null pointers when
// this value is not in a list.
unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
// SAFETY: The caller guarantees that `me` points at a valid value of type `Self`.
unsafe {
<Self as $crate::list::HasListLinks<$num>>::raw_get_list_links(me.cast_mut())
}
}
// GUARANTEES:
// * `me` originates from the most recent call to `prepare_to_insert`, which calls
// `raw_get_list_link`, which is implemented using `addr_of_mut!((*self)$(.$field)*)`.
// This method uses `container_of` to perform the inverse operation, so it returns the
// pointer originally passed to `prepare_to_insert`.
// * The pointer remains valid until the next call to `post_remove` because the caller
// of the most recent call to `prepare_to_insert` promised to retain ownership of the
// `ListArc` containing `Self` until the next call to `post_remove`. The value cannot
// be destroyed while a `ListArc` reference exists.
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.
unsafe { $crate::container_of!(me, Self, $($field).*) }
}
// GUARANTEES:
// This implementation of `ListItem` will not give out exclusive access to the same
// `ListLinks` several times because calls to `prepare_to_insert` and `post_remove`
// must alternate and exclusive access is given up when `post_remove` is called.
//
// Other invocations of `impl_list_item!` also cannot give out exclusive access to the
// same `ListLinks` because you can only implement `ListItem` once for each value of
// `ID`, and the `ListLinks` fields only work with the specified `ID`.
unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
// SAFETY: The caller promises that `me` points at a valid value.
unsafe { <Self as $crate::list::ListItem<$num>>::view_links(me) }
}
// GUARANTEES:
// * `me` originates from the most recent call to `prepare_to_insert`, which calls
// `raw_get_list_link`, which is implemented using `addr_of_mut!((*self)$(.$field)*)`.
// This method uses `container_of` to perform the inverse operation, so it returns the
// pointer originally passed to `prepare_to_insert`.
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.
unsafe { $crate::container_of!(me, Self, $($field).*) }
}
}
)*};
(
$(impl$({$($generics:tt)*})? ListItem<$num:tt> for $self:ty {
using ListLinksSelfPtr { self$(.$field:ident)* };
})*
) => {$(
$crate::list::impl_has_list_links_self_ptr! {
impl$({$($generics)*})? HasSelfPtr<$self> for $self { self$(.$field)* }
}
// SAFETY: See GUARANTEES comment on each method.
unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $self {
// GUARANTEES:
// This implementation of `ListItem` will not give out exclusive access to the same
// `ListLinks` several times because calls to `prepare_to_insert` and `post_remove`
// must alternate and exclusive access is given up when `post_remove` is called.
//
// Other invocations of `impl_list_item!` also cannot give out exclusive access to the
// same `ListLinks` because you can only implement `ListItem` once for each value of
// `ID`, and the `ListLinks` fields only work with the specified `ID`.
unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
// 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.
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 {
$crate::list::ListLinksSelfPtr::raw_get_self_ptr(container)
};
let cell_inner = $crate::types::Opaque::cast_into(self_ptr);
// SAFETY: This value is not accessed in any other places than `prepare_to_insert`,
// `post_remove`, or `view_value`. By the safety requirements of those methods,
// none of these three methods may be called in parallel with this call to
// `prepare_to_insert`, so this write will not race with any other access to the
// value.
unsafe { ::core::ptr::write(cell_inner, me) };
links_field
}
// GUARANTEES:
// * This returns the same pointer as `prepare_to_insert` because `prepare_to_insert`
// returns the return value of `view_links`.
// * By the type invariants of `ListLinks`, the `ListLinks` has two null pointers when
// this value is not in a list.
unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
// SAFETY: The caller promises that `me` points at a valid value of type `Self`.
unsafe {
<Self as $crate::list::HasListLinks<$num>>::raw_get_list_links(me.cast_mut())
}
}
// This function is also used as the implementation of `post_remove`, so the caller
// may choose to satisfy the safety requirements of `post_remove` instead of the safety
// requirements for `view_value`.
//
// GUARANTEES: (always)
// * This returns the same pointer as the one passed to the most recent call to
// `prepare_to_insert` since that call wrote that pointer to this location. The value
// is only modified in `prepare_to_insert`, so it has not been modified since the
// most recent call.
//
// GUARANTEES: (only when using the `view_value` safety requirements)
// * The pointer remains valid until the next call to `post_remove` because the caller
// of the most recent call to `prepare_to_insert` promised to retain ownership of the
// `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.
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 {
$crate::list::ListLinksSelfPtr::raw_get_self_ptr(container)
};
let cell_inner = $crate::types::Opaque::cast_into(self_ptr);
// SAFETY: This is not a data race, because the only function that writes to this
// value is `prepare_to_insert`, but by the safety requirements the
// `prepare_to_insert` method may not be called in parallel with `view_value` or
// `post_remove`.
unsafe { ::core::ptr::read(cell_inner) }
}
// GUARANTEES:
// The first guarantee of `view_value` is exactly what `post_remove` guarantees.
unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
// SAFETY: This specific implementation of `view_value` allows the caller to
// promise the safety requirements of `post_remove` instead of the safety
// requirements for `view_value`.
unsafe { <Self as $crate::list::ListItem<$num>>::view_value(me) }
}
}
)*};
}
pub use impl_list_item;