Clean up pgd handling and fix VSYSCALL and LDT
Subsystems (5)
Andy Lutomirski
maintainerv3 cover Clean up pgd handling and fix VSYSCALL and LDT
This should fix some existing 5-level bugs and get VSYSCALL and LDT working with PTI.
Changes from v1:
- vsyscalls actually work.
- Added the "Warn and fail" patch to prevent the testing goof I had on v1.
- Lots of cleanups
Andy Lutomirski (10):
x86/espfix/64: Fix espfix double-fault handling on 5-level systems x86/pti: Vastly simplify pgd synchronization
x86/pti/64: Fix ESPFIX64 user mapping
Revert "x86/mm/pti: Disable native VSYSCALL" x86/vsyscall/64: Explicitly set _PAGE_USER in the pagetable hierarchy x86/vsyscall/64: Warn and fail vsyscall emulation in NATIVE mode x86/pti: Map the vsyscall page if needed
x86/mm/64: Improve the memory map documentation
x86/mm/64: Make a full PGD-entry size hole in the memory map x86/pti: Put the LDT in its own PGD if PTI is on
Documentation/x86/x86_64/mm.txt | 15 +-- arch/x86/Kconfig | 8 -- arch/x86/entry/vsyscall/vsyscall_64.c | 37 +++++++- arch/x86/include/asm/mmu_context.h | 48 +++++++++- arch/x86/include/asm/pgtable.h | 6 +- arch/x86/include/asm/pgtable_64.h | 77 +++++++--------- arch/x86/include/asm/pgtable_64_types.h | 8 +- arch/x86/include/asm/processor.h | 23 +++-- arch/x86/include/asm/vsyscall.h | 1 + arch/x86/kernel/espfix_64.c | 16 ---- arch/x86/kernel/ldt.c | 155 +++++++++++++++++++++++++++++-- arch/x86/kernel/traps.c | 2 +- arch/x86/mm/dump_pagetables.c | 12 +++ arch/x86/mm/pti.c | 157 ++++++++++++++++++++++---------- init/main.c | 11 ++- 15 files changed, 426 insertions(+), 150 deletions(-)
Signature · 1 line
2.13.6
Quoted 20 lines · Andy Lutomirski
This should fix some existing 5-level bugs and get VSYSCALL and LDT working with PTI.
Changes from v1:
- vsyscalls actually work.
- Added the "Warn and fail" patch to prevent the testing goof I had on v1.
- Lots of cleanupsAndy Lutomirski (10):
x86/espfix/64: Fix espfix double-fault handling on 5-level systems x86/pti: Vastly simplify pgd synchronization
x86/pti/64: Fix ESPFIX64 user mapping
Revert "x86/mm/pti: Disable native VSYSCALL" x86/vsyscall/64: Explicitly set_PAGE_USERin the pagetable hierarchy x86/vsyscall/64: Warn and fail vsyscall emulation in NATIVE mode x86/pti: Map the vsyscall page if needed
x86/mm/64: Improve the memory map documentation
x86/mm/64: Make a full PGD-entry size hole in the memory map x86/pti: Put the LDT in its own PGD if PTI is on
Quoted 16 lines
Documentation/x86/x86_64/mm.txt | 15 +-- arch/x86/Kconfig | 8 -- arch/x86/entry/vsyscall/vsyscall_64.c | 37 +++++++- arch/x86/include/asm/mmu_context.h | 48 +++++++++- arch/x86/include/asm/pgtable.h | 6 +- arch/x86/include/asm/pgtable_64.h | 77 +++++++--------- arch/x86/include/asm/pgtable_64_types.h | 8 +- arch/x86/include/asm/processor.h | 23 +++-- arch/x86/include/asm/vsyscall.h | 1 + arch/x86/kernel/espfix_64.c | 16 ---- arch/x86/kernel/ldt.c | 155 +++++++++++++++++++++++++++++-- arch/x86/kernel/traps.c | 2 +- arch/x86/mm/dump_pagetables.c | 12 +++ arch/x86/mm/pti.c | 157 ++++++++++++++++++++++---------- init/main.c | 11 ++- 15 files changed, 426 insertions(+), 150 deletions(-)
Hm, I only received the 0/10 boilerplate email, not any of the patches - and Thomas tells me he too only got the cover letter.
Thanks,
Ingo
Andy Lutomirski
maintainerQuoted 40 lines · Ingo Molnar
Quoted 20 lines · Andy Lutomirski
This should fix some existing 5-level bugs and get VSYSCALL and LDT working with PTI.
Changes from v1:
- vsyscalls actually work.
- Added the "Warn and fail" patch to prevent the testing goof I had on v1.
- Lots of cleanupsAndy Lutomirski (10):
x86/espfix/64: Fix espfix double-fault handling on 5-level systems x86/pti: Vastly simplify pgd synchronization
x86/pti/64: Fix ESPFIX64 user mapping
Revert "x86/mm/pti: Disable native VSYSCALL" x86/vsyscall/64: Explicitly set_PAGE_USERin the pagetable hierarchy x86/vsyscall/64: Warn and fail vsyscall emulation in NATIVE mode x86/pti: Map the vsyscall page if needed
x86/mm/64: Improve the memory map documentation
x86/mm/64: Make a full PGD-entry size hole in the memory map x86/pti: Put the LDT in its own PGD if PTI is onQuoted 16 lines
Documentation/x86/x86_64/mm.txt | 15 +-- arch/x86/Kconfig | 8 -- arch/x86/entry/vsyscall/vsyscall_64.c | 37 +++++++- arch/x86/include/asm/mmu_context.h | 48 +++++++++- arch/x86/include/asm/pgtable.h | 6 +- arch/x86/include/asm/pgtable_64.h | 77 +++++++--------- arch/x86/include/asm/pgtable_64_types.h | 8 +- arch/x86/include/asm/processor.h | 23 +++-- arch/x86/include/asm/vsyscall.h | 1 + arch/x86/kernel/espfix_64.c | 16 ---- arch/x86/kernel/ldt.c | 155 +++++++++++++++++++++++++++++-- arch/x86/kernel/traps.c | 2 +- arch/x86/mm/dump_pagetables.c | 12 +++ arch/x86/mm/pti.c | 157 ++++++++++++++++++++++---------- init/main.c | 11 ++- 15 files changed, 426 insertions(+), 150 deletions(-)
Quoted 2 lines
Hm, I only received the 0/10 boilerplate email, not any of the patches - and Thomas tells me he too only got the cover letter.
Ugh, the whole send-email process exploded due to Cc:
[email protected] # 4.14. I stripped the # 4.14 and it's okay now.
Quoted 4 lines
Thanks,
Ingo
Borislav Petkov
maintainerQuoted 3 lines · Andy Lutomirski
Ugh, the whole send-email process exploded due to Cc:
[email protected] # 4.14. I stripped the # 4.14 and it's okay now.
In the future, strip the whole CC: stable directly. Although git send-email with --suppress-cc is kinda clumsy for that... or I haven't figured it out properly.
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.Quoted 9 lines · Borislav Petkov
Quoted 3 lines · Andy Lutomirski
Ugh, the whole send-email process exploded due to Cc:
[email protected] # 4.14. I stripped the # 4.14 and it's okay now.In the future, strip the whole CC: stable directly. Although git send-email with --suppress-cc is kinda clumsy for that... or I haven't figured it out properly.
Using
works for me.
Juergen
Borislav Petkov
maintainerQuoted 5 lines · Juergen Gross
Using
Cc <[email protected]> # 4.14works for me.
... except that you should not CC: stable on submission.
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.Andy Lutomirski
maintainerPATCH v3 1/10 x86/espfix/64: Fix espfix double-fault handling on 5-level systems
In reply to Andy LutomirskiUsing PGDIR_SHIFT to identify espfix64 addresses on 5-level systems was wrong, and it resulted in panics due to unhandled double faults. Use P4D_SHIFT instead, which is correct on 4-level and 5-level machines.
This fixes a panic when running x86 selftests on 5-level machines.
--- arch/x86/kernel/traps.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kernel/traps.c +1 -1
@@ -360,7 +360,7 @@ dotraplinkage void do_double_fault(struct pt_regs *regs, long error_code)* * No need for ist_enter here because we don't use RCU. */ -if (((long )regs->sp >>PGDIR_SHIFT ) ==ESPFIX_PGD_ENTRY && +if (((long )regs->sp >>P4D_SHIFT ) ==ESPFIX_PGD_ENTRY && regs->cs == __KERNEL_CS && regs->ip == (unsigned long )native_irq_return_iret) {
Signature · 1 line
2.13.6
Andy Lutomirski
maintainerPATCH v3 2/10 x86/pti: Vastly simplify pgd synchronization
Back when we would dynamically add mappings to the usermode tables, we needed to preallocate all the high top-level entries in the usermode tables. We don't need this in recent versions of PTI, so get rid of preallocation.
With preallocation gone, the comments in pti_set_user_pgd() make even less sense. Rearrange the function to make it entirely obvious what it does and does not do. FWIW, I haven't even tried to wrap my head around the old logic, since it seemed to be somewhere between incomprehensible and wrong.
I admit that a bit of the earlier complexity was based on my suggestions. Mea culpa.
--- arch/x86/include/asm/pgtable_64.h | 74 +++++++++++++++------------------------ arch/x86/mm/pti.c | 52 ++++++--------------------- 2 files changed, 39 insertions(+), 87 deletions(-)
arch/x86/include/asm/pgtable_64.h +29 -45
@@ -195,14 +195,6 @@ static inline bool pgdp_maps_userspace(void *__ptr) }/* -* Does this PGD allow access from userspace? -*/ -static inline bool pgd_userspace_access (pgd_t pgd) -{ -return pgd.pgd & _PAGE_USER; -} - -/* * Take a PGD location (pgdp) and a pgd value that needs to be set there. * Populates the user and returns the resulting PGD that must be set in * the kernel copy of the page tables.
@@ -213,50 +205,42 @@ static inline pgd_t pti_set_user_pgd(pgd_t *pgdp, pgd_t pgd)if (!static_cpu_has_bug (X86_BUG_CPU_SECURE_MODE_PTI ))return pgd; -if (pgd_userspace_access (pgd)) { -if (pgdp_maps_userspace (pgdp)) { -/* -* The user page tables get the full PGD, -* accessible from userspace: -*/ -kernel_to_user_pgdp (pgdp)->pgd = pgd.pgd; -/* -* For the copy of the pgd that the kernel uses, -* make it unusable to userspace. This ensures on -* in case that a return to userspace with the -* kernel CR3 value, userspace will crash instead -* of running. -* -* Note: NX might be not available or disabled. -*/ -if (__supported_pte_mask & _PAGE_NX) - pgd.pgd |= _PAGE_NX; - } - }else if (pgd_userspace_access (*pgdp)) { +if (pgdp_maps_userspace (pgdp)) {/* -* We are clearing a _PAGE_USER PGD for which we presumably -* populated the user PGD. We must now clear the user PGD -* entry. +* The user page tables get the full PGD, +* accessible from userspace: */ -if (pgdp_maps_userspace (pgdp)) { -kernel_to_user_pgdp (pgdp)->pgd = pgd.pgd; - }else { -/* -* Attempted to clear a _PAGE_USER PGD which is in -* the kernel portion of the address space. PGDs -* are pre-populated and we never clear them. -*/ -WARN_ON_ONCE (1 ); - } +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; }else {/* -* _PAGE_USER was not set in either the PGD being set or -* cleared. All kernel PGDs should be pre-populated so -* this should never happen after boot. +* 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 +/* return the copy of the PGD we want the kernel to use: */ return pgd; }
arch/x86/mm/pti.c +10 -42
70 lines not on this page · raw message
Signature · 1 line
2.13.6
Andy Lutomirski
maintainerPATCH v3 3/10 x86/pti/64: Fix ESPFIX64 user mapping
In reply to Andy LutomirskiThe ESPFIX64 user mapping belongs in pti.c just like all the other user mappings. Move it there and make it work correctly while we're at it.
--- arch/x86/kernel/espfix_64.c | 16 ---------------- arch/x86/mm/pti.c | 42 +++++++++++++++++++++++++++++++++++++----- init/main.c | 11 +++++++---- 3 files changed, 44 insertions(+), 25 deletions(-)
arch/x86/kernel/espfix_64.c +0 -16
@@ -129,22 +129,6 @@ void __init init_espfix_bsp(void) p4d =p4d_alloc (&init_mm, pgd,ESPFIX_BASE_ADDR );p4d_populate (&init_mm, p4d, espfix_pud_page); -/* -* Just copy the top-level PGD that is mapping the espfix area to -* ensure it is mapped into the user page tables. -* -* For 5-level paging, the espfix pgd was populated when -* pti_init() pre-populated all the pgd entries. The above -* p4d_alloc() would never do anything and the p4d_populate() would -* be done to a p4d already mapped in the userspace pgd. -*/ -#ifdef CONFIG_PAGE_TABLE_ISOLATION -if (CONFIG_PGTABLE_LEVELS <=4 ) { -set_pgd (kernel_to_user_pgdp (pgd), -__pgd (_KERNPG_TABLE | (p4d_pfn (*p4d) <<PAGE_SHIFT ))); - } -#endif -/* Randomize the locations */ init_espfix_random ();
arch/x86/mm/pti.c +37 -5
68 lines not on this page · raw message
init/main.c +7 -4
@@ -505,6 +505,13 @@ static void __init mm_init(void)pgtable_init ();vmalloc_init ();ioremap_huge_init (); + +#ifdef CONFIG_X86_ESPFIX64 +/* Should be run before the first non-init thread is created */ +init_espfix_bsp (); +#endif + +/* Should be run after espfix64 is set up. */ pti_init (); }
@@ -676,10 +683,6 @@ asmlinkage __visible void __init start_kernel(void)if (efi_enabled (EFI_RUNTIME_SERVICES ))efi_enter_virtual_mode ();#endif -#ifdef CONFIG_X86_ESPFIX64 -/* Should be run before the first non-init thread is created */ -init_espfix_bsp (); -#endif thread_stack_cache_init ();cred_init ();fork_init ();
Signature · 1 line
2.13.6
Re: [PATCH PTI v3 03/10] x86/pti/64: Fix ESPFIX64 user mapping
Quoted 5 lines · Andy Lutomirski
The ESPFIX64 user mapping belongs in
pti.cjust like all the other user mappings. Move it there and make it work correctly while we're at it.Signed-off-by Andy Lutomirski <[email protected]>
BTW, why do we open-code p?d_alloc() in pti_user_pagetable_walk_*()? It seems unnecessary and potentially bogus: see smp_wmb() in __p?d_alloc() helpers.
Signature · 1 line
Kirill A. Shutemov
Andy Lutomirski
maintainerRe: [PATCH PTI v3 03/10] x86/pti/64: Fix ESPFIX64 user mapping
Quoted 5 lines · Kirill A. Shutemov
Quoted 4 lines · Andy Lutomirski
The ESPFIX64 user mapping belongs in
pti.cjust like all the other user mappings. Move it there and make it work correctly while we're at it.
Quoted 7 lines
Quoted 1 line
Signed-off-by Andy Lutomirski <[email protected]>BTW, why do we open-code p?d_alloc() in pti_user_pagetable_walk_*()? It seems unnecessary and potentially bogus: see
smp_wmb()in __p?d_alloc() helpers.
The helpers won't work -- we're allocating kernel-owned tables in the usermode part of init_mm. The p?d_alloc() helpers allocate user-accounted tables in the kernelmode part of the mm.
--Andy
Re: [PATCH PTI v3 03/10] x86/pti/64: Fix ESPFIX64 user mapping
Quoted 11 lines · Andy Lutomirski
Quoted 9 lines · Kirill A. Shutemov
Quoted 5 lines · Andy Lutomirski
The ESPFIX64 user mapping belongs in
pti.cjust like all the other user mappings. Move it there and make it work correctly while we're at it.Signed-off-by Andy Lutomirski <[email protected]>
Quoted 7 lines
Quoted 3 lines
BTW, why do we open-code p?d_alloc() in pti_user_pagetable_walk_*()? It seems unnecessary and potentially bogus: see
smp_wmb()in __p?d_alloc() helpers.The helpers won't work -- we're allocating kernel-owned tables in the usermode part of
init_mm. The p?d_alloc() helpers allocate user-accounted tables in the kernelmode part of the mm.
What's wrong to account them against init_mm?
Signature · 1 line
Kirill A. Shutemov
Andy Lutomirski
maintainerRe: [PATCH PTI v3 03/10] x86/pti/64: Fix ESPFIX64 user mapping
Quoted 16 lines · Kirill A. Shutemov
Quoted 15 lines · Andy Lutomirski
Quoted 12 lines · Kirill A. Shutemov
Quoted 5 lines · Andy Lutomirski
The ESPFIX64 user mapping belongs in
pti.cjust like all the other user mappings. Move it there and make it work correctly while we're at it.Signed-off-by Andy Lutomirski <[email protected]>BTW, why do we open-code p?d_alloc() in pti_user_pagetable_walk_*()? It seems unnecessary and potentially bogus: see
smp_wmb()in __p?d_alloc() helpers.
Quoted 6 lines
Quoted 3 lines
The helpers won't work -- we're allocating kernel-owned tables in the usermode part of
init_mm. The p?d_alloc() helpers allocate user-accounted tables in the kernelmode part of the mm.What's wrong to account them against
init_mm?
Nothing. But the other parts are showstoppers: the helpers won't set _PAGE_USER in the higher level entries and they will put the pgds in the wrong place.
Andy Lutomirski
maintainerPATCH v3 4/10 Revert "x86/mm/pti: Disable native VSYSCALL"
In reply to Andy LutomirskiThis reverts commit 6a7b4041b853ecc653e2c1dda5b736ab5fd29357.
With the PGD-propagation logic simplified, there's no need for this.
--- arch/x86/Kconfig | 8 -------- 1 file changed, 8 deletions(-)
arch/x86/Kconfig +0 -8
@@ -2233,9 +2233,6 @@ choice config LEGACY_VSYSCALL_NATIVE bool "Native" - # The VSYSCALL page comes from the kernel page tables - # and is not available when PAGE_TABLE_ISOLATION is enabled. - depends on !PAGE_TABLE_ISOLATION help Actual executable code is located in the fixed vsyscall address mapping, implementing time() efficiently. Since
@@ -2253,11 +2250,6 @@ choice exploits. This configuration is recommended when userspace still uses the vsyscall area. - When PAGE_TABLE_ISOLATION is enabled, the vsyscall area will become - unreadable. This emulation option still works, but PAGE_TABLE_ISOLATION - will make it harder to do things like trace code using the - emulation. - config LEGACY_VSYSCALL_NONE bool "None" help
Signature · 1 line
2.13.6
Andy Lutomirski
maintainerPATCH v3 5/10 x86/vsyscall/64: Explicitly set _PAGE_USER in the pagetable hierarchy
In reply to Andy LutomirskiThe kernel is very erratic as to which pagetables have _PAGE_USER set. The vsyscall page gets lucky: it seems that all of the relevant pagetables are among the apparently arbitrary ones that set _PAGE_USER. Rather than relying on chance, just explicitly set _PAGE_USER.
This will let us clean up pagetable setup to stop setting _PAGE_USER. The added code can also be reused by pagetable isolation to manage the _PAGE_USER bit in the usermode tables.
--- arch/x86/entry/vsyscall/vsyscall_64.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)
arch/x86/entry/vsyscall/vsyscall_64.c +32 -1
@@ -329,16 +329,47 @@ int in_gate_area_no_mm(unsigned long addr)return vsyscall_mode !=NONE && (addr &PAGE_MASK ) ==VSYSCALL_ADDR ; } +/* +* The VSYSCALL page is the only user-accessible page in the kernel address +* range. Normally, the kernel page tables can have _PAGE_USER clear, but +* the tables covering VSYSCALL_ADDR need _PAGE_USER set if vsyscalls +* are enabled. +* +* Some day we may create a "minimal" vsyscall mode in which we emulate +* vsyscalls but leave the page not present. If so, we skip calling +* this. +*/ +static void __init set_vsyscall_pgtable_user_bits (void ) +{ +pgd_t *pgd; +p4d_t *p4d; +pud_t *pud; +pmd_t *pmd; + + pgd =pgd_offset_k (VSYSCALL_ADDR ); + pgd->pgd |= _PAGE_USER; + p4d =p4d_offset (pgd,VSYSCALL_ADDR ); +#if CONFIG_PGTABLE_LEVELS >=5 + p4d->p4d |= _PAGE_USER; +#endif + pud =pud_offset (p4d,VSYSCALL_ADDR ); + pud->pud |= _PAGE_USER; + pmd =pmd_offset (pud,VSYSCALL_ADDR ); + pmd->pmd |= _PAGE_USER; +} +void __init map_vsyscall (void ) {extern char __vsyscall_page;unsigned long physaddr_vsyscall =__pa_symbol (&__vsyscall_page); -if (vsyscall_mode !=NONE ) +if (vsyscall_mode !=NONE ) {__set_fixmap (VSYSCALL_PAGE , physaddr_vsyscall, vsyscall_mode ==NATIVE ?PAGE_KERNEL_VSYSCALL :PAGE_KERNEL_VVAR ); +set_vsyscall_pgtable_user_bits (); + }BUILD_BUG_ON ((unsigned long )__fix_to_virt (VSYSCALL_PAGE ) != (unsigned long )VSYSCALL_ADDR );
Signature · 1 line
2.13.6
Andy Lutomirski
maintainerPATCH v3 6/10 x86/vsyscall/64: Warn and fail vsyscall emulation in NATIVE mode
In reply to Andy LutomirskiIf something goes wrong with pagetable setup, vsyscall=native will accidentally fall back to emulation. Make it warn and fail so that we notice.
--- arch/x86/entry/vsyscall/vsyscall_64.c | 4 ++++ 1 file changed, 4 insertions(+)
arch/x86/entry/vsyscall/vsyscall_64.c +4 -0
@@ -138,6 +138,10 @@ bool emulate_vsyscall(struct pt_regs *regs, unsigned long address)WARN_ON_ONCE (address != regs->ip); +/* This should be unreachable in NATIVE mode. */ +if (WARN_ON (vsyscall_mode ==NATIVE )) +return false; +if (vsyscall_mode ==NONE ) {warn_bad_vsyscall (KERN_INFO , regs,"vsyscall attempted with vsyscall=none" );
Signature · 1 line
2.13.6
Andy Lutomirski
maintainerPATCH v3 7/10 x86/pti: Map the vsyscall page if needed
In reply to Andy LutomirskiMake VSYSCALLs work fully in PTI mode.
--- arch/x86/entry/vsyscall/vsyscall_64.c | 6 ++-- arch/x86/include/asm/pgtable.h | 6 +++- arch/x86/include/asm/pgtable_64.h | 9 +++-- arch/x86/include/asm/vsyscall.h | 1 + arch/x86/mm/pti.c | 63 +++++++++++++++++++++++++++++++++++ 5 files changed, 78 insertions(+), 7 deletions(-)
arch/x86/entry/vsyscall/vsyscall_64.c +3 -3
@@ -343,14 +343,14 @@ int in_gate_area_no_mm(unsigned long addr)* vsyscalls but leave the page not present. If so, we skip calling * this. */ -static void __init set_vsyscall_pgtable_user_bits (void ) +void __init set_vsyscall_pgtable_user_bits (pgd_t *root) {pgd_t *pgd;p4d_t *p4d;pud_t *pud;pmd_t *pmd; - pgd =pgd_offset_k (VSYSCALL_ADDR ); + pgd =pgd_offset_pgd (root,VSYSCALL_ADDR ); pgd->pgd |= _PAGE_USER; p4d =p4d_offset (pgd,VSYSCALL_ADDR );#if CONFIG_PGTABLE_LEVELS >=5
@@ -372,7 +372,7 @@ void __init map_vsyscall(void) vsyscall_mode ==NATIVE ?PAGE_KERNEL_VSYSCALL : PAGE_KERNEL_VVAR); -set_vsyscall_pgtable_user_bits (); +set_vsyscall_pgtable_user_bits (swapper_pg_dir); } BUILD_BUG_ON((unsigned long)__fix_to_virt(VSYSCALL_PAGE) !=
arch/x86/include/asm/pgtable.h +5 -1
@@ -920,7 +920,11 @@ static inline int pgd_none(pgd_t pgd)* pgd_offset() returns a (pgd_t *) * pgd_index() is used get the offset into the pgd page's array of pgd_t's; */ -#define pgd_offset (mm, address) ((mm)->pgd + pgd_index((address))) +#define pgd_offset_pgd (pgd, address) (pgd + pgd_index((address))) +/* +* a shortcut to get a pgd_t in a given mm +*/ +#define pgd_offset (mm, address) pgd_offset_pgd((mm)->pgd, (address))/* * a shortcut which implies the use of the kernel's pgd, instead * of a process's
arch/x86/include/asm/pgtable_64.h +6 -3
@@ -220,11 +220,14 @@ static inline pgd_t pti_set_user_pgd(pgd_t *pgdp, pgd_t pgd)* the wrong CR3. * * As exceptions, we don't set NX if: -* - this is EFI or similar, the kernel may execute from it +* - _PAGE_USER is not set. This could be an executable +* EFI runtime mapping or something similar, and the kernel +* may execute from it * - we don't have NX support -* - we're clearing the PGD (i.e. pgd.pgd == 0). +* - we're clearing the PGD (i.e. the new pgd is not present). */ -if ((pgd.pgd & _PAGE_USER) && (__supported_pte_mask & _PAGE_NX)) +if ((pgd.pgd & (_PAGE_USER|_PAGE_PRESENT)) == (_PAGE_USER|_PAGE_PRESENT) && + (__supported_pte_mask & _PAGE_NX)) pgd.pgd |= _PAGE_NX; }else {/*
arch/x86/include/asm/vsyscall.h +1 -0
@@ -7,6 +7,7 @@#ifdef CONFIG_X86_VSYSCALL_EMULATION extern void map_vsyscall (void ); +extern void set_vsyscall_pgtable_user_bits (pgd_t *root);/* * Called on instruction fetch fault in vsyscall page.
arch/x86/mm/pti.c +63 -0
85 lines not on this page · raw message
Signature · 1 line
2.13.6
Andy Lutomirski
maintainerPATCH v3 8/10 x86/mm/64: Improve the memory map documentation
In reply to Andy LutomirskiThe old docs had the vsyscall range wrong* and were missing the fixmap. Fix both.
* There used to be 8 MB reserved for future vsyscalls, but that's long gone.
--- Documentation/x86/x86_64/mm.txt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
Documentation/x86/x86_64/mm.txt +6 -4
@@ -17,8 +17,9 @@ ffffff0000000000 - ffffff7fffffffff (=39 bits) %esp fixup stacks ffffffef00000000 - fffffffeffffffff (=64 GB) EFI region mapping space ... unused hole ... ffffffff80000000 - ffffffff9fffffff (=512 MB) kernel text mapping, from phys 0 -ffffffffa0000000 - ffffffffff5fffff (=1526 MB) module mapping space (variable) -ffffffffff600000 - ffffffffffdfffff (=8 MB) vsyscalls +ffffffffa0000000 - [fixmap start] (~1526 MB) module mapping space (variable) +[fixmap start] - ffffffffff5fffff kernel-internal fixmap range +ffffffffff600000 - ffffffffff600fff (=4 kB) legacy vsyscall ABI ffffffffffe00000 - ffffffffffffffff (=2 MB) unused hole Virtual memory map with 5 level page tables:
@@ -39,8 +40,9 @@ ffffff0000000000 - ffffff7fffffffff (=39 bits) %esp fixup stacks ffffffef00000000 - fffffffeffffffff (=64 GB) EFI region mapping space ... unused hole ... ffffffff80000000 - ffffffff9fffffff (=512 MB) kernel text mapping, from phys 0 -ffffffffa0000000 - ffffffffff5fffff (=1526 MB) module mapping space -ffffffffff600000 - ffffffffffdfffff (=8 MB) vsyscalls +ffffffffa0000000 - [fixmap start] (~1526 MB) module mapping space +[fixmap start] - ffffffffff5fffff kernel-internal fixmap range +ffffffffff600000 - ffffffffff600fff (=4 kB) legacy vsyscall ABI ffffffffffe00000 - ffffffffffffffff (=2 MB) unused hole Architecture defines a 64-bit virtual address. Implementations can support
Signature · 1 line
2.13.6
Andy Lutomirski
maintainerPATCH v3 9/10 x86/mm/64: Make a full PGD-entry size hole in the memory map
In reply to Andy LutomirskiThis patch shrinks vmalloc space from 16384TiB to 12800TiB to enlarge the hole starting at 0xff90000000000000 to be a full PGD entry.
A subsequent patch will use this hole for the pagetable isolation LDT alias.
--- Documentation/x86/x86_64/mm.txt | 4 ++-- arch/x86/include/asm/pgtable_64_types.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
Documentation/x86/x86_64/mm.txt +2 -2
@@ -28,8 +28,8 @@ Virtual memory map with 5 level page tables: hole caused by [56:63] sign extension ff00000000000000 - ff0fffffffffffff (=52 bits) guard hole, reserved for hypervisor ff10000000000000 - ff8fffffffffffff (=55 bits) direct mapping of all phys. memory -ff90000000000000 - ff91ffffffffffff (=49 bits) hole -ff92000000000000 - ffd1ffffffffffff (=54 bits) vmalloc/ioremap space +ff90000000000000 - ff9fffffffffffff (=52 bits) hole +ffa0000000000000 - ffd1ffffffffffff (=54 bits) vmalloc/ioremap space ffd2000000000000 - ffd3ffffffffffff (=49 bits) hole ffd4000000000000 - ffd5ffffffffffff (=49 bits) virtual memory map (512TB) ... unused hole ...
arch/x86/include/asm/pgtable_64_types.h +2 -2
@@ -78,8 +78,8 @@ typedef struct { pteval_t pte; } pte_t;/* See Documentation/x86/x86_64/mm.txt for a description of the memory map. */ #define MAXMEM _AC(__AC(1, UL) << MAX_PHYSMEM_BITS, UL)#ifdef CONFIG_X86_5LEVEL -#define VMALLOC_SIZE_TB _AC(16384, UL) -#define __VMALLOC_BASE _AC(0xff92000000000000, UL) +#define VMALLOC_SIZE_TB _AC(12800, UL) +#define __VMALLOC_BASE _AC(0xffa0000000000000, UL)#define __VMEMMAP_BASE _AC(0xffd4000000000000, UL)#else #define VMALLOC_SIZE_TB _AC(32, UL)
Signature · 1 line
2.13.6
Re: [PATCH PTI v3 09/10] x86/mm/64: Make a full PGD-entry size hole in the memory map
Quoted 20 lines · Andy Lutomirski
This patch shrinks vmalloc space from 16384TiB to 12800TiB to enlarge the hole starting at
0xff90000000000000to be a full PGD entry.A subsequent patch will use this hole for the pagetable isolation LDT alias.
Cc "Kirill A. Shutemov" <[email protected]>Cc Dave Hansen <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- Documentation/x86/x86_64/mm.txt | 4 ++-- arch/x86/include/asm/pgtable_64_types.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)Documentation/x86/x86_64/mm.txt +0
-0
Documentation/x86/x86_64/mm.txt Virtual memory map with 5 level page tables: · line 32
Quoted 7 lines
Documentation/x86/x86_64/mm.txt +2
-2@@ -28,8 +28,8 @@ Virtual memory map with 5 level page tables: hole caused by [56:63] sign extension ff00000000000000 - ff0fffffffffffff (=52 bits) guard hole, reserved for hypervisor ff10000000000000 - ff8fffffffffffff (=55 bits) direct mapping of all phys. memory -ff90000000000000 - ff91ffffffffffff (=49 bits) hole -ff92000000000000 - ffd1ffffffffffff (=54 bits) vmalloc/ioremap space +ff90000000000000 - ff9fffffffffffff (=52 bits) hole +ffa0000000000000 - ffd1ffffffffffff (=54 bits) vmalloc/ioremap space
It's not 54 bits anymore.
Signature · 1 line
Kirill A. Shutemov
Andy Lutomirski
maintainerRe: [PATCH PTI v3 09/10] x86/mm/64: Make a full PGD-entry size hole in the memory map
Whoops. I'll change that to "(=12800 TB)".
Quoted 33 lines · Kirill A. Shutemov
Quoted 20 lines · Andy Lutomirski
This patch shrinks vmalloc space from 16384TiB to 12800TiB to enlarge the hole starting at
0xff90000000000000to be a full PGD entry.A subsequent patch will use this hole for the pagetable isolation LDT alias.
Cc "Kirill A. Shutemov" <[email protected]>Cc Dave Hansen <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- Documentation/x86/x86_64/mm.txt | 4 ++-- arch/x86/include/asm/pgtable_64_types.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)Documentation/x86/x86_64/mm.txt +0
-0Documentation/x86/x86_64/mm.txt Virtual memory map with 5 level page tables: · line 32
Quoted 7 lines
Documentation/x86/x86_64/mm.txt +2
-2@@ -28,8 +28,8 @@ Virtual memory map with 5 level page tables: hole caused by [56:63] sign extension ff00000000000000 - ff0fffffffffffff (=52 bits) guard hole, reserved for hypervisor ff10000000000000 - ff8fffffffffffff (=55 bits) direct mapping of all phys. memory -ff90000000000000 - ff91ffffffffffff (=49 bits) hole -ff92000000000000 - ffd1ffffffffffff (=54 bits) vmalloc/ioremap space +ff90000000000000 - ff9fffffffffffff (=52 bits) hole +ffa0000000000000 - ffd1ffffffffffff (=54 bits) vmalloc/ioremap spaceIt's not 54 bits anymore.
--
Kirill A. Shutemov
Andy Lutomirski
maintainerPATCH v3 10/10 x86/pti: Put the LDT in its own PGD if PTI is on
In reply to Andy LutomirskiWith PTI on, we need the LDT to be in the usermode tables somewhere, and the LDT is per-mm.
tglx had a hack to have a per-cpu LDT and context switch it, but it was probably insanely slow due to the required TLB flushes.
Instead, take advantage of the fact that we have an address space hole that gives us a completely unused pgd and make that pgd be per-mm. We can put the LDT in it.
This has a down side: the LDT isn't (currently) randomized, and an attack that can write the LDT is instant root due to call gates (thanks, AMD, for leaving call gates in AMD64 but designing them wrong so they're only useful for exploits). We could mitigate this by making the LDT read-only or randomizing it, either of which is strightforward on top of this patch.
This will significantly slow down LDT users, but that shouldn't matter for important workloads -- the LDT is only used by DOSEMU(2), Wine, and very old libc implementations.
--- Documentation/x86/x86_64/mm.txt | 3 +- arch/x86/include/asm/mmu_context.h | 48 +++++++++- arch/x86/include/asm/pgtable_64_types.h | 4 + arch/x86/include/asm/processor.h | 23 +++-- arch/x86/kernel/ldt.c | 155 ++++++++++++++++++++++++++++++-- arch/x86/mm/dump_pagetables.c | 12 +++ 6 files changed, 225 insertions(+), 20 deletions(-)
Documentation/x86/x86_64/mm.txt +2 -1
@@ -12,6 +12,7 @@ ffffea0000000000 - ffffeaffffffffff (=40 bits) virtual memory map (1TB) ... unused hole ... ffffec0000000000 - fffffbffffffffff (=44 bits) kasan shadow memory (16TB) ... unused hole ... +fffffe8000000000 - fffffeffffffffff (=39 bits) LDT remap for PTI ffffff0000000000 - ffffff7fffffffff (=39 bits) %esp fixup stacks ... unused hole ... ffffffef00000000 - fffffffeffffffff (=64 GB) EFI region mapping space
@@ -28,7 +29,7 @@ Virtual memory map with 5 level page tables: hole caused by [56:63] sign extension ff00000000000000 - ff0fffffffffffff (=52 bits) guard hole, reserved for hypervisor ff10000000000000 - ff8fffffffffffff (=55 bits) direct mapping of all phys. memory -ff90000000000000 - ff9fffffffffffff (=52 bits) hole +ff90000000000000 - ff9fffffffffffff (=49 bits) LDT remap for PTI ffa0000000000000 - ffd1ffffffffffff (=54 bits) vmalloc/ioremap space ffd2000000000000 - ffd3ffffffffffff (=49 bits) hole ffd4000000000000 - ffd5ffffffffffff (=49 bits) virtual memory map (512TB)
arch/x86/include/asm/mmu_context.h +45 -3
74 lines not on this page · raw message
arch/x86/include/asm/pgtable_64_types.h +4 -0
@@ -81,10 +81,14 @@ typedef struct { pteval_t pte; } pte_t;#define VMALLOC_SIZE_TB _AC(12800, UL)#define __VMALLOC_BASE _AC(0xffa0000000000000, UL)#define __VMEMMAP_BASE _AC(0xffd4000000000000, UL) +#define LDT_PGD_ENTRY _AC(-112, UL) +#define LDT_BASE_ADDR (LDT_PGD_ENTRY << PGDIR_SHIFT)#else #define VMALLOC_SIZE_TB _AC(32, UL)#define __VMALLOC_BASE _AC(0xffffc90000000000, UL)#define __VMEMMAP_BASE _AC(0xffffea0000000000, UL) +#define LDT_PGD_ENTRY _AC(-3, UL) +#define LDT_BASE_ADDR (LDT_PGD_ENTRY << PGDIR_SHIFT)#endif #ifdef CONFIG_RANDOMIZE_MEMORY #define VMALLOC_START vmalloc_base
arch/x86/include/asm/processor.h +16 -7
@@ -851,13 +851,22 @@ static inline void spin_lock_prefetch(const void *x)#else /* -* User space process size. 47bits minus one guard page. The guard -* page is necessary on Intel CPUs: if a SYSCALL instruction is at -* the highest possible canonical userspace address, then that -* syscall will enter the kernel with a non-canonical return -* address, and SYSRET will explode dangerously. We avoid this -* particular problem by preventing anything from being mapped -* at the maximum canonical address. +* User space process size. This is the first address outside the user range. +* There are a few constraints that determine this: +* +* On Intel CPUs, if a SYSCALL instruction is at the highest canonical +* address, then that syscall will enter the kernel with a +* non-canonical return address, and SYSRET will explode dangerously. +* We avoid this particular problem by preventing anything executable +* from being mapped at the maximum canonical address. +* +* On AMD CPUs in the Ryzen family, there's a nasty bug in which the +* CPUs malfunction if they execute code from the highest canonical page. +* They'll speculate right off the end of the canonical space, and +* bad things happen. This is worked around in the same way as the +* Intel problem. +* +* With page table isolation enabled, we map the LDT in ... [stay tuned] */ #define TASK_SIZE_MAX ((1UL << __VIRTUAL_MASK_SHIFT) - PAGE_SIZE)
arch/x86/kernel/ldt.c +146 -9
212 lines not on this page · raw message
arch/x86/mm/dump_pagetables.c +12 -0
@@ -50,12 +50,18 @@ enum address_markers_idx {#ifdef CONFIG_X86_64 KERNEL_SPACE_NR, LOW_KERNEL_NR, +#if defined(CONFIG_MODIFY_LDT_SYSCALL) && defined(CONFIG_X86_5LEVEL) + LDT_NR, +#endif VMALLOC_START_NR, VMEMMAP_START_NR, #ifdef CONFIG_KASAN KASAN_SHADOW_START_NR, KASAN_SHADOW_END_NR, #endif +#if defined(CONFIG_MODIFY_LDT_SYSCALL) && !defined(CONFIG_X86_5LEVEL) + LDT_NR, +#endif # ifdef CONFIG_X86_ESPFIX64 ESPFIX_START_NR,# endif
@@ -79,12 +85,18 @@ static struct addr_marker address_markers[] = { #ifdef CONFIG_X86_64 { 0x8000000000000000UL, "Kernel Space" }, { 0/* PAGE_OFFSET */, "Low Kernel Mapping" }, +#if defined(CONFIG_MODIFY_LDT_SYSCALL) && defined(CONFIG_X86_5LEVEL) + { LDT_BASE_ADDR, "LDT remap" }, +#endif { 0/* VMALLOC_START */, "vmalloc() Area" }, { 0/* VMEMMAP_START */, "Vmemmap" }, #ifdef CONFIG_KASAN { KASAN_SHADOW_START, "KASAN shadow" }, { KASAN_SHADOW_END, "KASAN shadow end" }, #endif +#if defined(CONFIG_MODIFY_LDT_SYSCALL) && !defined(CONFIG_X86_5LEVEL) + { LDT_BASE_ADDR, "LDT remap" }, +#endif # ifdef CONFIG_X86_ESPFIX64 { ESPFIX_BASE_ADDR, "ESPfix Area", 16 }, # endif
Signature · 1 line
2.13.6
Re: [PATCH PTI v3 10/10] x86/pti: Put the LDT in its own PGD if PTI is on
arch/x86/include/asm/mmu_context.h struct ldt_struct { · line 71
Quoted 4 lines · Andy Lutomirski
arch/x86/include/asm/mmu_context.h +4
-0+/* 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 ....
Thanks,
tglx
Re: [PATCH PTI v3 10/10] x86/pti: Put the LDT in its own PGD if PTI is on
In reply to Andy Lutomirskiarch/x86/kernel/ldt.c static struct ldt_struct *alloc_ldt_struct(unsigned int num_entries) · line 189
Quoted 4 lines · Andy Lutomirski
arch/x86/kernel/ldt.c +4
-0+ +return 0 ; +#else +return -EINVAL ;
Errm. What's the point of that? Breaking non PTI?
Quoted 7 lines
arch/x86/kernel/ldt.c +1
-0new_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 );
arch/x86/kernel/ldt.c int init_new_context_ldt(struct task_struct *tsk, struct mm_struct *mm) · line 279
Quoted 10 lines
arch/x86/kernel/ldt.c +8
-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;
Thanks,
tglx
Andy Lutomirski
maintainerRe: [PATCH PTI v3 10/10] x86/pti: Put the LDT in its own PGD if PTI is on
Quoted 7 lines · Thomas Gleixner
arch/x86/include/asm/mmu_context.h struct ldt_struct { · line 71
Quoted 4 lines · Andy Lutomirski
arch/x86/include/asm/mmu_context.h +4
-0+/* 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.
Quoted 7 lines · Thomas Gleixner
arch/x86/kernel/ldt.c static struct ldt_struct *alloc_ldt_struct(unsigned int num_entries) · line 189
Quoted 4 lines · Andy Lutomirski
arch/x86/kernel/ldt.c +4
-0+ +return 0 ; +#else +return -EINVAL ;Errm. What's the point of that? Breaking non PTI?
That's embarrassing.
Quoted 11 lines
Quoted 11 lines
new_ldt = alloc_ldt_struct(old_mm->context.ldt->nr_entries); if (!new_ldt) {arch/x86/kernel/ldt.c +3
-0@@ -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;
Quoted 10 lines
arch/x86/kernel/ldt.c int init_new_context_ldt(struct task_struct *tsk, struct mm_struct *mm) · line 279
Quoted 7 lines
arch/x86/kernel/ldt.c +6
-0mm->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.
Re: [PATCH PTI v3 01/10] x86/espfix/64: Fix espfix double-fault handling on 5-level systems
In reply to Andy LutomirskiQuoted 5 lines · Andy Lutomirski
Using
PGDIR_SHIFTto identify espfix64 addresses on 5-level systems was wrong, and it resulted in panics due to unhandled double faults. UseP4D_SHIFTinstead, which is correct on 4-level and 5-level machines.
Quoted 7 lines
This fixes a panic when running x86 selftests on 5-level machines.
Fixes 1d33b219563f ("x86/espfix: Add support for 5-level paging")Cc "Kirill A. Shutemov" <[email protected]>Cc Dave Hansen <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>
Thanks for catching this.
I remember that I tested espfix before and it was fine. That's strange.
With this patch on top of tip/WIP.x86/pti plus the change I've mentioned before, sigreturn_64 doesn't crash qemu for me anymore.
Signature · 1 line
Kirill A. Shutemov
tip-bot for Andy Lutomirski
merge bot[tip:x86/urgent] x86/espfix/64: Fix espfix double-fault handling on 5-level systems
In reply to Andy Lutomirskix86/espfix/64: Fix espfix double-fault handling on 5-level systems
Commit-ID: c739f930be1dd5fd949030e3475a884fe06dae9b Gitweb: https://git.kernel.org/tip/c739f930be1dd5fd949030e3475a884fe06dae9b Author: Andy Lutomirski <[email protected]> AuthorDate: Tue, 12 Dec 2017 07:56:36 -0800 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 15 Dec 2017 16:58:53 +0100
x86/espfix/64: Fix espfix double-fault handling on 5-level systems
Using PGDIR_SHIFT to identify espfix64 addresses on 5-level systems was wrong, and it resulted in panics due to unhandled double faults. Use P4D_SHIFT instead, which is correct on 4-level and 5-level machines.
This fixes a panic when running x86 selftests on 5-level machines.
--- arch/x86/kernel/traps.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kernel/traps.c +1 -1
@@ -355,7 +355,7 @@ dotraplinkage void do_double_fault(struct pt_regs *regs, long error_code)* * No need for ist_enter here because we don't use RCU. */ -if (((long )regs->sp >>PGDIR_SHIFT ) ==ESPFIX_PGD_ENTRY && +if (((long )regs->sp >>P4D_SHIFT ) ==ESPFIX_PGD_ENTRY && regs->cs == __KERNEL_CS && regs->ip == (unsigned long )native_irq_return_iret) {