linux-kernel - Re: [PATCH] rust: add global lock support

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: <CAH5fLgjtSVMcKL4aMrNT=zJPGvzAPJt3qAUMYdB-+3=s80FNKQ@mail.gmail.com>
Date: Mon, 26 Aug 2024 17:31:08 +0200
From: Alice Ryhl <aliceryhl@...gle.com>
To: Benno Lossin <benno.lossin@...ton.me>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, 
	Wedson Almeida Filho <wedsonaf@...il.com>, Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Andreas Hindborg <a.hindborg@...sung.com>, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rust: add global lock support

On Mon, Aug 26, 2024 at 5:30 PM Benno Lossin <benno.lossin@...ton.me> wrote:
>
> On 26.08.24 17:27, Alice Ryhl wrote:
> > We don't currently have any support for global locks in Rust, however
> > they are very useful and I have needed to work around this limitation
> > several times. My workarounds generally involve initializing the mutex
> > in the module's init function, and this workaround is reflected here.
> >
> > Due to the initialization requirement, constructing a global mutex is
> > unsafe with the current approach. In the future, it would be really nice
> > to support global mutexes that don't need to be initialized, which would
> > make them safe. Unfortunately, this is not possible today because
> > bindgen refuses to expose __ARCH_SPIN_LOCK_UNLOCKED to Rust as a
> > compile-time constant. It just generates an `extern "C"` global
> > reference instead.
> >
> > On most architectures, we could initialize the lock to just contain all
> > zeros. A possible improvement would be to create a Kconfig constant
> > that is set whenever the current architecture uses all zeros for the
> > initializer and have `unsafe_const_init` be a no-op on those
> > architectures. We could also provide a safe const initializer that is
> > only available when that Kconfig option is set.
> >
> > For architectures that don't use all-zeros for the unlocked case, we
> > will most likely have to hard-code the correct representation on the
> > Rust side.
> >
> > Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
> > ---
> >  rust/kernel/sync/lock.rs | 34 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> >
> > diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
> > index f6c34ca4d819..19e8ecd8d194 100644
> > --- a/rust/kernel/sync/lock.rs
> > +++ b/rust/kernel/sync/lock.rs
> > @@ -117,6 +117,40 @@ pub fn new(t: T, name: &'static CStr, key: &'static LockClassKey) -> impl PinIni
> >              }),
> >          })
> >      }
> > +
> > +    /// Create a global lock that has not yet been initialized.
> > +    ///
> > +    /// Since global locks is not yet fully supported, this method implements global locks by
> > +    /// requiring you to initialize them before you start using it. Usually this is best done in
> > +    /// the module's init function.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// You must call [`unsafe_const_init`] before calling any other method on this lock.
> > +    ///
> > +    /// [`unsafe_const_init`]: Self::unsafe_const_init
> > +    pub const unsafe fn unsafe_const_new(t: T) -> Self {
> > +        Self {
> > +            data: UnsafeCell::new(t),
> > +            state: Opaque::uninit(),
> > +            _pin: PhantomPinned,
> > +        }
> > +    }
> > +
> > +    /// Initialize a global lock.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// * This lock must have been created with [`unsafe_const_new`].
> > +    /// * This lock must be pinned.
>
> You could also ask for `self: Pin<&Self>` and remove this constraint, or
> is that not possible in your use-case?

The value is going to be in a static, and it's inconvenient to have to
use Pin::new_unchecked when calling this initializer. Not sure much
value is gained.

Alice