linux-kernel - Re: [PATCH v2 3/3] rust: clist: Add typed iteration with FromListHead trait

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <04b9f849-94fc-4bfd-94d7-7337a8cffdf3@nvidia.com>
Date: Tue, 25 Nov 2025 18:29:28 -0500
From: Joel Fernandes <joelagnelf@...dia.com>
To: Alexandre Courbot <acourbot@...dia.com>, linux-kernel@...r.kernel.org,
 rust-for-linux@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 dakr@...nel.org, airlied@...il.com
Cc: apopple@...dia.com, ojeda@...nel.org, alex.gaynor@...il.com,
 boqun.feng@...il.com, gary@...yguo.net, bjorn3_gh@...tonmail.com,
 lossin@...nel.org, a.hindborg@...nel.org, aliceryhl@...gle.com,
 tmgross@...ch.edu, simona@...ll.ch, maarten.lankhorst@...ux.intel.com,
 mripard@...nel.org, tzimmermann@...e.de, jhubbard@...dia.com,
 ttabi@...dia.com, joel@...lfernandes.org, elle@...thered-steel.dev,
 daniel.almeida@...labora.com, arighi@...dia.com, phasta@...nel.org,
 nouveau@...ts.freedesktop.org,
 Nouveau <nouveau-bounces@...ts.freedesktop.org>
Subject: Re: [PATCH v2 3/3] rust: clist: Add typed iteration with FromListHead
 trait

Hi Alex,

On 11/24/2025 2:01 AM, Alexandre Courbot wrote:
>>  ///
>>  /// # Invariants
>>  ///
>> @@ -69,6 +156,15 @@ pub fn iter_heads(&self) -> ClistHeadIter<'_> {
>>              head: &self.0,
>>          }
>>      }
>> +
>> +    /// Create a high-level iterator over typed items.
>> +    #[inline]
>> +    pub fn iter<L: ClistLink>(&self) -> ClistIter<'_, L> {
>> +        ClistIter {
>> +            head_iter: self.iter_heads(),
>> +            _phantom: PhantomData,
>> +        }
>> +    }
> This looks very dangerous, as it gives any caller the freedom to specify
> the type they want to upcast the `Clist` to, without using unsafe code.
> One could easily invoke this with the wrong type and get no build error
> or warning whatsoever.
> 
> A safer version would have the `Clist` generic over the kind of
> conversion that needs to be performed, using e.g. a closure:
> 
>   pub struct Clist<'a, T, C: Fn(*mut bindings::list_head) -> *mut T> {
>       head: &'a ClistHead,
>       conv: C,
>   }
> 
> `from_raw` would also take the closure as argument, which forces the
> creator of the list to both specify what that list is for, and use an
> `unsafe` statement for unsafe code. Here is a dummy example:
> 
>     let head: bindings::list_head = ...;
> 
>     // SAFETY: list_head always corresponds to the `list` member of
>     // `type_embedding_list_head`.
>     let conv = |head: *mut bindings::list_head| unsafe {
>         crate::container_of!(head, type_embedding_list_head, list)
>     };
> 
>     // SAFETY: ...
>     unsafe { Clist::from_raw(head, conv) }
> 
> Then `conv` would be passed down to the `ClistIter` so it can return
> references to the correct type.
> 
> By doing so you can remove the `ClinkList` and `FromListHead` traits,
> the `impl_from_list_head` and `clist_iterate` macros, as well as the
> hidden ad-hoc types these create. And importantly, all unsafe code must
> be explicitly specified in an `unsafe` block, nothing is hidden by
> macros.
> 
> This approach works better imho because each `list_head` is unique in
> how it has to be iterated: there is no benefit in implementing things
> using types and traits that will only ever be used in a single place
> anyway. And if there was, we could always create a newtype for that.

I agree with your safety concerns, indeed it is possible without any safety
comments to build iterators yielding objects of random type. I think the conv
function is a good idea and with the addition of unsafe blocks within the conv.

One thing I am concerned is with the user interface. I would like to keep the
user interface as simple as possible. I am hoping that with implementing your
idea here on this with the closure, we can still keep it simple, perhaps getting
the assistance of macros. I will give it a try.

> Also as I suspected in v1 `Clist` appears to do very little apart from
> providing an iterator, so I'm more convinced that the front type for
> this should be `ClistHead`.

This part I don't agree with. I prefer to keep it as `Clist` which wraps a
sentinel list head. A random `ClistHead` is not necessarily a sentinel.

thanks,

 - Joel