[PATCH 0/2] rust: list: fix incomplete SAFETY comments in list implementation

[PATCH 0/2] rust: list: fix incomplete SAFETY comments in list implementation

[Christian Benton]

Four SAFETY comments in the linked list implementation were left as
TODO. This series fills them in with proper safety proofs explaining
why each unsafe operation is sound.

Christian Benton (2):
  rust: list: fix SAFETY comment in List::remove
  rust: list: fix SAFETY comments in impl_list_item_mod

 rust/kernel/list.rs                    |  4 +++-
 rust/kernel/list/impl_list_item_mod.rs | 17 ++++++++++++++---
 2 files changed, 17 insertions(+), 4 deletions(-)

-- 
2.53.0

[PATCH 1/2] rust: list: fix SAFETY comment in List::remove

[Christian Benton]

The SAFETY comment for the call to ListLinks::fields in List::remove
was left as TODO. Fill it in: the call is safe because T::view_links
returns a reference to the ListLinks field of item, and references are
always valid and non-dangling.

Signed-off-by: Christian Benton <t1bur0n.kernel.org@protonmail.ch>
---
 rust/kernel/list.rs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index 406e3a028..b5705af43 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -599,7 +599,9 @@ pub fn pop_front(&mut self) -> Option<ListArc<T, ID>> {
     ///
     /// `item` must not be in a different linked list (with the same id).
     pub unsafe fn remove(&mut self, item: &T) -> Option<ListArc<T, ID>> {
-        // SAFETY: TODO.
+        // SAFETY: `T::view_links` returns a reference to the `ListLinks` field of `item`.
+        // References are always valid and non-dangling, so converting to a raw pointer
+        // via `ListLinks::fields` is sound.
         let mut item = unsafe { ListLinks::fields(T::view_links(item)) };
         // SAFETY: The user provided a reference, and reference are never dangling.
         //
-- 
2.53.0

[PATCH 2/2] rust: list: fix SAFETY comments in impl_list_item_mod

[Christian Benton]

Three SAFETY comments were left as TODO in impl_list_item_mod.rs.
Fill them in:

- impl_has_list_links_self_ptr!: the HasListLinks impl is safe because
  raw_get_list_links only compiles if the field has type ListLinksSelfPtr,
  which the type system enforces statically.

- prepare_to_insert: the container_of! call is safe because links_field
  is valid from view_links and Self: HasSelfPtr guarantees links_field
  points to the inner field of a ListLinksSelfPtr.

- view_value: the container_of! call is safe because the caller of
  prepare_to_insert promised to retain the ListArc, and Self: HasSelfPtr
  guarantees links_field points to the inner field of a ListLinksSelfPtr.

Signed-off-by: Christian Benton <t1bur0n.kernel.org@protonmail.ch>
---
 rust/kernel/list/impl_list_item_mod.rs | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index 5a3eac9f3..2d1f4723a 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -86,7 +86,11 @@ macro_rules! impl_has_list_links_self_ptr {
         // right type.
         unsafe impl$(<$($generics)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
 
-        // SAFETY: TODO.
+        // SAFETY: The implementation of `raw_get_list_links` returns a pointer to the
+        // `ListLinks` field inside `ListLinksSelfPtr`. This cast is valid because
+        // `ListLinksSelfPtr` is a wrapper around `ListLinks` and shares the same memory
+        // layout. The macro only compiles if the field has type `ListLinksSelfPtr`, which
+        // the type system enforces statically.
         unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
             #[inline]
             unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
@@ -274,7 +278,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) };
 
-                // SAFETY: TODO.
+                // SAFETY: `links_field` is valid because `view_links` returned it from a valid
+                // `me` pointer as promised by the caller. `links_field` points to the `inner`
+                // field of a `ListLinksSelfPtr` because `Self: HasSelfPtr` guarantees that the
+                // `ListLinks` field is always inside a `ListLinksSelfPtr`.
                 let container = unsafe {
                     $crate::container_of!(
                         links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
@@ -326,7 +333,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 {
-                // SAFETY: TODO.
+                // SAFETY: `links_field` is valid and points to a live value because the caller
+                // of `prepare_to_insert` promised to retain ownership of the `ListArc`, and the
+                // value cannot be destroyed while a `ListArc` exists. `links_field` points to
+                // the `inner` field of a `ListLinksSelfPtr` because `Self: HasSelfPtr`
+                // guarantees this.
                 let container = unsafe {
                     $crate::container_of!(
                         links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
-- 
2.53.0

Re: [PATCH 1/2] rust: list: fix SAFETY comment in List::remove

[Alice Ryhl]

On Fri, Apr 03, 2026 at 10:08:15PM +0000, Christian Benton wrote:
> The SAFETY comment for the call to ListLinks::fields in List::remove
> was left as TODO. Fill it in: the call is safe because T::view_links
> returns a reference to the ListLinks field of item, and references are
> always valid and non-dangling.
> 
> Signed-off-by: Christian Benton <t1bur0n.kernel.org@protonmail.ch>

Thanks. I agree that `item` being a reference is the only thing needed
for this to be sound, as reference implies that the pointer is not
dangling.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 2/2] rust: list: fix SAFETY comments in impl_list_item_mod

[Alice Ryhl]

On Fri, Apr 03, 2026 at 10:08:24PM +0000, Christian Benton wrote:
> Three SAFETY comments were left as TODO in impl_list_item_mod.rs.
> Fill them in:
> 
> - impl_has_list_links_self_ptr!: the HasListLinks impl is safe because
>   raw_get_list_links only compiles if the field has type ListLinksSelfPtr,
>   which the type system enforces statically.
> 
> - prepare_to_insert: the container_of! call is safe because links_field
>   is valid from view_links and Self: HasSelfPtr guarantees links_field
>   points to the inner field of a ListLinksSelfPtr.
> 
> - view_value: the container_of! call is safe because the caller of
>   prepare_to_insert promised to retain the ListArc, and Self: HasSelfPtr
>   guarantees links_field points to the inner field of a ListLinksSelfPtr.
> 
> Signed-off-by: Christian Benton <t1bur0n.kernel.org@protonmail.ch>
> ---
>  rust/kernel/list/impl_list_item_mod.rs | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
> index 5a3eac9f3..2d1f4723a 100644
> --- a/rust/kernel/list/impl_list_item_mod.rs
> +++ b/rust/kernel/list/impl_list_item_mod.rs
> @@ -86,7 +86,11 @@ macro_rules! impl_has_list_links_self_ptr {
>          // right type.
>          unsafe impl$(<$($generics)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {}
>  
> -        // SAFETY: TODO.
> +        // SAFETY: The implementation of `raw_get_list_links` returns a pointer to the
> +        // `ListLinks` field inside `ListLinksSelfPtr`. This cast is valid because
> +        // `ListLinksSelfPtr` is a wrapper around `ListLinks` and shares the same memory
> +        // layout. The macro only compiles if the field has type `ListLinksSelfPtr`, which
> +        // the type system enforces statically.

No, `ListLinksSelfPtr` doesn't share the same memory layout as
`ListLinks`. It has an extra field. The cast is okay because of the
repr(C) annotation.

>          unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self {
>              #[inline]
>              unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
> @@ -274,7 +278,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) };
>  
> -                // SAFETY: TODO.
> +                // SAFETY: `links_field` is valid because `view_links` returned it from a valid
> +                // `me` pointer as promised by the caller. `links_field` points to the `inner`
> +                // field of a `ListLinksSelfPtr` because `Self: HasSelfPtr` guarantees that the
> +                // `ListLinks` field is always inside a `ListLinksSelfPtr`.
>                  let container = unsafe {
>                      $crate::container_of!(
>                          links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
> @@ -326,7 +333,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 {
> -                // SAFETY: TODO.
> +                // SAFETY: `links_field` is valid and points to a live value because the caller
> +                // of `prepare_to_insert` promised to retain ownership of the `ListArc`, and the
> +                // value cannot be destroyed while a `ListArc` exists. `links_field` points to
> +                // the `inner` field of a `ListLinksSelfPtr` because `Self: HasSelfPtr`
> +                // guarantees this.
>                  let container = unsafe {
>                      $crate::container_of!(
>                          links_field, $crate::list::ListLinksSelfPtr<Self, $num>, inner
> -- 
> 2.53.0
> 
> 

Re: [PATCH 1/2] rust: list: fix SAFETY comment in List::remove

[Gary Guo]

On Tue Apr 7, 2026 at 9:15 AM BST, Alice Ryhl wrote:
> On Fri, Apr 03, 2026 at 10:08:15PM +0000, Christian Benton wrote:
>> The SAFETY comment for the call to ListLinks::fields in List::remove
>> was left as TODO. Fill it in: the call is safe because T::view_links
>> returns a reference to the ListLinks field of item, and references are
>> always valid and non-dangling.
>> 
>> Signed-off-by: Christian Benton <t1bur0n.kernel.org@protonmail.ch>
>
> Thanks. I agree that `item` being a reference is the only thing needed
> for this to be sound, as reference implies that the pointer is not
> dangling.

(cc Philipp)

FWIW, I want to change `item` from a reference to a raw pointer. In Philipp's
WIP DRM job scheduler implementation, there is a case where the list is
conceptually a list of `UniqueArc`s; each job only needs to handle that is
sufficient to locate the item in the list and remove it.

I suggested to him that keeping a pointer and do a list iter with ptr
comparison is sufficient to achieve that without needing to do additional
reference counting. Of course I don't want to iterate the list when I could just
call `remove` function on the list itself; for that use case I want to change
the `remove` function to only require a pointer (with additional safety
requirement that it is valid).

It's orthogonal to this change as I'll probably be more careful about pointer
provenance too when making that change, so just a heads up.

Best,
Gary

Re: [PATCH 1/2] rust: list: fix SAFETY comment in List::remove

[Philipp Stanner]

On Tue, 2026-04-07 at 12:56 +0100, Gary Guo wrote:
> On Tue Apr 7, 2026 at 9:15 AM BST, Alice Ryhl wrote:
> > On Fri, Apr 03, 2026 at 10:08:15PM +0000, Christian Benton wrote:
> > > The SAFETY comment for the call to ListLinks::fields in List::remove
> > > was left as TODO. Fill it in: the call is safe because T::view_links
> > > returns a reference to the ListLinks field of item, and references are
> > > always valid and non-dangling.
> > > 
> > > Signed-off-by: Christian Benton <t1bur0n.kernel.org@protonmail.ch>
> > 
> > Thanks. I agree that `item` being a reference is the only thing needed
> > for this to be sound, as reference implies that the pointer is not
> > dangling.
> 
> (cc Philipp)
> 
> FWIW, I want to change `item` from a reference to a raw pointer.
> 

Are you still working on that, or should I or someone else pick it up?

Regards
P.


>  In Philipp's
> WIP DRM job scheduler implementation, there is a case where the list is
> conceptually a list of `UniqueArc`s; each job only needs to handle that is
> sufficient to locate the item in the list and remove it.
> 
> I suggested to him that keeping a pointer and do a list iter with ptr
> comparison is sufficient to achieve that without needing to do additional
> reference counting. Of course I don't want to iterate the list when I could just
> call `remove` function on the list itself; for that use case I want to change
> the `remove` function to only require a pointer (with additional safety
> requirement that it is valid).
> 
> It's orthogonal to this change as I'll probably be more careful about pointer
> provenance too when making that change, so just a heads up.
> 
> Best,
> Gary

Re: [PATCH 1/2] rust: list: fix SAFETY comment in List::remove

[Christian Benton]

Hello Phillip,

I am interested in taking this on will do so. I had reached out to Gary about his email and asked if he would like me to. I wasn't sure if his email implied he was to make the change or was mentioning the idea to me for me to do it. I am, however, more than happy to. 

Sorry for any confusion on my end.

Christian Benton



Sent with Proton Mail secure email.

On Friday, April 24th, 2026 at 7:29 AM, Philipp Stanner <phasta@mailbox.org> wrote:

> On Tue, 2026-04-07 at 12:56 +0100, Gary Guo wrote:
> > On Tue Apr 7, 2026 at 9:15 AM BST, Alice Ryhl wrote:
> > > On Fri, Apr 03, 2026 at 10:08:15PM +0000, Christian Benton wrote:
> > > > The SAFETY comment for the call to ListLinks::fields in List::remove
> > > > was left as TODO. Fill it in: the call is safe because T::view_links
> > > > returns a reference to the ListLinks field of item, and references are
> > > > always valid and non-dangling.
> > > >
> > > > Signed-off-by: Christian Benton <t1bur0n.kernel.org@protonmail.ch>
> > >
> > > Thanks. I agree that `item` being a reference is the only thing needed
> > > for this to be sound, as reference implies that the pointer is not
> > > dangling.
> >
> > (cc Philipp)
> >
> > FWIW, I want to change `item` from a reference to a raw pointer.
> >
> 
> Are you still working on that, or should I or someone else pick it up?
> 
> Regards
> P.
> 
> 
> >  In Philipp's
> > WIP DRM job scheduler implementation, there is a case where the list is
> > conceptually a list of `UniqueArc`s; each job only needs to handle that is
> > sufficient to locate the item in the list and remove it.
> >
> > I suggested to him that keeping a pointer and do a list iter with ptr
> > comparison is sufficient to achieve that without needing to do additional
> > reference counting. Of course I don't want to iterate the list when I could just
> > call `remove` function on the list itself; for that use case I want to change
> > the `remove` function to only require a pointer (with additional safety
> > requirement that it is valid).
> >
> > It's orthogonal to this change as I'll probably be more careful about pointer
> > provenance too when making that change, so just a heads up.
> >
> > Best,
> > Gary
> 
>

Re: [PATCH 1/2] rust: list: fix SAFETY comment in List::remove

[Gary Guo]

On Mon Apr 27, 2026 at 5:53 PM BST, Christian Benton wrote:
> Hello Phillip,
>
> I am interested in taking this on will do so. I had reached out to Gary about
> his email and asked if he would like me to. I wasn't sure if his email implied
> he was to make the change or was mentioning the idea to me for me to do it. I
> am, however, more than happy to. 

Feel free to pick it up if you want.

Also please wrap your email and use bottom posting
(https://subspace.kernel.org/etiquette.html)

Best,
Gary

>
> Sorry for any confusion on my end.
>
> Christian Benton
>
>
>
> Sent with Proton Mail secure email.
>
> On Friday, April 24th, 2026 at 7:29 AM, Philipp Stanner <phasta@mailbox.org> wrote:
>
>> On Tue, 2026-04-07 at 12:56 +0100, Gary Guo wrote:
>> > On Tue Apr 7, 2026 at 9:15 AM BST, Alice Ryhl wrote:
>> > > On Fri, Apr 03, 2026 at 10:08:15PM +0000, Christian Benton wrote:
>> > > > The SAFETY comment for the call to ListLinks::fields in List::remove
>> > > > was left as TODO. Fill it in: the call is safe because T::view_links
>> > > > returns a reference to the ListLinks field of item, and references are
>> > > > always valid and non-dangling.
>> > > >
>> > > > Signed-off-by: Christian Benton <t1bur0n.kernel.org@protonmail.ch>
>> > >
>> > > Thanks. I agree that `item` being a reference is the only thing needed
>> > > for this to be sound, as reference implies that the pointer is not
>> > > dangling.
>> >
>> > (cc Philipp)
>> >
>> > FWIW, I want to change `item` from a reference to a raw pointer.
>> >
>> 
>> Are you still working on that, or should I or someone else pick it up?
>> 
>> Regards
>> P.
>> 

Re: [PATCH 1/2] rust: list: fix SAFETY comment in List::remove

[Alice Ryhl]

On Tue, Apr 07, 2026 at 12:56:52PM +0100, Gary Guo wrote:
> On Tue Apr 7, 2026 at 9:15 AM BST, Alice Ryhl wrote:
> > On Fri, Apr 03, 2026 at 10:08:15PM +0000, Christian Benton wrote:
> >> The SAFETY comment for the call to ListLinks::fields in List::remove
> >> was left as TODO. Fill it in: the call is safe because T::view_links
> >> returns a reference to the ListLinks field of item, and references are
> >> always valid and non-dangling.
> >> 
> >> Signed-off-by: Christian Benton <t1bur0n.kernel.org@protonmail.ch>
> >
> > Thanks. I agree that `item` being a reference is the only thing needed
> > for this to be sound, as reference implies that the pointer is not
> > dangling.
> 
> (cc Philipp)
> 
> FWIW, I want to change `item` from a reference to a raw pointer. In Philipp's
> WIP DRM job scheduler implementation, there is a case where the list is
> conceptually a list of `UniqueArc`s; each job only needs to handle that is
> sufficient to locate the item in the list and remove it.
> 
> I suggested to him that keeping a pointer and do a list iter with ptr
> comparison is sufficient to achieve that without needing to do additional
> reference counting. Of course I don't want to iterate the list when I could just
> call `remove` function on the list itself; for that use case I want to change
> the `remove` function to only require a pointer (with additional safety
> requirement that it is valid).
> 
> It's orthogonal to this change as I'll probably be more careful about pointer
> provenance too when making that change, so just a heads up.

Could you explain why? Is there any scenario where the caller couldn't
just create a reference to the value from their raw pointer?

Alice