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

From: Andy Lutomirski
Date: Sat Dec 16 2017 - 01:41:52 EST


On Fri, Dec 15, 2017 at 2:54 PM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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.