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

Message-ID: <aPHmuA9kXSn438v5@tardis.local>
Date: Thu, 16 Oct 2025 23:48:24 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: David Laight <david.laight.linux@...il.com>
Cc: Lyude Paul <lyude@...hat.com>, rust-for-linux@...r.kernel.org,
	Thomas Gleixner <tglx@...utronix.de>, 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 Thu, Oct 16, 2025 at 10:24:21PM +0100, David Laight wrote:
> 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.
> 

This patch is exactly about using percpu (in this case it's the preempt
count) to track interrupt disabling nested level and enabling interrupts
when the count reaches to 0 ;-)

Regards,
Boqun

> 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
>