| 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
| ||
|
Message-ID: <0b766dba0b004ced94131e158cd8e67d@hisilicon.com>
Date: Fri, 12 Feb 2021 23:46:59 +0000
From: "Song Bao Hua (Barry Song)" <song.bao.hua@...ilicon.com>
To: Arnd Bergmann <arnd@...nel.org>
CC: "tglx@...utronix.de" <tglx@...utronix.de>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"arnd@...db.de" <arnd@...db.de>,
"geert@...ux-m68k.org" <geert@...ux-m68k.org>,
"funaho@...ai.org" <funaho@...ai.org>,
"philb@....org" <philb@....org>, "corbet@....net" <corbet@....net>,
"mingo@...hat.com" <mingo@...hat.com>,
"linux-m68k@...ts.linux-m68k.org" <linux-m68k@...ts.linux-m68k.org>,
"fthain@...egraphics.com.au" <fthain@...egraphics.com.au>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFC] IRQ handlers run with some high-priority interrupts(not
NMI) enabled on some platform
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@...nel.org]
> Sent: Saturday, February 13, 2021 12:06 PM
> To: Song Bao Hua (Barry Song) <song.bao.hua@...ilicon.com>
> Cc: tglx@...utronix.de; gregkh@...uxfoundation.org; arnd@...db.de;
> geert@...ux-m68k.org; funaho@...ai.org; philb@....org; corbet@....net;
> mingo@...hat.com; linux-m68k@...ts.linux-m68k.org;
> fthain@...egraphics.com.au; linux-kernel@...r.kernel.org
> Subject: Re: [RFC] IRQ handlers run with some high-priority interrupts(not NMI)
> enabled on some platform
>
> On Sat, Feb 13, 2021 at 12:00 AM Song Bao Hua (Barry Song)
> <song.bao.hua@...ilicon.com> wrote:
> > > -----Original Message-----
> > > From: Arnd Bergmann [mailto:arnd@...nel.org]
> > > Sent: Saturday, February 13, 2021 11:34 AM
> > > To: Song Bao Hua (Barry Song) <song.bao.hua@...ilicon.com>
> > > Cc: tglx@...utronix.de; gregkh@...uxfoundation.org; arnd@...db.de;
> > > geert@...ux-m68k.org; funaho@...ai.org; philb@....org; corbet@....net;
> > > mingo@...hat.com; linux-m68k@...ts.linux-m68k.org;
> > > fthain@...egraphics.com.au; linux-kernel@...r.kernel.org
> > > Subject: Re: [RFC] IRQ handlers run with some high-priority interrupts(not
> NMI)
> > > enabled on some platform
> > >
> > > On Fri, Feb 12, 2021 at 2:18 AM Song Bao Hua (Barry Song)
> > > <song.bao.hua@...ilicon.com> wrote:
> > >
> > > > So I am requesting comments on:
> > > > 1. are we expecting all interrupts except NMI to be disabled in irq handler,
> > > > or do we actually allow some high-priority interrupts between low and
> NMI
> > > to
> > > > come in some platforms?
> > >
> > > I tried to come to an answer but this does not seem particularly well-defined.
> > > There are a few things I noticed:
> > >
> > > - going through the local_irq_save()/restore() implementations on all
> > > architectures, I did not find any other ones besides m68k that leave
> > > high-priority interrupts enabled. I did see that at least alpha and openrisc
> > > are designed to support that in hardware, but the code just leaves the
> > > interrupts disabled.
> >
> > The case is a little different. Explicit local_irq_save() does disable all
> > high priority interrupts on m68k. The only difference is arch_irqs_disabled()
> > of m68k will return true while low-priority interrupts are masked and high
> > -priority are still open. M68k's hardIRQ also runs in this context with high
> > priority interrupts enabled.
>
> My point was that on most other architectures, local_irq_save()/restore()
> always disables/enables all interrupts, while on m68k it restores the
> specific level they were on before. On alpha, it does the same as on m68k,
> but then the top-level interrupt handler just disables them all before calling
> into any other code.
That's what I think m68k is better to do.
Looks weird that nested interrupts can enter while arch_irqs_disabled()
is true on m68k because masking low-priority interrupts with
high-interrupts still enabled would be able to make m68k's
arch_irqs_disabled() true, which is exactly the environment
m68k's irq handler is running.
So I was actually trying to warn this unusual case - interrupts
get nested while both in_hardirq() and irqs_disabled() are true.
diff --git a/include/linux/hardirq.h b/include/linux/hardirq.h
index 7c9d6a2d7e90..b8ca27555c76 100644
--- a/include/linux/hardirq.h
+++ b/include/linux/hardirq.h
@@ -32,6 +32,7 @@ static __always_inline void rcu_irq_enter_check_tick(void)
*/
#define __irq_enter() \
do { \
+ WARN_ONCE(in_hardirq() && irqs_disabled(), "nested
interrupts\n"); \
preempt_count_add(HARDIRQ_OFFSET); \
lockdep_hardirq_enter(); \
account_hardirq_enter(current); \
@@ -44,6 +45,7 @@ static __always_inline void rcu_irq_enter_check_tick(void)
*/
#define __irq_enter_raw() \
do { \
+ WARN_ONCE(in_hardirq() && irqs_disabled(), " nested
interrupts\n"); \
preempt_count_add(HARDIRQ_OFFSET); \
lockdep_hardirq_enter(); \
} while (0)
And I also think it is better for m68k's arch_irqs_disabled() to
return true only when both low and high priority interrupts are
disabled rather than try to mute this warn in genirq by a weaker
condition:
irqreturn_t __handle_irq_event_percpu(struct irq_desc *desc, unsigned int *flags)
{
...
trace_irq_handler_entry(irq, action);
res = action->handler(irq, action->dev_id);
trace_irq_handler_exit(irq, action, res);
if (WARN_ONCE(!irqs_disabled(),"irq %u handler %pS enabled interrupts\n",
irq, action->handler))
local_irq_disable();
}
This warn is not activated on m68k because its arch_irqs_disabled() return
true though its high-priority interrupts are still enabled.
>
> It's possible that I missed some other implementation doing the same
> as m68k, as this code is fairly subtle on some architectures.
>
> Arnd
Thanks
Barry
Powered by blists - more mailing lists