linux-kernel - Re: [PATCH v10 6/6] x86/split_lock: Enable split lock detection by kernel parameter

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: <CALCETrVjXC7RHZCkAcWEeCrJq7DPeVBooK8S3mG0LT8q9AxvPw@mail.gmail.com>
Date:   Fri, 22 Nov 2019 13:23:30 -0800
From:   Andy Lutomirski <luto@...nel.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     "Luck, Tony" <tony.luck@...el.com>,
        Andy Lutomirski <luto@...nel.org>,
        "Yu, Fenghua" <fenghua.yu@...el.com>,
        David Laight <David.Laight@...lab.com>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        H Peter Anvin <hpa@...or.com>,
        "Raj, Ashok" <ashok.raj@...el.com>,
        "Shankar, Ravi V" <ravi.v.shankar@...el.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        x86 <x86@...nel.org>, Will Deacon <will@...nel.org>
Subject: Re: [PATCH v10 6/6] x86/split_lock: Enable split lock detection by
 kernel parameter

On Fri, Nov 22, 2019 at 12:31 PM Peter Zijlstra <peterz@...radead.org> wrote:
>
> On Fri, Nov 22, 2019 at 05:48:14PM +0000, Luck, Tony wrote:
> > > When we use byte ops, we must consider the word as 4 independent
> > > variables. And in that case the later load might observe the lock-byte
> > > state from 3, because the modification to the lock byte from 4 is in
> > > CPU2's store-buffer.
> >
> > So we absolutely violate this with the optimization for constant arguments
> > to set_bit(), clear_bit() and change_bit() that are implemented as byte ops.
> >
> > So is code that does:
> >
> >       set_bit(0, bitmap);
> >
> > on one CPU. While another is doing:
> >
> >       set_bit(mybit, bitmap);
> >
> > on another CPU safe? The first operates on just one byte, the second  on 8 bytes.
>
> It is safe if all you care about is the consistency of that one bit.
>

I'm still lost here.  Can you explain how one could write code that
observes an issue?  My trusty SDM, Vol 3 8.2.2 says "Locked
instructions have a total order."  8.2.3.9 says "Loads and Stores Are
Not Reordered with Locked Instructions."  Admittedly, the latter is an
"example", but the section is very clear about the fact that a locked
instruction prevents reordering of a load or a store issued by the
same CPU relative to the locked instruction *regardless of whether
they overlap*.

So using LOCK to impleent smb_mb() is correct, and I still don't
understand your particular concern.

I understand that the CPU is probably permitted to optimize a LOCK RMW
operation such that it retires before the store buffers of earlier
instructions are fully flushed, but only if the store buffer and cache
coherency machinery work together to preserve the architecturally
guaranteed ordering.