linux-kernel - Re: [PATCH v3 2/2] rust: lockdep: Use Pin for all LockClassKey usages

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: <7bc31b4e-cfe2-40ff-9a00-a73bff64df1c@proton.me>
Date: Fri, 07 Feb 2025 00:22:19 +0000
From: Benno Lossin <benno.lossin@...ton.me>
To: Boqun Feng <boqun.feng@...il.com>
Cc: Mitchell Levy <levymitchell0@...il.com>, Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, Gary Guo <gary@...yguo.net>, Björn Roy Baron <bjorn3_gh@...tonmail.com>, Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, Andreas Hindborg <a.hindborg@...nel.org>, linux-block@...r.kernel.org, rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] rust: lockdep: Use Pin for all LockClassKey usages

On 06.02.25 22:27, Boqun Feng wrote:
> On Wed, Feb 05, 2025 at 08:30:50PM +0000, Benno Lossin wrote:
>> On 05.02.25 20:59, Mitchell Levy wrote:
>>> diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
>>> index 41dcddac69e2..119e5f569bdb 100644
>>> --- a/rust/kernel/sync/lock.rs
>>> +++ b/rust/kernel/sync/lock.rs
>>> @@ -7,12 +7,9 @@
>>>
>>>  use super::LockClassKey;
>>>  use crate::{
>>> -    init::PinInit,
>>> -    pin_init,
>>> -    str::CStr,
>>> -    types::{NotThreadSafe, Opaque, ScopeGuard},
>>> +    init::PinInit, pin_init, str::CStr, types::NotThreadSafe, types::Opaque, types::ScopeGuard,
>>>  };
>>> -use core::{cell::UnsafeCell, marker::PhantomPinned};
>>> +use core::{cell::UnsafeCell, marker::PhantomPinned, pin::Pin};
>>>  use macros::pin_data;
>>>
>>>  pub mod mutex;
>>> @@ -121,7 +118,7 @@ unsafe impl<T: ?Sized + Send, B: Backend> Sync for Lock<T, B> {}
>>>
>>>  impl<T, B: Backend> Lock<T, B> {
>>>      /// Constructs a new lock initialiser.
>>> -    pub fn new(t: T, name: &'static CStr, key: &'static LockClassKey) -> impl PinInit<Self> {
>>> +    pub fn new(t: T, name: &'static CStr, key: Pin<&'static LockClassKey>) -> impl PinInit<Self> {
>>
>> Static references do not need `Pin`, since `Pin::static_ref` [1] exists,
>> so you can just as well not add the `Pin` here and the other places
>> where you have `Pin<&'static T>`.
>>
> 
> You're right about `Pin` not needed for 'static. However, the
> `Pin<&'static LockClassKey>` signature is the intermediate state, and
> eventually we will need to support initializing a lock (and others) with
> a dynamically allocated `LockClassKey`, and when that is available, the
> type of `key` will become `Pin<&'a LockClassKey>`, so `Pin` is needed.
> 
> So I would like to keep this patch as it is. Works for you?

Makes sense and fine by me. It would be nice if it was also mentioned in
the commit message. Thanks!

---
Cheers,
Benno

> 
> Regards,
> Boqun
> 
>> The reasoning is that since the data lives for `'static` at that
>> location, it will never move (since it is borrowed for `'static` after
>> all).
>>
>> [1]: https://doc.rust-lang.org/std/pin/struct.Pin.html#method.static_ref
>>
>> ---
>> Cheers,
>> Benno
>>
>>>          pin_init!(Self {
>>>              data: UnsafeCell::new(t),
>>>              _pin: PhantomPinned,
>>> diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
>>> index 480ee724e3cc..d65f94b5caf2 100644
>>> --- a/rust/kernel/sync/lock/global.rs
>>> +++ b/rust/kernel/sync/lock/global.rs
>>> @@ -13,6 +13,7 @@
>>>  use core::{
>>>      cell::UnsafeCell,
>>>      marker::{PhantomData, PhantomPinned},
>>> +    pin::Pin,
>>>  };
>>>
>>>  /// Trait implemented for marker types for global locks.
>>
>>