linux-kernel - Re: [PATCH] x86, mm: make alternatives code do stronger TLB flush

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: <CALCETrVw5nJoK99FQ+n4SiJPKEQ6umDBYat9zesaxFLLcE+yZg@mail.gmail.com>
Date:   Wed, 1 Nov 2017 01:12:24 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Dave Hansen <dave.hansen@...ux.intel.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>, X86 ML <x86@...nel.org>,
        Andrew Lutomirski <luto@...nel.org>
Subject: Re: [PATCH] x86, mm: make alternatives code do stronger TLB flush

On Tue, Oct 31, 2017 at 11:07 AM, Dave Hansen
<dave.hansen@...ux.intel.com> wrote:
>
> From: Dave Hansen <dave.hansen@...ux.intel.com>
>
> local_flush_tlb() does a CR3 write.  But, that kind of TLB flush is
> not guaranteed to invalidate global pages.  The entire kernel is
> mapped with global pages.
>
> Also, now that we have PCIDs, local_flush_tlb() will only flush the
> *current* PCID.  It would not flush the entries for all PCIDs.
> At the moment, this is a moot point because all kernel pages are
> _PAGE_GLOBAL which do not really *have* a particular PCID.
>
> Use the stronger __flush_tlb_all() which does flush global pages.
>
> This was found because of a warning I added to __native_flush_tlb()
> to look for calls to it when PCIDs are enabled.  This patch does
> not fix any bug known to be hit in practice.

I'm very confused here.  set_fixmap() does a flush.  clear_fixmap()
calls set_fixmap() and therefore also flushes.  So I don't see why the
flush you're modifying is needed at all.  Could you just delete it
instead?

If your KAISER series were applied, then the situation is slightly
different.  We have this code:

static void __set_pte_vaddr(pud_t *pud, unsigned long vaddr, pte_t new_pte)
{
        pmd_t *pmd = fill_pmd(pud, vaddr);
        pte_t *pte = fill_pte(pmd, vaddr);

        set_pte(pte, new_pte);

        /*
         * It's enough to flush this one mapping.
         * (PGE mappings get flushed as well)
         */
        __flush_tlb_one(vaddr);
}

and that is no longer correct.  You may need to add a helper
__flush_tlb_kernel_one() that does the right thing.  For the
alternatives case, you could skip it since you know that the mapping
never got propagated to any other PCID slot on the current CPU, but
that's probably not worth trying to optimize.

--Andy