linux-kernel - Re: [PATCH v3 1/4] rust: implement `kernel::sync::Refcount`

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: <87h64o6axp.fsf@kloenk.dev>
Date: Thu, 20 Feb 2025 13:46:26 +0100
From: Fiona Behrens <me@...enk.dev>
To: Gary Guo <gary@...yguo.net>
Cc: Miguel Ojeda <ojeda@...nel.org>,  Alex Gaynor <alex.gaynor@...il.com>,
  Boqun Feng <boqun.feng@...il.com>,  Björn Roy Baron
 <bjorn3_gh@...tonmail.com>,  Benno Lossin <benno.lossin@...ton.me>,
  Andreas Hindborg <a.hindborg@...nel.org>,  Alice Ryhl
 <aliceryhl@...gle.com>,  Trevor Gross <tmgross@...ch.edu>,  Danilo
 Krummrich <dakr@...nel.org>,  Will Deacon <will@...nel.org>,  Peter
 Zijlstra <peterz@...radead.org>,  Mark Rutland <mark.rutland@....com>,
  Tamir Duberstein <tamird@...il.com>,  Lyude Paul <lyude@...hat.com>,
  Wedson Almeida Filho <wedsonaf@...il.com>,
  rust-for-linux@...r.kernel.org,  linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/4] rust: implement `kernel::sync::Refcount`

Gary Guo <gary@...yguo.net> writes:

> This is a wrapping layer of `include/linux/refcount.h`. Currently the
> kernel refcount has already been used in `Arc`, however it calls into
> FFI directly.
>
> Reviewed-by: Alice Ryhl <aliceryhl@...gle.com>
> Reviewed-by: Boqun Feng <boqun.feng@...il.com>
> Signed-off-by: Gary Guo <gary@...yguo.net>

With the doc link fixed below

Reviewed-by: Fiona Behrens <me@...enk.dev>

> ---
[snip]
> diff --git a/rust/kernel/sync/refcount.rs b/rust/kernel/sync/refcount.rs
> new file mode 100644
> index 000000000000..a6a683f5d7b8
> --- /dev/null
> +++ b/rust/kernel/sync/refcount.rs
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Atomic reference counting.
> +//!
> +//! C header: [`include/linux/refcount.h`](srctree/include/linux/refcount.h)
> +
> +use crate::types::Opaque;
> +
> +/// Atomic reference counter.
> +///
> +/// This type is conceptually an atomic integer, but provides saturation semantics compared to
> +/// normal atomic integers. Values in the negative range when viewed as a signed integer are
> +/// saturation (bad) values. For details about the saturation semantics, please refer to top of
> +/// [`include/linux/refcount.h`](srctree/include/refcount.h).

Link is not pointing to the linux directory as in the text of the link.

Thanks,
Fiona

> +///
> +/// Wraps the kernel's C `refcount_t`.
> +#[repr(transparent)]
> +pub struct Refcount(Opaque<bindings::refcount_t>);
> +
> +impl Refcount {
> +    /// Construct a new [`Refcount`] from an initial value.
> +    #[inline]
> +    pub fn new(value: i32) -> Self {
> +        // SAFETY: There are no safety requirements for this FFI call.
> +        Self(Opaque::new(unsafe { bindings::REFCOUNT_INIT(value) }))
> +    }
> +
> +    #[inline]
> +    fn as_ptr(&self) -> *mut bindings::refcount_t {
> +        self.0.get()
> +    }
> +
> +    /// Set a refcount's value.
> +    #[inline]
> +    pub fn set(&self, value: i32) {
> +        // SAFETY: `self.as_ptr()` is valid.
> +        unsafe { bindings::refcount_set(self.as_ptr(), value) }
> +    }
> +
> +    /// Increment a refcount.
> +    ///
> +    /// It will saturate if overflows and `WARN`. It will also `WARN` if the refcount is 0, as this
> +    /// represents a possible use-after-free condition.
> +    ///
> +    /// Provides no memory ordering, it is assumed that caller already has a reference on the
> +    /// object.
> +    #[inline]
> +    pub fn inc(&self) {
> +        // SAFETY: self is valid.
> +        unsafe { bindings::refcount_inc(self.as_ptr()) }
> +    }
> +
> +    /// Decrement a refcount.
> +    ///
> +    /// It will `WARN` on underflow and fail to decrement when saturated.
> +    ///
> +    /// Provides release memory ordering, such that prior loads and stores are done
> +    /// before.
> +    #[inline]
> +    pub fn dec(&self) {
> +        // SAFETY: `self.as_ptr()` is valid.
> +        unsafe { bindings::refcount_dec(self.as_ptr()) }
> +    }
> +
> +    /// Decrement a refcount and test if it is 0.
> +    ///
> +    /// It will `WARN` on underflow and fail to decrement when saturated.
> +    ///
> +    /// Provides release memory ordering, such that prior loads and stores are done
> +    /// before, and provides an acquire ordering on success such that memory deallocation
> +    /// must come after.
> +    ///
> +    /// Returns true if the resulting refcount is 0, false otherwise.
> +    #[inline]
> +    #[must_use = "use `dec` instead you do not need to test if it is 0"]
> +    pub fn dec_and_test(&self) -> bool {
> +        // SAFETY: `self.as_ptr()` is valid.
> +        unsafe { bindings::refcount_dec_and_test(self.as_ptr()) }
> +    }
> +}
> +
> +// SAFETY: `refcount_t` is thread-safe.
> +unsafe impl Send for Refcount {}
> +
> +// SAFETY: `refcount_t` is thread-safe.
> +unsafe impl Sync for Refcount {}