linux-kernel - Re: [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems

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:   Fri, 23 Jun 2017 08:28:05 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Andy Lutomirski <luto@...nel.org>, X86 ML <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Mel Gorman <mgorman@...e.de>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        Nadav Amit <nadav.amit@...il.com>,
        Rik van Riel <riel@...hat.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Arjan van de Ven <arjan@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Juergen Gross <jgross@...e.com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>
Subject: Re: [PATCH v3 10/11] x86/mm: Enable CR4.PCIDE on supported systems

On Fri, Jun 23, 2017 at 4:50 AM, Borislav Petkov <bp@...en8.de> wrote:
> On Tue, Jun 20, 2017 at 10:22:16PM -0700, Andy Lutomirski wrote:
>> We can use PCID if the CPU has PCID and PGE and we're not on Xen.
>>
>> By itself, this has no effect.  The next patch will start using
>> PCID.
>>
>> Cc: Juergen Gross <jgross@...e.com>
>> Cc: Boris Ostrovsky <boris.ostrovsky@...cle.com>
>> Signed-off-by: Andy Lutomirski <luto@...nel.org>
>> ---
>>  arch/x86/include/asm/tlbflush.h |  8 ++++++++
>>  arch/x86/kernel/cpu/common.c    | 15 +++++++++++++++
>>  arch/x86/xen/enlighten_pv.c     |  6 ++++++
>>  3 files changed, 29 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
>> index 87b13e51e867..57b305e13c4c 100644
>> --- a/arch/x86/include/asm/tlbflush.h
>> +++ b/arch/x86/include/asm/tlbflush.h
>> @@ -243,6 +243,14 @@ static inline void __flush_tlb_all(void)
>>               __flush_tlb_global();
>>       else
>>               __flush_tlb();
>> +
>> +     /*
>> +      * Note: if we somehow had PCID but not PGE, then this wouldn't work --
>> +      * we'd end up flushing kernel translations for the current ASID but
>> +      * we might fail to flush kernel translations for other cached ASIDs.
>> +      *
>> +      * To avoid this issue, we force PCID off if PGE is off.
>> +      */
>>  }
>>
>>  static inline void __flush_tlb_one(unsigned long addr)
>> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
>> index 904485e7b230..01caf66b270f 100644
>> --- a/arch/x86/kernel/cpu/common.c
>> +++ b/arch/x86/kernel/cpu/common.c
>> @@ -1143,6 +1143,21 @@ static void identify_cpu(struct cpuinfo_x86 *c)
>>       setup_smep(c);
>>       setup_smap(c);
>>
>> +     /* Set up PCID */
>> +     if (cpu_has(c, X86_FEATURE_PCID)) {
>> +             if (cpu_has(c, X86_FEATURE_PGE)) {
>
> What are we protecting ourselves here against? Funny virtualization guests?
>
> Because PGE should be ubiquitous by now. Or have you heard something?

Yes, funny VM guests.  I've been known to throw weird options at qemu
myself, and I prefer when the system works.  In this particular case,
I think the failure mode would be stale kernel TLB entries, and that
would be really annoying.

>
>> +                     cr4_set_bits(X86_CR4_PCIDE);
>> +             } else {
>> +                     /*
>> +                      * flush_tlb_all(), as currently implemented, won't
>> +                      * work if PCID is on but PGE is not.  Since that
>> +                      * combination doesn't exist on real hardware, there's
>> +                      * no reason to try to fully support it.
>> +                      */
>> +                     clear_cpu_cap(c, X86_FEATURE_PCID);
>> +             }
>> +     }
>
> This whole in setup_pcid() I guess, like the rest of the features.

Done.