linux-kernel - Re: [PATCH v6 3/3] rust: sync: Add SpinLockIrq

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: <8734lew7jn.ffs@tglx>
Date: Wed, 02 Oct 2024 22:53:32 +0200
From: Thomas Gleixner <tglx@...utronix.de>
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>,
 linux-kernel@...r.kernel.org, Benno Lossin <benno.lossin@...ton.me>,
 Daniel Almeida <daniel.almeida@...labora.com>, Gary Guo
 <gary@...yguo.net>, Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor
 <alex.gaynor@...il.com>, Wedson Almeida Filho <wedsonaf@...il.com>, Boqun
 Feng <boqun.feng@...il.com>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>,
 Andreas Hindborg <a.hindborg@...sung.com>, Alice Ryhl
 <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, Martin Rodriguez
 Reboredo <yakoyoku@...il.com>, Valentin Obst <kernel@...entinobst.de>
Subject: Re: [PATCH v6 3/3] rust: sync: Add SpinLockIrq

On Mon, Sep 16 2024 at 17:28, Lyude Paul wrote:
> A variant of SpinLock that is expected to be used in noirq contexts, and
> thus requires that the user provide an kernel::irq::IrqDisabled to prove
> they are in such a context upon lock acquisition. This is the rust
> equivalent of spin_lock_irqsave()/spin_lock_irqrestore().

This fundamentally does not work with CONFIG_PREEMPT_RT. See:

   https://www.kernel.org/doc/html/latest/locking/locktypes.html

for further information. TLDR:

On RT enabled kernels spin/rw_lock are substituted by sleeping locks. So
you _cannot_ disable interrupts before taking the lock on RT enabled
kernels. That will result in a 'might_sleep()' splat.

There is a reason why the kernel has two distinct spinlock types:

    raw_spinlock_t and spinlock_t

raw_spinlock_t is a real spinning lock independent of CONFIG_PREEMPT_RT,
spinlock_t is mapped to raw_spinlock_t on CONFIG_PREEMPT_RT=n and to a
rtmutex based implementation for CONFIG_PREEMPT_RT=y.

As a consequence spin_lock_irq() and spin_lock_irqsave() will _NOT_
disable interrupts on a CONFIG_PREEMPT_RT=y kernel.

The proposed rust abstraction is not suitable for that.

At this phase of rust integration there is no need to wrap
raw_spinlock_t, so you have two options to solve that:

   1) Map Rust's SpinLockIrq() to spin_lock_irqsave() and
      spin_unlock_irqrestore() which does the right thing

   2) Play all the PREEMPT_RT games in the local irq disable abstraction

#1 is the right thing to do because no driver should rely on actually
disabling interrupts on the CPU. If there is a driver which does that,
then it's not compatible with RT and should use a local lock instead.

local locks aside of being RT compatible have the benefit that they give
scope to the protected region/data, while a plain local_irq_disable()
does not.

Don't even think about exposing this 'with_irq_disabled' interface
unless you are trying to move actual core code like the scheduler or low
level interrupt handling to rust.

Create explicit interrupt safe interfaces which map to the underlying
locking primitives instead.

Thanks,

        tglx