linux-kernel - Re: [PATCH v13 05/17] irq & spin_lock: Add counted interrupt disabling/enabling

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: <20251016222421.512ca8d1@pumpkin>
Date: Thu, 16 Oct 2025 22:24:21 +0100
From: David Laight <david.laight.linux@...il.com>
To: Lyude Paul <lyude@...hat.com>
Cc: 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>, Ingo Molnar <mingo@...hat.com>,
 Peter Zijlstra <peterz@...radead.org>, Juri Lelli <juri.lelli@...hat.com>,
 Vincent Guittot <vincent.guittot@...aro.org>, Dietmar Eggemann
 <dietmar.eggemann@....com>, Steven Rostedt <rostedt@...dmis.org>, Ben
 Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>, Valentin
 Schneider <vschneid@...hat.com>, Will Deacon <will@...nel.org>, Waiman Long
 <longman@...hat.com>, Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor
 <alex.gaynor@...il.com>, Gary Guo <gary@...yguo.net>, Björn
 Roy Baron <bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>,
 Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl
 <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich
 <dakr@...nel.org>, David Woodhouse <dwmw@...zon.co.uk>, Sebastian Andrzej
 Siewior <bigeasy@...utronix.de>, Joel Fernandes <joelagnelf@...dia.com>,
 Ryo Takakura <ryotkkr98@...il.com>, K Prateek Nayak
 <kprateek.nayak@....com>
Subject: Re: [PATCH v13 05/17] irq & spin_lock: Add counted interrupt
 disabling/enabling

On Mon, 13 Oct 2025 11:48:07 -0400
Lyude Paul <lyude@...hat.com> wrote:

> From: Boqun Feng <boqun.feng@...il.com>
> 
> Currently the nested interrupt disabling and enabling is present by
> _irqsave() and _irqrestore() APIs, which are relatively unsafe, for
> example:
> 
> 	<interrupts are enabled as beginning>
> 	spin_lock_irqsave(l1, flag1);
> 	spin_lock_irqsave(l2, flag2);
> 	spin_unlock_irqrestore(l1, flags1);
> 	<l2 is still held but interrupts are enabled>
> 	// accesses to interrupt-disable protect data will cause races.

To do this right you have to correctly 'nest' the flags even though
the locks are chained.
So you should have:
	spin_unlock_irqrestore(l1, flags2);
Which is one reason why schemes that save the interrupt state in the
lock are completely broken.

Did you consider a scheme where the interrupt disable count is held in a
per-cpu variable (rather than on-stack)?
It might have to be the same per-cpu variable that is used for disabling
pre-emption.
If you add (say) 256 to disable interrupts and do the hardware disable
when the count ends up between 256 and 511 and the enable on the opposite
transition I think it should work.
An interrupt after the increment will be fine - it can't do a process
switch.

The read-add-write doesn't even need to be atomic.
The problem is a process switch and that can only happen when the only
value is zero - so it doesn't matter it is can from a different cpu!

I know some systems (I think including x86) have only incremented such a
counter instead of doing the hardware interrupt disable.
When an interrupt happens they realise it shouldn't have, block the IRQ,
remember there is a deferred interrupt, and return from the ISR.
This is good for very short disables - because the chance of an IRQ
is low.

	David