linux-kernel - Re: [PATCH v4] rust: lock: Export Guard::do

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: <c1ff48ea-53ca-40ea-9541-85abd1a528d0@redhat.com>
Date: Thu, 30 Oct 2025 11:43:30 +0100
From: Paolo Bonzini <pbonzini@...hat.com>
To: Lyude Paul <lyude@...hat.com>, linux-kernel@...r.kernel.org,
 rust-for-linux@...r.kernel.org, Benno Lossin <lossin@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Will Deacon <will@...nel.org>, Boqun Feng <boqun.feng@...il.com>,
 Waiman Long <longman@...hat.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>,
 Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
 Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>
Subject: Re: [PATCH v4] rust: lock: Export Guard::do_unlocked()

On 10/29/25 19:35, Lyude Paul wrote:
> +    /// // Since we hold work.lock, which work will also try to acquire in WorkItem::run. Dropping
> +    /// // the lock temporarily while we wait for completion works around this.
> +    /// g.do_unlocked(|| work.done.wait_for_completion());
> +    ///
> +    /// assert_eq!(*g, 42);
> +    /// ```
> +    pub fn do_unlocked<U>(&mut self, cb: impl FnOnce() -> U) -> U {
>           // SAFETY: The caller owns the lock, so it is safe to unlock it.
>           unsafe { B::unlock(self.lock.state.get(), &self.state) };

Getting self as &mut is incorrect.  That's because owning a lock guard 
implicitly tells you that no other thread can observe the intermediate 
states of the object.  (The same is even more obviously true for a 
RefCell's mutable borrow, i.e. core::cell::RefMut)

Let's say you have a lock-protected data structure with an invariant 
that is preserved at the end of every critical section.  Let's say also 
that you have a function

     fn do_something() {
         let g = self.inner.lock();
         g.mess_up_the_invariant();          // (1)
         self.do_something_else(&mut g);     // uses do_unlocked()
         g.fix_the_invariant();              // (2)
     }

Because the function holds a guard between the calls (1) and (2), it 
expects that other thread cannot observe the temporary state.  The fact 
that do_unlocked() takes a &mut doesn't help, because the common case 
for RAII objects is that they're passed around mutably.

Instead, do_unlocked should take the guard and return another one:

     fn do_something() {
         let mut g = self.inner.lock();
         g.mess_up_the_invariant();          // (1)
         g = self.do_something_else(g);      // uses do_unlocked()
         g.fix_the_invariant();              // (2)
     }

This version of the interface makes it clear that (1) and (2) are in a 
separate critical section.  Unfortunately it makes the signature uglier 
for do_unlocked() itself:

     #[must_use]
     pub fn do_unlocked<U>(self, cb: impl FnOnce() -> U) -> (Self, U)

Paolo