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: <CALCETrW17PWUywMCKWn=KGNLfJQ9kPHXeX2h9sVR47TzyGOJww@mail.gmail.com>
Date:	Tue, 23 Feb 2016 09:47:20 -0800
From:	Andy Lutomirski <luto@...capital.net>
To:	"Ravi V. Shankar" <ravi.v.shankar@...el.com>,
	Toshi Kani <toshi.kani@...com>,
	Brian Gerst <brgerst@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...nel.org>,
	Sai Praneeth Prakhya <sai.praneeth.prakhya@...el.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Luis Rodriguez <mcgrof@...e.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	Matt Fleming <matt@...eblueprint.co.uk>,
	"H. Peter Anvin" <hpa@...or.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Borislav Petkov <bp@...en8.de>, ricardo.neri@...el.com,
	Hugh Dickins <hughd@...gle.com>,
	Ard Biesheuvel <ard.biesheuvel@...aro.org>
Subject: Re: [tip:efi/core] x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page
 table mappings

On Feb 23, 2016 1:09 AM, <&quot;tip-bot for Sai Praneeth
&lt;tipbot@...or.com&gt;&quot;@zytor.com> wrote:

Something's wrong with tip-bot.  This should say:


commit 397630150632639b3ca5b4414accd5011c45e276
Author: Sai Praneeth <sai.praneeth.prakhya@...el.com>
Date:   Wed Feb 17 12:35:56 2016 +0000

    x86/mm/pat: Use _PAGE_GLOBAL bit for EFI page table mappings

    Since EFI page tables can be treated as kernel page tables they should
    be global. All the other page mapping functions in pageattr.c set the
    _PAGE_GLOBAL bit and we want to avoid inconsistencies when we map a page
    in the EFI code paths, for example when that page is split in
    __split_large_page(), etc. It also makes it easier to validate that the
    EFI region mappings have the correct attributes because there are fewer
    differences compared with regular kernel mappings.

But the actual patch is:


@@ -909,6 +909,20 @@ static void populate_pte(struct cpa_data *cpa,

        pte = pte_offset_kernel(pmd, start);

+       /*
+        * Set the GLOBAL flags only if the PRESENT flag is
+        * set otherwise pte_present will return true even on
+        * a non present pte. The canon_pgprot will clear
+        * _PAGE_GLOBAL for the ancient hardware that doesn't
+        * support it.
+        */
+       if (pgprot_val(pgprot) & _PAGE_PRESENT)
+               pgprot_val(pgprot) |= _PAGE_GLOBAL;
+       else
+               pgprot_val(pgprot) &= ~_PAGE_GLOBAL;
+
+       pgprot = canon_pgprot(pgprot);
+

The comment is confusing.  This code is setting GLOBAL if PRESENT is
set even if not requested, but the comment is about setting GLOBAL
*only* if PRESENT is set.

Can you explain:

a) Why this wasn't already broken. (were there no callers who set
GLOBAL but not PRESENT?  If there weren't any, why is that part
needed?)

b) Why setting GLOBAL for EFI mappings is useful.

c) Why setting GLOBAL for EFI mappings is safe.  Don't we unmap the
EFI mappings when we're not actively using them in new kernels?  If
so, don't we explicitly want them *not* to be GLOBAL to avoid needing
an extra-expensive global flush?

d) Why this doesn't break any non-EFI code.

--Andy

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ