linux-kernel - Re: [PATCH] rust: sync: document that Guard is not a stable lock guard

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: <Z1HuNaDAiKV7L9ea@boqun-archlinux>
Date: Thu, 5 Dec 2024 10:17:25 -0800
From: Boqun Feng <boqun.feng@...il.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
	Will Deacon <will@...nel.org>, 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>,
	Benno Lossin <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Trevor Gross <tmgross@...ch.edu>, rust-for-linux@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rust: sync: document that Guard is not a stable lock
 guard

On Thu, Dec 05, 2024 at 12:35:51PM +0000, Alice Ryhl wrote:
> Most locks in the linux kernel are stable, which means that holding the
> lock is sufficient to keep the value from being freed. For example, this
> means that if you acquire a lock on a refcounted value during rcu, then
> you do not need to acquire a refcount to keep it alive past
> rcu_read_unlock().
> 
> However, the Rust `Guard` type is written in a way where it cannot be
> used with this pattern. One reason for this is the existence of the
> `do_unlocked` method that is used with `Condvar`. The method allows you
> to unlock the lock, run some code, and then reacquire the lock. This
> operation is not okay if the lock itself is what keeps the value alive,
> as it could be freed right after the unlock call.
> 

Hmm... but `Guard` holds a reference to the corresponding `Lock`. How
could this happen? Do you have an example?

Regards,
Boqun

> If we want to support stable locks, we'll need a different guard type
> that does not have a `do_unlocked` operation.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
> ---
>  rust/kernel/sync/lock.rs | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
> index 41dcddac69e2..7eab46d4060a 100644
> --- a/rust/kernel/sync/lock.rs
> +++ b/rust/kernel/sync/lock.rs
> @@ -159,6 +159,17 @@ pub fn try_lock(&self) -> Option<Guard<'_, T, B>> {
>  /// Allows mutual exclusion primitives that implement the [`Backend`] trait to automatically unlock
>  /// when a guard goes out of scope. It also provides a safe and convenient way to access the data
>  /// protected by the lock.
> +///
> +/// This guard may be released and reacquired with [`do_unlocked`]. Note that this implies that
> +/// this `Guard` type is _not_ stable, that is, holding this lock is not sufficient to keep the
> +/// underlying [`Lock`] alive. That must be done by some other mechanism such as a refcount or
> +/// ownership.
> +///
> +/// # Invariants
> +///
> +/// This `Guard` owns the lock as defined by the [`Backend`] trait.
> +///
> +/// [`do_unlocked`]: Guard::do_unlocked
>  #[must_use = "the lock unlocks immediately when the guard is unused"]
>  pub struct Guard<'a, T: ?Sized, B: Backend> {
>      pub(crate) lock: &'a Lock<T, B>,
> 
> ---
> base-commit: 40384c840ea1944d7c5a392e8975ed088ecf0b37
> change-id: 20241205-guard-stable-doc-efad6812d0cb
> 
> Best regards,
> -- 
> Alice Ryhl <aliceryhl@...gle.com>
>