linux-kernel - RE: [RFC] IRQ handlers run with some high-priority interrupts(not NMI) enabled on some platform

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: <24e0652b3afa48cdbf7c83287e43c087@hisilicon.com>
Date:   Fri, 12 Feb 2021 23:00:39 +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 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.

> 
> - The generic code is clearly prepared to handle nested hardirqs, and
>    the irq_enter()/irq_exit() functions have a counter in preempt_count
>    for the nesting level, using a 4-bit number for hardirq, plus another
>    4-bit number for NMI.

Yes, I understand nested interrupts are supported by an explicit 
local_irq_enable_in_hardirq(). Mk68k's case is different, nested
interrupts can come with arch_irqs_disabled() is true and while
nobody has called local_irq_enable_in_hardirq() in the previous
hardIRQ because hardIRQ keeps high-priority interrupts open.

> 
> - There are a couple of (ancient) drivers that enable interrupts in their
>    interrupt handlers, see the four callers of local_irq_enable_in_hardirq()
>    (all in the old drivers/ide stack) and arch/ia64/kernel/time.c, which
>    enables interupts in its timer function (I recently tried removing this
>    and my patch broke ia64 timers, but I'm not sure if the cause was
>    the local_irq_enable() or something else).
> 
> - The local_irq_enable_in_hardirq() function itself turns into a nop
>   when lockdep is enabled, since d7e9629de051 ("[PATCH] lockdep:
>   add local_irq_enable_in_hardirq() API"). According to the comment
>   in there, lockdep already enforces the behavior you suggest. Note that
>   lockdep support is missing on m68k (and also alpha, h8300, ia64, nios2,
>   and parisc).
> 
> > 2. If either side is true, I think we need to document it somewhere as there
> > is always confusion about this.
> >
> > Personally, I would expect all interrupts to be disabled and I like the way
> > of ARM64 to only use high-priority interrupt as pseudo NMI:
> > https://lwn.net/Articles/755906/
> > Though Finn argued that this will contribute to lose hardware feature of m68k.
> 
> Regardless of what is documented, I would argue that any platform
> that relies on this is at the minimum doing something risky because at
> the minimum this runs into hard to debug code paths that are not
> exercised on any of the common architectures.
> 
>         Arnd


Thanks
Barry