| 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
| ||
|
Message-ID: <CAH5fLgg7o7hs5B4mMzPd6RzYm+RcX8gw1Aw8voJqnmfnA_aM4Q@mail.gmail.com>
Date: Tue, 25 Feb 2025 10:55:17 +0100
From: Alice Ryhl <aliceryhl@...gle.com>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: Yury Norov <yury.norov@...il.com>, Rasmus Villemoes <linux@...musvillemoes.dk>,
Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
Boqun Feng <boqun.feng@...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>, linux-kernel@...r.kernel.org,
Danilo Krummrich <dakr@...hat.com>, rust-for-linux@...r.kernel.org,
Vincent Guittot <vincent.guittot@...aro.org>
Subject: Re: [PATCH 1/2] rust: Add initial cpumask abstractions
On Tue, Feb 25, 2025 at 10:47 AM Viresh Kumar <viresh.kumar@...aro.org> wrote:
>
> Add initial Rust abstractions for struct cpumask, covering a subset of
> its APIs. Additional APIs can be added as needed.
>
> These abstractions will be used in upcoming Rust support for cpufreq and
> OPP frameworks.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
> ---
> rust/kernel/cpumask.rs | 168 +++++++++++++++++++++++++++++++++++++++++
> rust/kernel/lib.rs | 1 +
> 2 files changed, 169 insertions(+)
> create mode 100644 rust/kernel/cpumask.rs
>
> diff --git a/rust/kernel/cpumask.rs b/rust/kernel/cpumask.rs
> new file mode 100644
> index 000000000000..13864424420b
> --- /dev/null
> +++ b/rust/kernel/cpumask.rs
> @@ -0,0 +1,168 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! CPU mask abstractions.
> +//!
> +//! C header: [`include/linux/cpumask.h`](srctree/include/linux/cpumask.h)
> +
> +use crate::{bindings, error::Result, prelude::ENOMEM};
> +
> +#[cfg(not(CONFIG_CPUMASK_OFFSTACK))]
> +use crate::prelude::{KBox, GFP_KERNEL};
> +
> +#[cfg(CONFIG_CPUMASK_OFFSTACK)]
> +use core::ptr;
> +
> +/// A simple implementation of `struct cpumask` from the C code.
> +pub struct Cpumask {
> + ptr: *mut bindings::cpumask,
> + owned: bool,
> +}
> +
> +impl Cpumask {
> + /// Creates cpumask.
> + #[cfg(CONFIG_CPUMASK_OFFSTACK)]
> + fn new_inner(empty: bool) -> Result<Self> {
> + let mut ptr: *mut bindings::cpumask = ptr::null_mut();
> +
> + // SAFETY: Depending on the value of `gfp_flags`, this call may sleep. Other than that, it
> + // is always safe to call this method.
> + if !unsafe {
> + if empty {
> + bindings::zalloc_cpumask_var(&mut ptr, bindings::GFP_KERNEL)
> + } else {
> + bindings::alloc_cpumask_var(&mut ptr, bindings::GFP_KERNEL)
> + }
> + } {
> + return Err(ENOMEM);
> + }
> +
> + Ok(Self { ptr, owned: true })
> + }
> +
> + /// Creates cpumask.
> + #[cfg(not(CONFIG_CPUMASK_OFFSTACK))]
> + fn new_inner(empty: bool) -> Result<Self> {
> + let ptr = KBox::into_raw(KBox::new([bindings::cpumask::default(); 1], GFP_KERNEL)?);
I don't really understand this CPUMASK_OFFSTACK logic. You seem to
always allocate memory, but if OFFSTACK=n, then shouldn't it be on the
stack ...?
> + // SAFETY: Depending on the value of `gfp_flags`, this call may sleep. Other than that, it
> + // is always safe to call this method.
> + if !unsafe {
> + if empty {
> + bindings::zalloc_cpumask_var(ptr, bindings::GFP_KERNEL)
> + } else {
> + bindings::alloc_cpumask_var(ptr, bindings::GFP_KERNEL)
> + }
> + } {
> + return Err(ENOMEM);
> + }
> +
> + Ok(Self {
> + ptr: ptr as *mut _,
> + owned: true,
> + })
> + }
> +
> + /// Creates empty cpumask.
> + pub fn new() -> Result<Self> {
> + Self::new_inner(true)
> + }
> +
> + /// Creates uninitialized cpumask.
> + fn new_uninit() -> Result<Self> {
> + Self::new_inner(false)
> + }
> +
> + /// Clones cpumask.
> + pub fn try_clone(&self) -> Result<Self> {
> + let mut cpumask = Self::new_uninit()?;
> +
> + self.copy(&mut cpumask);
> + Ok(cpumask)
> + }
> +
> + /// Creates a new abstraction instance of an existing `struct cpumask` pointer.
> + ///
> + /// # Safety
> + ///
> + /// Callers must ensure that `ptr` is valid, and non-null.
> + #[cfg(CONFIG_CPUMASK_OFFSTACK)]
> + pub unsafe fn get_cpumask(ptr: &mut *mut bindings::cpumask) -> Self {
> + Self {
> + ptr: *ptr,
> + owned: false,
Using an owned variable in this way isn't great, IMO. We should use
two different types such as Owned<CpuMask> and &CpuMask to represent
owned vs borrowed instead.
> + }
> + }
> +
> + /// Creates a new abstraction instance of an existing `struct cpumask` pointer.
> + ///
> + /// # Safety
> + ///
> + /// Callers must ensure that `ptr` is valid, and non-null.
> + #[cfg(not(CONFIG_CPUMASK_OFFSTACK))]
> + pub unsafe fn get_cpumask(ptr: &mut bindings::cpumask_var_t) -> Self {
> + Self {
> + ptr: ptr as *mut _,
> + owned: false,
> + }
> + }
> +
> + /// Obtain the raw `struct cpumask *`.
> + pub fn as_raw(&mut self) -> *mut bindings::cpumask {
> + self.ptr
> + }
> +
> + /// Sets CPU in the cpumask.
> + ///
> + /// Update the cpumask with a single CPU.
> + pub fn set(&mut self, cpu: u32) {
> + // SAFETY: `ptr` is guaranteed to be valid for the lifetime of `self`. And it is safe to
> + // call `cpumask_set_cpus()` for any CPU.
> + unsafe { bindings::cpumask_set_cpu(cpu, self.ptr) };
> + }
> +
> + /// Clears CPU in the cpumask.
> + ///
> + /// Update the cpumask with a single CPU.
> + pub fn clear(&mut self, cpu: i32) {
> + // SAFETY: `ptr` is guaranteed to be valid for the lifetime of `self`. And it is safe to
> + // call `cpumask_clear_cpu()` for any CPU.
> + unsafe { bindings::cpumask_clear_cpu(cpu, self.ptr) };
> + }
> +
> + /// Sets all CPUs in the cpumask.
> + pub fn set_all(&mut self) {
> + // SAFETY: `ptr` is guaranteed to be valid for the lifetime of `self`. And it is safe to
> + // call `cpumask_setall()`.
> + unsafe { bindings::cpumask_setall(self.ptr) };
> + }
> +
> + /// Gets weight of a cpumask.
> + pub fn weight(&self) -> u32 {
> + // SAFETY: `ptr` is guaranteed to be valid for the lifetime of `self`. And it is safe to
> + // call `cpumask_weight()`.
> + unsafe { bindings::cpumask_weight(self.ptr) }
> + }
> +
> + /// Copies cpumask.
> + pub fn copy(&self, dstp: &mut Self) {
> + // SAFETY: `ptr` is guaranteed to be valid for the lifetime of `self`. And it is safe to
> + // call `cpumask_copy()`.
> + unsafe { bindings::cpumask_copy(dstp.as_raw(), self.ptr) };
> + }
> +}
> +
> +impl Drop for Cpumask {
> + fn drop(&mut self) {
> + if self.owned {
> + // SAFETY: `ptr` is guaranteed to be valid for the lifetime of `self`. And it is safe
> + // to call `free_cpumask_var()`.
> + unsafe { bindings::free_cpumask_var(self.ptr) }
This is missing a semicolon, but it's not the last statement in the
block. Did you compile this with CPUMASK_OFFSTACK=n? I don't think it
compiles with that setting.
> + #[cfg(not(CONFIG_CPUMASK_OFFSTACK))]
> + // SAFETY: The pointer was earlier initialized from the result of `KBox::into_raw()`.
> + unsafe {
> + drop(KBox::from_raw(self.ptr))
> + };
This looks like you did not run rustfmt.
Alice
Powered by blists - more mailing lists