Re: [RFC PTI 1/3] x86/pti: Vastly simplify pgd synchronization

[Andy Lutomirski]

On Fri, Dec 8, 2017 at 12:17 PM, Dave Hansen <dave.hansen@xxxxxxxxx> wrote:
> From a high level, what finally allowed this to happen?  Because
> kpti_add_user_map() all went away, including the LDT one?
>

Yes, exactly.

>
>> +     if (pgdp_maps_userspace(pgdp)) {
>>               /*
>> +              * The user page tables get the full PGD,
>> +              * accessible from userspace:
>>                */
>> +             kernel_to_user_pgdp(pgdp)->pgd = pgd.pgd;
>> +
>> +             /*
>> +              * If this is normal user memory, make it NX in the kernel
>> +              * pagetables so that, if we somehow screw up and return to
>> +              * usermode with the kernel CR3 loaded, we'll get a page
>> +              * fault instead of allowing user code to execute with
>> +              * the wrong CR3.
>> +              *
>> +              * As exceptions, we don't set NX if:
>> +              *  - this is EFI or similar, the kernel may execute from it
>> +              *  - we don't have NX support
>> +              *  - we're clearing the PGD (i.e. pgd.pgd == 0).
>> +              */
>> +             if ((pgd.pgd & _PAGE_USER) && (__supported_pte_mask & _PAGE_NX))
>> +                     pgd.pgd |= _PAGE_NX;
>
> I scratched my head for a sec to realize why you didn't need an explicit
> pgd.pgd==0 check.  It's part of the _PAGE_USER check, which is a bit
> confusing.  Could we do:
>
> if ((pgd.pgd & (_PAGE_USER|_PAGE_PRESENT)) &&
>     (__supported_pte_mask & _PAGE_NX))

I assume you mean pgd.pgd & (_PAGE_USER|_PAGE_PRESENT) ==
(_PAGE_USER|_PAGE_PRESENT)?

>
> And change the comment:
>
>          * As exceptions, we don't set NX fo:
>          *  - PGDs without _PAGE_USER:  Assume this is for a weird in-kernel
>          *    user like EFI from which we may need to execute.
>          *  - PGDs withoout _PAGE_PRESENT: PGD is being cleared, must
>          *    not set _PAGE_NX
>          *  - we don't have NX support
>
>
>
>
>
>>       } else {
>>               /*
>> +              * Changes to the high (kernel) portion of the kernelmode
>> +              * page tables are not automatically propagated to the
>> +              * usermode tables.
>> +              *
>> +              * Users should keep in mind that, unlike the kernelmode
>> +              * tables, there is no vmalloc_fault equivalent for the
>> +              * usermode tables.  Top-level entries added to init_mm's
>> +              * usermode pgd after boot will not be automatically
>> +              * propagated to other mms.
>>                */
>> -             WARN_ON_ONCE(system_state == SYSTEM_RUNNING);
>>       }
>>  #endif
>
> How does the VSYSCALL page get into the user page tables now?

Fantastic question.  I appear to be bad at testing.  Lemme fix that.