| 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: <20250225180223.763026-1-aliceryhl@google.com>
Date: Tue, 25 Feb 2025 18:02:23 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: aliceryhl@...gle.com
Cc: a.hindborg@...nel.org, alex.gaynor@...il.com, benno.lossin@...ton.me,
bjorn3_gh@...tonmail.com, boqun.feng@...il.com, dakr@...hat.com,
gary@...yguo.net, linux-kernel@...r.kernel.org, linux@...musvillemoes.dk,
ojeda@...nel.org, rust-for-linux@...r.kernel.org, tmgross@...ch.edu,
vincent.guittot@...aro.org, viresh.kumar@...aro.org, yury.norov@...il.com
Subject: Re: [PATCH 1/2] rust: Add initial cpumask abstractions
On Tue, Feb 25, 2025 at 6:12 PM Alice Ryhl <aliceryhl@...gle.com> wrote:
>
> On Tue, Feb 25, 2025 at 6:02 PM Viresh Kumar <viresh.kumar@...aro.org> wrote:
> >
> > On Tue, 25 Feb 2025 at 17:23, Alice Ryhl <aliceryhl@...gle.com> wrote:
> >
> > > Is it a problem if a value of type struct cpumask is moved? It looks
> > > like it is just an array of longs?
> >
> > With the current code, if I replace the Box with an on-stack variable,
> > the kernel crashes.
> >
> > In my usecase, the pointer to the cpumask array is sent to the OPP
> > core, which may update the content, though it doesn't save the pointer.
> >
> > But for another usecase, the C code may end up saving the pointer.
>
> I don't think that case applies here. Variables that could be stashed
> like that need a destructor that removes the pointer from the relevant
> C structures. This is because pinning only prevents the owner from
> moving the variable - it doesn't prevent the owner from destroying it.
I believe you can implement the offstack stuff like this. The idea is to have
one type that corresponds to `struct cpumask` and another type that corresponds
to `cpumask_var_t` on the C side. Only the latter type would need to do
anything wrt. the CPUMASK_OFFSTACK setting.
diff --git a/rust/kernel/cpumask.rs b/rust/kernel/cpumask.rs
index 13864424420b..f82dafbc4e61 100644
--- a/rust/kernel/cpumask.rs
+++ b/rust/kernel/cpumask.rs
@@ -4,111 +4,45 @@
//!
//! 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};
-
+use crate::{
+ alloc::{AllocError, Flags},
+ bindings,
+ prelude::*,
+ types::Opaque,
+};
#[cfg(CONFIG_CPUMASK_OFFSTACK)]
-use core::ptr;
+use core::ptr::{self, NonNull};
-/// A simple implementation of `struct cpumask` from the C code.
+/// This corresponds to the C type `struct cpumask`.
+#[repr(transparent)]
pub struct Cpumask {
- ptr: *mut bindings::cpumask,
- owned: bool,
+ mask: Opaque<bindings::cpumask>,
}
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)?);
-
- // 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.
+ /// Creates a reference to 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,
- }
+ /// The caller must ensure that `ptr` is valid for writing and remains valid for the lifetime
+ /// of the returned reference.
+ pub unsafe fn from_raw_mut<'a>(ptr: *mut bindings::cpumask) -> &'a mut Self {
+ unsafe { &mut *ptr.cast() }
}
- /// Creates a new abstraction instance of an existing `struct cpumask` pointer.
+ /// Creates a reference to 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,
- }
+ /// The caller must ensure that `ptr` is valid for reading and remains valid for the lifetime
+ /// of the returned reference.
+ pub unsafe fn from_raw<'a>(ptr: *const bindings::cpumask) -> &'a Self {
+ unsafe { &*ptr.cast() }
}
/// Obtain the raw `struct cpumask *`.
- pub fn as_raw(&mut self) -> *mut bindings::cpumask {
- self.ptr
+ pub fn as_raw(&self) -> *mut bindings::cpumask {
+ self as *const Cpumask as *mut bindings::cpumask
}
/// Sets CPU in the cpumask.
@@ -117,7 +51,7 @@ pub fn as_raw(&mut self) -> *mut bindings::cpumask {
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) };
+ unsafe { bindings::cpumask_set_cpu(cpu, self.as_raw()) };
}
/// Clears CPU in the cpumask.
@@ -126,43 +60,74 @@ pub fn set(&mut self, cpu: u32) {
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) };
+ unsafe { bindings::cpumask_clear_cpu(cpu, self.as_raw()) };
}
/// 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) };
+ unsafe { bindings::cpumask_setall(self.as_raw()) };
}
/// 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) }
+ unsafe { bindings::cpumask_weight(self.as_raw()) }
}
/// 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) };
+ unsafe { bindings::cpumask_copy(dstp.as_raw(), self.as_raw()) };
}
}
-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 corresponds to the C type alias `cpumask_var_t`.
+pub struct CpumaskBox {
+ #[cfg(CONFIG_CPUMASK_OFFSTACK)]
+ ptr: NonNull<Cpumask>,
+ #[cfg(not(CONFIG_CPUMASK_OFFSTACK))]
+ mask: Cpumask,
+}
+impl CpumaskBox {
+ pub fn new(_flags: Flags) -> Result<Self, AllocError> {
+ Ok(Self {
+ #[cfg(CONFIG_CPUMASK_OFFSTACK)]
+ ptr: {
+ let mut ptr: *mut bindings::cpumask = ptr::null_mut();
+ unsafe { bindings::zalloc_cpumask_var(&mut ptr, _flags.as_raw()) };
+ NonNull::new(ptr.cast()).ok_or(AllocError)?
+ },
#[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))
- };
- }
+ mask: unsafe { core::mem::zeroed() },
+ })
+ }
+}
+
+// Make CpumaskBox behave like a pointer to Cpumask.
+impl core::ops::Deref for CpumaskBox {
+ type Target = Cpumask;
+
+ #[cfg(CONFIG_CPUMASK_OFFSTACK)]
+ fn deref(&self) -> &Cpumask {
+ unsafe { &*self.ptr.as_ptr() }
+ }
+
+ #[cfg(not(CONFIG_CPUMASK_OFFSTACK))]
+ fn deref(&self) -> &Cpumask {
+ &self.mask
+ }
+}
+
+impl Drop for CpumaskBox {
+ fn drop(&mut self) {
+ #[cfg(CONFIG_CPUMASK_OFFSTACK)]
+ unsafe {
+ bindings::free_cpumask_var(self.as_raw())
+ };
}
}
Powered by blists - more mailing lists