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: <aLn2PzD1KRbu4SpZ@yury>
Date: Thu, 4 Sep 2025 16:27:43 -0400
From: Yury Norov <yury.norov@...il.com>
To: Mitchell Levy <levymitchell0@...il.com>
Cc: 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>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Dennis Zhou <dennis@...nel.org>, Tejun Heo <tj@...nel.org>,
	Christoph Lameter <cl@...ux.com>,
	Danilo Krummrich <dakr@...nel.org>,
	Benno Lossin <lossin@...nel.org>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Tyler Hicks <code@...icks.com>, linux-kernel@...r.kernel.org,
	rust-for-linux@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH v3 1/7] rust: percpu: introduce a rust API for per-CPU
 variables

On Thu, Sep 04, 2025 at 12:53:59PM -0700, Mitchell Levy wrote:
> On Wed, Sep 03, 2025 at 05:42:08PM -0400, Yury Norov wrote:
> > On Thu, Aug 28, 2025 at 12:00:08PM -0700, Mitchell Levy wrote:

...

> > > +    /// Get a `&mut MaybeUninit<T>` to the per-CPU variable on the current CPU represented by `&self`
> > > +    ///
> > > +    /// # Safety
> > > +    /// The returned `&mut T` must follow Rust's aliasing rules. That is, no other `&(mut) T` may
> > > +    /// exist that points to the same location in memory. In practice, this means that `get_(mut_)ref`
> > 
> > How long is this line?
> 
> 102 chars, or 103 if you include the newline. `rustfmt` doesn't break
> the line, so I left it as-is for this patch. Happy to change it if it
> poses a problem, though.

Then don't use that tool - it's broken. In kernel we used to have 80-chars
limit for the lines, recently relaxed to 100.
 
> > > +    /// must not be called on another `PerCpuPtr<T>` that is a copy/clone of `&self` for as long as
> > > +    /// the returned reference lives.
> > > +    ///
> > > +    /// CPU preemption must be disabled before calling this function and for the lifetime of the
> > > +    /// returned reference. Otherwise, the returned reference might end up being a reference to a
> > > +    /// different CPU's per-CPU area, causing the potential for a data race.
> > > +    #[allow(clippy::mut_from_ref)] // Safety requirements prevent aliasing issues
> > > +    pub unsafe fn get_mut_ref(&self) -> &mut MaybeUninit<T> {
> > > +        // SAFETY: `self.get_ptr()` returns a valid pointer to a `MaybeUninit<T>` by its contract,
> > > +        // and the safety requirements of this function ensure that the returned reference is
> > > +        // exclusive.
> > > +        unsafe { &mut *(self.get_ptr()) }
> > > +    }
> > 
> > Here and everywhere: would it make sense to enforce it by testing
> > the CPU with preemptible() before returning a reference? 
> 
> The only thing we could do would be to panic, which I don't 100% love.
> Another alternative would be to take a &'a CpuGuard and bound the
> lifetime of the returned reference to 'a, and then we don't need to do
> any run-time checking at all.
> 
> Originally, I had left this to the caller because it might make sense
> down the line for some complex behavior based on per-CPU (e.g., per-CPU
> refcount) to do all its own management of per-CPU variables using
> `PerCpuPtr` as a core primitive. In these cases, I believe there are
> some times where being non-preemptible wouldn't actually be required
> (that said, my thoughts on this aren't well reflected in the safety
> comment, since I said it must be disabled... gah). But, the more I think
> about it, the more I think these use cases would be better served by
> just using `get_ptr` --- conjuring `&mut` references seems like it would
> be a big footgun. And the safety comment already actually reflects these
> thoughts somewhat :)

If you think that in future there will be a user who will not need to
disable preemption before dereferencing a percpu pointer, then you can
add another less restricted flavor of the helper.

> For v4 I will probably have this function take a &'a CpuGuard and use
> that to bound the liftetime of the returned reference, unless there are
> other thoughts on this point.

I don't want you to panic just because of invlid user call, but
whatever you call in comment must be enforced in code, right?


You can use the guard, if it guarantees the preemption disabled; or 
you can return None; you can create CONFIG_RUST_PERCPU_HARDENED for
panics.

Please refer the recent bitmap API wrapper, and how erroneous request
is handled there.

https://lore.kernel.org/all/20250904165015.3791895-4-bqe@google.com/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ