linux-kernel - Re: [PATCH 0/3] Remove volatile from arch_raw_cpu

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:   Thu, 24 Mar 2022 11:28:16 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Boqun Feng <boqun.feng@...il.com>,
        Borislav Petkov <bp@...en8.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Waiman Long <longman@...hat.com>,
        Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Will Deacon <will@...nel.org>, Dennis Zhou <dennis@...nel.org>,
        Tejun Heo <tj@...nel.org>, Christoph Lameter <cl@...ux.com>
Subject: Re: [PATCH 0/3] Remove volatile from arch_raw_cpu_ptr() and revert
 the hacks.

On Thu, Mar 24, 2022 at 10:39 AM Sebastian Andrzej Siewior
<bigeasy@...utronix.de> wrote:
>
> The following series does that. The assembly code looks okay. In a few
> simple test cases the this_cpu_ptr() usage is always created and is not
> moved passed preempt_enable() statement.

Hmm. I didn't think things through, and you're very correct in having
thought about preempt_enable/disable as needing barriers for that asm.

So the "volatile" does have that potential significance for that
arch_raw_cpu_ptr() in case it was what ordered it wrt preemption.

It turns out that it's all ok, because

 - even without the 'volatile', arch_raw_cpu_ptr() has an *input* that
is a memory location:

                     : "m" (this_cpu_off)

 - and prempt_{dis,en}able() fundamentally has a 'barrier()' in it

and as such it's not just random luck that the percpu thing is never
moved around preemption points. They are properly serialized even
without any 'volatile'.

But that "it's not just random luck" might be worth a comment.

So Ack on your series, but that additional comment might be worth it,
since I didn't even think of it until you mentioned it. Clearly much
too subtle.

Thanks,
              Linus