linux-kernel - Re: [PATCH RFC x86/nmi] Fix out-of-order nesting checks

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]

Date:   Thu, 12 Oct 2023 03:45:15 -0700
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Ingo Molnar <mingo@...nel.org>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org, clm@...com,
        tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        dave.hansen@...ux.intel.com, hpa@...or.com
Subject: Re: [PATCH RFC x86/nmi] Fix out-of-order nesting checks

On Thu, Oct 12, 2023 at 08:37:25AM +0200, Ingo Molnar wrote:
> 
> * Paul E. McKenney <paulmck@...nel.org> wrote:
> 
> > The ->idt_seq and ->recv_jiffies variables added by commit 1a3ea611fc10
> > ("x86/nmi: Accumulate NMI-progress evidence in exc_nmi()") place
> > the exit-time check of the bottom bit of ->idt_seq after the
> > this_cpu_dec_return() that re-enables NMI nesting.  This can result in
> > the following sequence of events on a given CPU in kernels built with
> > CONFIG_NMI_CHECK_CPU=y:
> > 
> > o       An NMI arrives, and ->idt_seq is incremented to an odd number.
> >         In addition, nmi_state is set to NMI_EXECUTING==1.
> > 
> > o       The NMI is processed.
> > 
> > o       The this_cpu_dec_return(nmi_state) zeroes nmi_state and returns
> >         NMI_EXECUTING==1, thus opting out of the "goto nmi_restart".
> > 
> > o       Another NMI arrives and ->idt_seq is incremented to an even
> >         number, triggering the warning.  But all is just fine, at least
> >         assuming we don't get so many closely spaced NMIs that the stack
> >         overflows or some such.
> > 
> > Experience on the fleet indicates that the MTBF of this false positive
> > is about 70 years.  Or, for those who are not quite that patient, the
> > MTBF appears to be about one per week per 4,000 systems.
> > 
> > Fix this false-positive warning by moving the "nmi_restart" label before
> > the initial ->idt_seq increment/check and moving the this_cpu_dec_return()
> > to follow the final ->idt_seq increment/check.  This way, all nested NMIs
> > that get past the NMI_NOT_RUNNING check get a clean ->idt_seq slate.
> > And if they don't get past that check, they will set nmi_state to
> > NMI_LATCHED, which will cause the this_cpu_dec_return(nmi_state)
> > to restart.
> 
> This looks like a sensible fix: the warning should obviously be atomic wrt. 
> the no-nesting region. I've applied your fix to tip:x86/irq, as it doesn't 
> seem urgent enough with a MTBF of 70 years to warrant tip:x86/urgent handling. ;-)

Works for me!  ;-)

And thank you!

							Thanx, Paul