linux-kernel - Re: [PATCH v2] x86/mm/pti: remove dead logic during user pagetable population

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:   Sun, 7 Jan 2018 10:34:11 -0800
From:   Dave Hansen <dave.hansen@...el.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...en8.de>
Cc:     Jike Song <albcamus@...il.com>, linux-kernel@...r.kernel.org,
        David Woodhouse <dwmw@...zon.co.uk>,
        Alan Cox <gnomes@...rguk.ukuu.org.uk>,
        Jiri Koshina <jikos@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        Andi Lutomirski <luto@...capital.net>,
        Andi Kleen <ak@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Paul Turner <pjt@...gle.com>,
        Tom Lendacky <thomas.lendacky@....com>,
        Greg KH <gregkh@...ux-foundation.org>,
        Kees Cook <keescook@...gle.com>, stable@...r.kernel.org
Subject: Re: [PATCH v2] x86/mm/pti: remove dead logic during user pagetable
 population

On 01/07/2018 04:05 AM, Thomas Gleixner wrote:
>> static DEFINE_SPINLOCK(shadow_table_allocation_lock);
>>
>> Now I have my suspicions why that's not needed anymore upstream but I'd
>> let tglx explain better.
> We got rid of all that runtime mapping stuff and the functions are only
> called from pti_init(). So the locking and therefor the tests above are not
> needed anymore. While at it we should mark all those function __init.

Yes, the double-test was part of an optimization where we attempted to
avoid using a global spinlock in the fork() path.  We would check for
unallocated mid-level page tables without the lock.  The lock was only
taken it when we needed to *make* an entry to avoid collisions.

Now that it is all single-threaded, there is no chance of a collision,
no need for a lock, and no need for the re-check.

^^ Just in case someone wants to include a bit more first-hand
information about wtf that code was doing there.