linux-kernel - Re: [PATCH 1/4] rcu: Allow for page faults in NMI handlers

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: <20170925000303.GJ3521@linux.vnet.ibm.com>
Date:   Sun, 24 Sep 2017 17:03:03 -0700
From:   "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        stable <stable@...r.kernel.org>
Subject: Re: [PATCH 1/4] rcu: Allow for page faults in NMI handlers

On Sun, Sep 24, 2017 at 12:42:32PM -0700, Linus Torvalds wrote:
> On Sat, Sep 23, 2017 at 1:56 PM, Steven Rostedt <rostedt@...dmis.org> wrote:
> > +
> > +       /* Page faults can happen in NMI handlers, so check... */
> > +       if (READ_ONCE(rdtp->dynticks_nmi_nesting))
> > +               return;
> > +
> 
> What is the reason for the READ_ONCE() here (and in the other case)?
> 
> It doesn't seem to have any actual reason.  It's a "stable" per-cpu
> value in that even if an NMI were to happen, it gets incremented and
> then decremented, so there is nothing really volatile about it
> anywhere that I can see.
> 
> So the READ_ONCE() seems to be just pure confusion.
> 
> What am I missing?

Mostly just paranoia on my part.  I would be happy to remove it if
you prefer.  Or you or Steve can do so if that is more convenient.

And yes, consistency would dictate that the uses in rcu_nmi_enter()
and rcu_nmi_exit() should be _ONCE(), particularly the stores to
->dynticks_nmi_nesting.  But I am not too worried about that right now
because I suspect that I should be able to combine rcu_irq_{enter,exit}()
and rcu_nmi_{enter,exit}(), which would be a good simplification.

							Thanx, aul