linux-kernel - Re: [RFC RESEND v10 04/14] rust: Introduce interrupt module

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] [day] [month] [year] [list]

Message-Id: <DA8J8BHPNBAM.IUBJ8TL9L6U8@kernel.org>
Date: Thu, 29 May 2025 11:21:07 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Lyude Paul" <lyude@...hat.com>, <rust-for-linux@...r.kernel.org>,
 "Thomas Gleixner" <tglx@...utronix.de>, "Boqun Feng"
 <boqun.feng@...il.com>, <linux-kernel@...r.kernel.org>, "Daniel Almeida"
 <daniel.almeida@...labora.com>
Cc: "Miguel Ojeda" <ojeda@...nel.org>, "Alex Gaynor"
 <alex.gaynor@...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>, "Danilo Krummrich" <dakr@...nel.org>,
 "Wedson Almeida Filho" <wedsonaf@...il.com>, "FUJITA Tomonori"
 <fujita.tomonori@...il.com>, "Greg Kroah-Hartman"
 <gregkh@...uxfoundation.org>, "Xiangfei Ding" <dingxiangfei2009@...il.com>
Subject: Re: [RFC RESEND v10 04/14] rust: Introduce interrupt module

On Wed May 28, 2025 at 12:21 AM CEST, Lyude Paul wrote:
> This introduces a module for dealing with interrupt-disabled contexts,
> including the ability to enable and disable interrupts along with the
> ability to annotate functions as expecting that IRQs are already
> disabled on the local CPU.
>
> [Boqun: This is based on Lyude's work on interrupt disable abstraction,
> I port to the new local_interrupt_disable() mechanism to make it work
> as a guard type. I cannot even take the credit of this design, since
> Lyude also brought up the same idea in zulip. Anyway, this is only for
> POC purpose, and of course all bugs are mine]
>
> Signed-off-by: Lyude Paul <lyude@...hat.com>
> Co-Developed-by: Boqun Feng <boqun.feng@...il.com>
> Signed-off-by: Boqun Feng <boqun.feng@...il.com>

Two nits below, with those fixed:

Reviewed-by: Benno Lossin <lossin@...nel.org>

> diff --git a/rust/kernel/interrupt.rs b/rust/kernel/interrupt.rs
> new file mode 100644
> index 0000000000000..e66aa85f79940
> --- /dev/null
> +++ b/rust/kernel/interrupt.rs
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Interrupt controls
> +//!
> +//! This module allows Rust code to annotate areas of code where local processor interrupts should
> +//! be disabled, along with actually disabling local processor interrupts.
> +//!
> +//! # ⚠️ Warning! ⚠️
> +//!
> +//! The usage of this module can be more complicated than meets the eye, especially surrounding
> +//! [preemptible kernels]. It's recommended to take care when using the functions and types defined
> +//! here and familiarize yourself with the various documentation we have before using them, along
> +//! with the various documents we link to here.
> +//!
> +//! # Reading material
> +//!
> +//! - [Software interrupts and realtime (LWN)](https://lwn.net/Articles/520076)
> +//!
> +//! [preemptible kernels]: https://www.kernel.org/doc/html/latest/locking/preempt-locking.html
> +
> +use bindings;

This shouldn't be necessary, right?

> +impl LocalInterruptDisabled {
> +    const ASSUME_DISABLED: &'static LocalInterruptDisabled = &LocalInterruptDisabled(NotThreadSafe);

I'd move this into the function body.

---
Cheers,
Benno

> +
> +    /// Assume that local processor interrupts are disabled on preemptible kernels.
> +    ///
> +    /// This can be used for annotating code that is known to be run in contexts where local
> +    /// processor interrupts are disabled on preemptible kernels. It makes no changes to the local
> +    /// interrupt state on its own.
> +    ///
> +    /// # Safety
> +    ///
> +    /// For the whole life `'a`, local interrupts must be disabled on preemptible kernels. This
> +    /// could be a context like for example, an interrupt handler.
> +    pub unsafe fn assume_disabled<'a>() -> &'a LocalInterruptDisabled {
> +        Self::ASSUME_DISABLED
> +    }
> +}