Messages in this thread | | | From | Andy Lutomirski <> | Date | Fri, 8 Dec 2017 12:49:18 -0800 | Subject | Re: [RFC PTI 1/3] x86/pti: Vastly simplify pgd synchronization |
| |
On Fri, Dec 8, 2017 at 12:17 PM, Dave Hansen <dave.hansen@intel.com> 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.
| |