linux-kernel - Re: [PATCH] percpu: make this_cpu_generic

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: <alpine.DEB.2.20.1709261134280.25986@nuc-kabylake>
Date:   Tue, 26 Sep 2017 11:42:09 -0500 (CDT)
From:   Christopher Lameter <cl@...ux.com>
To:     Thomas Gleixner <tglx@...utronix.de>
cc:     Tejun Heo <tj@...nel.org>, Mark Rutland <mark.rutland@....com>,
        linux-kernel@...r.kernel.org, Arnd Bergmann <arnd@...db.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Pranith Kumar <bobby.prani@...il.com>,
        linux-arch@...r.kernel.org,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Subject: Re: [PATCH] percpu: make this_cpu_generic_read() atomic w.r.t.
 interrupts

On Tue, 26 Sep 2017, Thomas Gleixner wrote:

> > because it could also occur before or after the preempt_enable/disable
> > without the code being able to distinguish that case.
> >
> > The fetch of a scalar value from memory is an atomic operation and that is
> > required from all arches. There is an exception for double word fetches.
>
> this_cpu_read_8() is a double word fetch on many 32bit architectures.

Ok then this_cpu_read_8 for those platforms need to disable interrupts.
But that is not true for all arches.

> > Maybe we would need to special code that case but so far this does not
> > seem to have been an issue.
>
> Just because nobody ran into problem with that it is a non issue? That's
> just hillarious.

Its is even more (see your above statement) stupid to figure out that this
is actually unnecessary in the generic case and then continue the
argument.

> It's obviously not correct and needs to be fixed _before_ someone has to go
> through the pain of debugging such a problem.

As you pointed out the current approach *is* correct. Fetching a scalar
word is an atomic operation and must be one. If an arch cannot guarantee
that then arch specific measures need to be implemented to ensure that
this guarantee is kept. Could be done with interrupts disables, cmpxchg or
the reservation scheme on RISC processors. It definitely does not
belong into generic code.