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: <Z6UpNpefdxyvYBPe@boqun-archlinux>
Date: Thu, 6 Feb 2025 13:27:18 -0800
From: Boqun Feng <boqun.feng@...il.com>
To: Benno Lossin <benno.lossin@...ton.me>
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 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?

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.
> 
>