linux-kernel - Re: [PATCH -v1.1] x86/mm: Fix INVPCID asm constraint

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] [day] [month] [year] [list]

Message-ID: <CALCETrVuq1Kckf5nYoCXV+cT=S2GTRqnZAvF6AEPW-ppBpE_HQ@mail.gmail.com>
Date:	Wed, 10 Feb 2016 18:48:50 -0800
From:	Andy Lutomirski <luto@...capital.net>
To:	Borislav Petkov <bp@...en8.de>
Cc:	Michael Matz <matz@...e.de>, Ingo Molnar <mingo@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Oleg Nesterov <oleg@...hat.com>,
	Brian Gerst <brgerst@...il.com>,
	Luis Rodriguez <mcgrof@...e.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Toshi Kani <toshi.kani@...com>,
	Andrew Lutomirski <luto@...nel.org>,
	Andrey Ryabinin <aryabinin@...tuozzo.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"linux-tip-commits@...r.kernel.org" 
	<linux-tip-commits@...r.kernel.org>
Subject: Re: [PATCH -v1.1] x86/mm: Fix INVPCID asm constraint

On Wed, Feb 10, 2016 at 6:51 AM, Borislav Petkov <bp@...en8.de> wrote:
> On Wed, Feb 10, 2016 at 02:48:02PM +0100, Michael Matz wrote:
>> Hi,
>>
>> On Wed, 10 Feb 2016, Borislav Petkov wrote:
>>
>> > --- a/arch/x86/include/asm/tlbflush.h
>> > +++ b/arch/x86/include/asm/tlbflush.h
>> > @@ -23,7 +23,7 @@ static inline void __invpcid(unsigned long pcid, unsigned long addr,
>> >      * invpcid (%rcx), %rax in long mode.
>> >      */
>> >     asm volatile (".byte 0x66, 0x0f, 0x38, 0x82, 0x01"
>> > -                 : : "m" (desc), "a" (type), "c" (desc) : "memory");
>> > +                 : : "m" (*desc), "a" (type), "c" (desc) : "memory");
>>
>> That still doesn't do what you want.  Arrays in C are funny.  *desc is
>> exactly equivalent to desc[0], _not_ to the whole array,
>
> Doh!
>
>> indeed there's no C syntax to name an lvalue of array type in normal
>> expressions. You need to jump through hoops for this:
>>
>>   "m" (*(struct {unsigned long x[2];} *)desc)
>
> Aha! That's why we wrapped the array in clwb() in a struct too, btw:
>
> static inline void clwb(volatile void *__p)
> {
>         volatile struct { char x[64]; } *p = __p;
>         ...
>
>> It'd probably be easier to simply declare the descriptor as a struct,
>> rather than an array, then the original syntax would have been mostly
>> correct:
>>
>>   struct {u64 d[2];} desc = { pcid, addr };
>>   asm ... "m" (desc), "c" (&desc)
>
> Sounds better. Done. How does that below look like?
>
> Thanks Micha!
>
> ---
> From: Borislav Petkov <bp@...e.de>
> Date: Wed, 10 Feb 2016 12:53:48 +0100
> Subject: [PATCH -v1.1] x86/mm: Fix INVPCID asm constraint
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> So we want to specify the dependency on both @pcid and @addr so that the
> compiler doesn't reorder accesses to them *before* the TLB flush. But
> for that to work, we need to express this properly in the inline asm and
> deref the whole desc array, not the pointer to it. See clwb() for an
> example.
>
> This fixes the build error on 32-bit:
>
>   arch/x86/include/asm/tlbflush.h: In function ‘__invpcid’:
>   arch/x86/include/asm/tlbflush.h:26:18: error: memory input 0 is not directly addressable
>

Acked-by: Andy Lutomirski <luto@...nel.org>