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]

Date:   Sun, 24 Sep 2017 17:12:13 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Paul McKenney <paulmck@...ux.vnet.ibm.com>
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 5:03 PM, Paul E. McKenney
<paulmck@...ux.vnet.ibm.com> wrote:
>
> 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.

I really don't think it's warranted. The values are *stable*. There's
no subtle lack of locking, or some optimistic access to a value that
can change.

The compiler can generate code to read the value fifteen billion
times, and it will always get the same value.

Yes, maybe in between the different accesses, an NMI will happen, and
the value will be incremented, but then as the NMI exits, it will
decrement again, so the code that got interrupted will not actually
see the change.

So the READ_ONCE() isn't "paranoia". It's just confusing.

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

NO.

That would be just more of that confusion.

That value is STABLE. It's stable even within an NMI handler. The NMI
code can read it, modify it, write it back, do a little dance, all
without having to care. There's no "_ONCE()" about it - not for the
readers, not for the writers, not for _anybody_.

So adding even more READ/WRITE_ONCE() accesses wouldn't be
"consistent", it would just be insanity.

Now, if an NMI happens and the value would be different on entry than
it is on exit, that would be something else. Then it really wouldn't
be stable wrt random users. But that would also be a major bug in the
NMI handler, as far as I can tell.

So the reason I'm objecting to that READ_ONCE() is that it isn't
"paranoia", it's "voodoo programming". And we don't do voodoo
programming.

            Linus