linux-kernel - Re: [PATCH v2 1/3] rust: Introduce irq 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <5e449ae3-d858-45a2-ace8-28d9040b83a7@proton.me>
Date: Thu, 01 Aug 2024 10:10:00 +0000
From: Benno Lossin <benno.lossin@...ton.me>
To: Lyude Paul <lyude@...hat.com>, rust-for-linux@...r.kernel.org
Cc: Danilo Krummrich <dakr@...hat.com>, airlied@...hat.com, Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>, Waiman Long <longman@...hat.com>, Peter Zijlstra <peterz@...radead.org>, Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, Wedson Almeida Filho <wedsonaf@...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@...sung.com>, Alice Ryhl <aliceryhl@...gle.com>, Martin Rodriguez Reboredo <yakoyoku@...il.com>, FUJITA Tomonori <fujita.tomonori@...il.com>, Aakash Sen Sharma <aakashsensharma@...il.com>, Valentin Obst <kernel@...entinobst.de>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] rust: Introduce irq module

On 01.08.24 11:51, Benno Lossin wrote:
> On 01.08.24 00:35, Lyude Paul wrote:
>> +/// Run the closure `cb` with interrupts disabled on the local CPU.
>> +///
>> +/// This creates an [`IrqDisabled`] token, which can be passed to functions that must be run
>> +/// without interrupts.
>> +///
>> +/// # Examples
>> +///
>> +/// Using [`with_irqs_disabled`] to call a function that can only be called with interrupts
>> +/// disabled:
>> +///
>> +/// ```
>> +/// use kernel::irq::{IrqDisabled, with_irqs_disabled};
>> +///
>> +/// // Requiring interrupts be disabled to call a function
>> +/// fn dont_interrupt_me(_irq: IrqDisabled<'_>) {
>> +///     /* When this token is available, IRQs are known to be disabled. Actions that rely on this
>> +///      * can be safely performed
>> +///      */
>> +/// }
>> +///
>> +/// // Disabling interrupts. They'll be re-enabled once this closure completes.
>> +/// with_irqs_disabled(|irq| dont_interrupt_me(irq));
>> +/// ```
>> +#[inline]
>> +pub fn with_irqs_disabled<'a, T, F>(cb: F) -> T
>> +where
>> +    F: FnOnce(IrqDisabled<'a>) -> T,
> 
> You can use this as the signature:
> 
>     pub fn with_irqs_disabled<'a, T>(cb: impl FnOnce(IrqDisabled<'a>) -> T) -> T

I just noticed that this and the version above are wrong, since they
allow `T` to depend on `'a` ie you can do the following:

    pub fn cheat() -> IrqDisabled<'static> {
        with_irqs_disabled(|irq| irq)
    }

And thus obtain an `IrqDisabled` token without IRQs being disabled.

To fix this, you must use the `for<'a>` notation, so either

    pub fn with_irqs_disabled<T>(cb: impl for<'a> FnOnce(IrqDisabled<'a>) -> T) -> T

or

    pub fn with_irqs_disabled<T, F>(cb: F) -> T
    where
        F: for<'a> FnOnce(IrqDisabled<'a>) -> T,

This ensures that the callback works for any lifetime and thus `T` is
not allowed to depend on it.

---
Cheers,
Benno