linux-kernel - Re: [PATCH PTI v3 10/10] x86/pti: Put the LDT in its own PGD if PTI is on

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]

Date:   Fri, 15 Dec 2017 22:41:23 -0800
From:   Andy Lutomirski <luto@...nel.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Andy Lutomirski <luto@...nel.org>, X86 ML <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Borislav Petkov <bp@...en8.de>,
        Brian Gerst <brgerst@...il.com>,
        David Laight <David.Laight@...lab.com>,
        Kees Cook <keescook@...omium.org>,
        Peter Zijlstra <peterz@...radead.org>,
        "Kirill A. Shutemov" <kirill@...temov.name>,
        Dave Hansen <dave.hansen@...el.com>
Subject: Re: [PATCH PTI v3 10/10] x86/pti: Put the LDT in its own PGD if PTI
 is on

On Fri, Dec 15, 2017 at 2:54 PM, Thomas Gleixner <tglx@...utronix.de> wrote:
> On Tue, 12 Dec 2017, Andy Lutomirski wrote:
>> +/* This is a multiple of PAGE_SIZE. */
>> +#define LDT_SLOT_STRIDE (LDT_ENTRIES * LDT_ENTRY_SIZE)
>> +
>> +static void *ldt_slot_va(int slot)
>
> How is that supposed to compile w/o warnings? Want's to be inline ....

Good question.  But it does compile for me and apparently for the 0day
bot without warnings.  Fixed in my tree.

On Fri, Dec 15, 2017 at 4:39 PM, Thomas Gleixner <tglx@...utronix.de> wrote:
> On Tue, 12 Dec 2017, Andy Lutomirski wrote:
>> +
>> +     return 0;
>> +#else
>> +     return -EINVAL;
>
> Errm. What's the point of that? Breaking non PTI?

That's embarrassing.

>
>>       new_ldt = alloc_ldt_struct(old_mm->context.ldt->nr_entries);
>>       if (!new_ldt) {
>> @@ -155,8 +266,17 @@ int init_new_context_ldt(struct task_struct *tsk, struct mm_struct *mm)
>>       memcpy(new_ldt->entries, old_mm->context.ldt->entries,
>>              new_ldt->nr_entries * LDT_ENTRY_SIZE);
>>       finalize_ldt_struct(new_ldt);
>> +     retval = map_ldt_struct(mm, new_ldt, 0);
>> +     if (retval)
>> +             goto out_free;
>>
>>       mm->context.ldt = new_ldt;
>> +     goto out_unlock;
>> +
>> +out_free:
>> +     free_ldt_pgtables(mm);
>> +     free_ldt_struct(new_ldt);
>> +     return retval;
>
> Leaks old_mm->context_lock;
>

Indeed.

I'm going to go test the failure paths better tomorrow.  Meanwhile,
all three issues should be fixed in my tree.