Clean up pgd handling and fix VSYSCALL and LDT

The 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.

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/entry/vsyscall/vsyscall_64.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

Quoted 6 lines · Andy Lutomirski

The 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.

Quoted 8 lines

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.

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/entry/vsyscall/vsyscall_64.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

Btw., would it make sense to clean up all this confusion?

In particular a 'KERNEL' pre of post fix is ambiguous in this context I think, and the PAGE_KERNEL_ prefix is actively harmful I think and is at the root of the confusion.

So if renamed it and used this nomenclature consistently instead:

PAGE_USER_
PAGE_SYSTEM_

... and got rid of PAGE_KERNEL uses in arch/x86/, then it would be obvious at first glance what kind of mapping is established in a particular place - and it would stay so in the future as well.

( There's some interaction with generic MM code which needs the original defines
like PAGE_KERNEL[_EXEC], but those generic masks could be defined as aliases, to keep this cleanup within x86 for now. )

Thanks,

Ingo

Quoted 20 lines · Ingo Molnar

Quoted 6 lines · Andy Lutomirski

The 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.

Quoted 8 lines

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.

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/entry/vsyscall/vsyscall_64.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

Btw., would it make sense to clean up all this confusion?

Quoted 8 lines

In particular a 'KERNEL' pre of post fix is ambiguous in this context I think, and the PAGE_KERNEL_ prefix is actively harmful I think and is at the root of the confusion.

So if renamed it and used this nomenclature consistently instead:

PAGE_USER_
PAGE_SYSTEM_

Like _PAGE_USER_VSYSCALL?

Anyway, that's not the confusion I'm talking about. I'm talking about _KERNPG_TABLE vs _PAGE_TABLE. The latter should be called _USERPG_TABLE, and a whole bunch of its users should be switched to _KERNPG_TABLE.

But, since PTI is intended for backporting, I think these types of big cleanups should wait.

/*
- * 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 +1 -0

index ae5615b03def..46ad333ed797 100644
@@ -19,6 +19,7 @@
 #include <linux/uaccess.h>
 
 #include <asm/ldt.h>
+#include <asm/tlb.h>
 #include <asm/desc.h>
 #include <asm/mmu_context.h>
 #include <asm/syscalls.h>

@@ -46,13 +47,12 @@ static void refresh_ldt_segments(void)
 static void flush_ldt(void *__mm)
 {

arch/x86/kernel/ldt.c +15 -0

+/*
+ * If PTI is enabled, this maps the LDT into the kernelmode and
+ * usermode tables for the given mm.
+ *
+ * There is no corresponding unmap function.  Even if the LDT is freed, we
+ * leave the PTEs around until the slot is reused or the mm is destroyed.
+ * This is harmless: the LDT is always in ordinary memory, and no one will
+ * access the freed slot.
+ *
+ * If we wanted to unmap freed LDTs, we'd also need to do a flush to make
+ * it useful, and the flush would slow down modify_ldt().
+ */
+static int map_ldt_struct(struct mm_struct *mm, struct ldt_struct *ldt, int slot)
+{
+#ifdef CONFIG_PAGE_TABLE_ISOLATION

arch/x86/kernel/ldt.c +9 -0

+	/*
+	 * Did we already have the top level entry allocated?  We can't
+	 * use pgd_none() for this because it doens't do anything on
+	 * 4-level page table kernels.
+	 */
+	pgd = pgd_offset(mm, LDT_BASE_ADDR);
+	had_top_level_entry = (pgd->pgd != 0);
+
+	is_vmalloc = is_vmalloc_addr(ldt->entries);

arch/x86/kernel/ldt.c +15 -0

+	if (mm->context.ldt) {
+		/*
+		 * We already had an LDT.  The top-level entry should already
+		 * have been allocated and synchronized with the usermode
+		 * tables.
+		 */
+		WARN_ON(!had_top_level_entry);
+		if (static_cpu_has_bug(X86_BUG_CPU_SECURE_MODE_PTI))
+			WARN_ON(!kernel_to_user_pgdp(pgd)->pgd);
+	} else {
+		/*
+		 * This is the first time we're mapping an LDT for this process.
+		 * Sync the pgd to the usermode tables.
+		 */
+		WARN_ON(had_top_level_entry);

arch/x86/kernel/ldt.c +13 -0

+	ldt->slot = slot;
+
+	return 0;
+#else
+	return -EINVAL;
+#endif
+}
+
+static void free_ldt_pgtables(struct mm_struct *mm)
+{
+#ifdef CONFIG_PAGE_TABLE_ISOLATION
+	struct mmu_gather tlb;
+	unsigned long start = LDT_BASE_ADDR;

arch/x86/kernel/ldt.c +18 -6

 /* After calling this, the LDT is immutable. */
 static void finalize_ldt_struct(struct ldt_struct *ldt)
 {

@@ -134,17 +237,15 @@ int init_new_context_ldt(struct task_struct *tsk, struct mm_struct *mm)
 	int retval = 0;
 
 	mutex_init(&mm->context.lock);
+	mm->context.ldt = NULL;
+
 	old_mm = current->mm;
-	if (!old_mm) {
-		mm->context.ldt = NULL;
+	if (!old_mm)
 		return 0;
-	}
 
 	mutex_lock(&old_mm->context.lock);
-	if (!old_mm->context.ldt) {
-		mm->context.ldt = NULL;
+	if (!old_mm->context.ldt)
 		goto out_unlock;
-	}
 
 	new_ldt = alloc_ldt_struct(old_mm->context.ldt->nr_entries);
 	if (!new_ldt) {

@@ -155,8 +256,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;
 
 out_unlock:
 	mutex_unlock(&old_mm->context.lock);

@@ -174,6 +284,11 @@ void destroy_context_ldt(struct mm_struct *mm)
 	mm->context.ldt = NULL;
 }
 
+void ldt_arch_exit_mmap(struct mm_struct *mm)
+{
+	free_ldt_pgtables(mm);
+}
+
 static int read_ldt(void __user *ptr, unsigned long bytecount)
 {
 	struct mm_struct *mm = current->mm;

So, before this,

On 12/10/2017 10:47 PM, Andy Lutomirski wrote:
...> + if (unlikely(ldt)) {

arch/x86/include/asm/mmu_context.h static inline void load_mm_ldt(struct mm_struct *mm) · line 114

Quoted 5 lines

arch/x86/include/asm/mmu_context.h +5 -0

+             if (static_cpu_has_bug(X86_BUG_CPU_SECURE_MODE_PTI)) {
+                     if (WARN_ON_ONCE((unsigned long)ldt->slot > 1)) {
+                             clear_LDT();
+                             return;
+                     }

I'm missing the purpose of the slots. Are you hoping to use those eventually for randomization, but just punting on implementing it for now?

arch/x86/include/asm/mmu_context.h static inline void load_mm_ldt(struct mm_struct *mm) · line 119

Quoted 5 lines

arch/x86/include/asm/mmu_context.h +5 -0

+
+                     set_ldt(ldt_slot_va(ldt->slot), ldt->nr_entries);
+             } else {
+                     set_ldt(ldt->entries, ldt->nr_entries);
+             }

This seems like a much better place to point out why the aliasing exists and what it is doing than the place it is actually commented.

Maybe:

                        /*
                         * ldt->entries is not mapped into the user page
                         * tables when page table isolation is enabled.
                         * Point the hardware to the alias we created.
                         */
                        set_ldt(ldt_slot_va(ldt->slot), ...
                } else {
                        /*
                         * Point the hardware at the normal kernel
                         * mapping when not isolated.
                         */
                        set_ldt(ldt->entries, ldt->nr_entries);
                }

Quoted 48 lines

/*
- * 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 +1 -1

index ae5615b03def..46ad333ed797 100644
@@ -19,6 +19,7 @@
 #include <linux/uaccess.h>
 
 #include <asm/ldt.h>
+#include <asm/tlb.h>
 #include <asm/desc.h>
 #include <asm/mmu_context.h>
 #include <asm/syscalls.h>

@@ -46,13 +47,12 @@ static void refresh_ldt_segments(void)
 static void flush_ldt(void *__mm)
 {
      struct mm_struct *mm = __mm;
-     mm_context_t *pc;
 
      if (this_cpu_read(cpu_tlbstate.loaded_mm) != mm)
              return;
 

arch/x86/kernel/ldt.c static void refresh_ldt_segments(void) · line 55

Quoted 4 lines

arch/x86/kernel/ldt.c +2 -2

-     pc = &mm->context;
-     set_ldt(pc->ldt->entries, pc->ldt->nr_entries);
+     __flush_tlb_all();
+     load_mm_ldt(mm);

Why the new TLB flush?

arch/x86/kernel/ldt.c static struct ldt_struct *alloc_ldt_struct(unsigned int num_entries) · line 95

Quoted 7 lines

arch/x86/kernel/ldt.c +1 -0

@@ -90,9 +90,112 @@ static struct ldt_struct *alloc_ldt_struct(unsigned int num_entries)
      }
 
      new_ldt->nr_entries = num_entries;
+     new_ldt->slot = -1;
      return new_ldt;
 }

This seems a bit silly to do given that 'slot' is an int and this patch introduces warnings looking for positive values:

        if (WARN_ON_ONCE((unsigned long)ldt->slot > 1)) {

Seems like a good idea to just have a single warning in there looking for non-zero, probably covering the PTI and non-PTI cases (at least for now until the slots get used).

Quoted 16 lines

arch/x86/kernel/ldt.c +15 -0

+/*
+ * If PTI is enabled, this maps the LDT into the kernelmode and
+ * usermode tables for the given mm.
+ *
+ * There is no corresponding unmap function.  Even if the LDT is freed, we
+ * leave the PTEs around until the slot is reused or the mm is destroyed.
+ * This is harmless: the LDT is always in ordinary memory, and no one will
+ * access the freed slot.
+ *
+ * If we wanted to unmap freed LDTs, we'd also need to do a flush to make
+ * it useful, and the flush would slow down modify_ldt().
+ */
+static int map_ldt_struct(struct mm_struct *mm, struct ldt_struct *ldt, int slot)
+{
+#ifdef CONFIG_PAGE_TABLE_ISOLATION

arch/x86/kernel/ldt.c static struct ldt_struct *alloc_ldt_struct(unsigned int num_entries) · line 121

Quoted 10 lines

arch/x86/kernel/ldt.c +10 -0

+     spinlock_t *ptl;
+     bool is_vmalloc;
+     bool had_top_level_entry;
+     pgd_t *pgd;
+     int i;
+
+     if (!static_cpu_has_bug(X86_BUG_CPU_SECURE_MODE_PTI))
+             return 0;
+
+     WARN_ON(ldt->slot != -1);

Only allow mapping newly-allocated LDTs?

Quoted 9 lines

arch/x86/kernel/ldt.c +9 -0

+     /*
+      * Did we already have the top level entry allocated?  We can't
+      * use pgd_none() for this because it doens't do anything on
+      * 4-level page table kernels.
+      */
+     pgd = pgd_offset(mm, LDT_BASE_ADDR);
+     had_top_level_entry = (pgd->pgd != 0);
+
+     is_vmalloc = is_vmalloc_addr(ldt->entries);

arch/x86/kernel/ldt.c static struct ldt_struct *alloc_ldt_struct(unsigned int num_entries) · line 141

Quoted 10 lines

arch/x86/kernel/ldt.c +10 -0

+
+     for (i = 0; i * PAGE_SIZE < ldt->nr_entries * LDT_ENTRY_SIZE; i++) {
+             unsigned long offset = i << PAGE_SHIFT;
+             unsigned long va = (unsigned long)ldt_slot_va(slot) + offset;
+             const void *src = (char *)ldt->entries + offset;
+             unsigned long pfn = is_vmalloc ? vmalloc_to_pfn(src) :
+                     page_to_pfn(virt_to_page(src));
+             pte_t pte, *ptep;
+
+             ptep = get_locked_pte(mm, va, &ptl);

It's *probably* worth calling out that all the page table allocation happens in there. I went looking for it in this patch and it took me a few minutes to find it.

arch/x86/kernel/ldt.c static struct ldt_struct *alloc_ldt_struct(unsigned int num_entries) · line 144

Quoted 3 lines

arch/x86/kernel/ldt.c +3 -0

+             if (!ptep)
+                     return -ENOMEM;
+             pte = pfn_pte(pfn, __pgprot(__PAGE_KERNEL & ~_PAGE_GLOBAL));

This ~_PAGE_GLOBAL is for the same reason as all the other KPTI code, right? BTW, does this function deserve to be in the LDT code or kpti?

arch/x86/kernel/ldt.c static struct ldt_struct *alloc_ldt_struct(unsigned int num_entries) · line 147

Quoted 3 lines

arch/x86/kernel/ldt.c +3 -0

+                           set_pte_at(mm, va, ptep, pte);
+             pte_unmap_unlock(ptep, ptl);
+     }

Might want to fix up the set_pte_at() whitespace damage.

Quoted 16 lines

arch/x86/kernel/ldt.c +15 -0

+     if (mm->context.ldt) {
+             /*
+              * We already had an LDT.  The top-level entry should already
+              * have been allocated and synchronized with the usermode
+              * tables.
+              */
+             WARN_ON(!had_top_level_entry);
+             if (static_cpu_has_bug(X86_BUG_CPU_SECURE_MODE_PTI))
+                     WARN_ON(!kernel_to_user_pgdp(pgd)->pgd);
+     } else {
+             /*
+              * This is the first time we're mapping an LDT for this process.
+              * Sync the pgd to the usermode tables.
+              */
+             WARN_ON(had_top_level_entry);

arch/x86/kernel/ldt.c static struct ldt_struct *alloc_ldt_struct(unsigned int num_entries) · line 173

Quoted 10 lines

arch/x86/kernel/ldt.c +10 -0

+             if (static_cpu_has_bug(X86_BUG_CPU_SECURE_MODE_PTI)) {
+                     WARN_ON(kernel_to_user_pgdp(pgd)->pgd);
+                     set_pgd(kernel_to_user_pgdp(pgd), *pgd);
+             }
+     }
+
+     flush_tlb_mm_range(mm,
+                        (unsigned long)ldt_slot_va(slot),
+                        (unsigned long)ldt_slot_va(slot) + LDT_SLOT_STRIDE,
+                        0);

Why wait until here to flush? Isn't this primarily for the case where set_pte_at() overwrote something?

Quoted 11 lines

arch/x86/kernel/ldt.c +11 -0

+
+static void free_ldt_pgtables(struct mm_struct *mm)
+{
+#ifdef CONFIG_PAGE_TABLE_ISOLATION
+     struct mmu_gather tlb;
+     unsigned long start = LDT_BASE_ADDR;
+     unsigned long end = start + (1UL << PGDIR_SHIFT);
+
+     if (!static_cpu_has_bug(X86_BUG_CPU_SECURE_MODE_PTI))
+             return;
+

arch/x86/kernel/ldt.c static struct ldt_struct *alloc_ldt_struct(unsigned int num_entries) · line 197

Quoted 5 lines

arch/x86/kernel/ldt.c +5 -0

+     tlb_gather_mmu(&tlb, mm, start, end);
+     free_pgd_range(&tlb, start, end, start, end);
+     tlb_finish_mmu(&tlb, start, end);
+#endif
+}

Isn't this primarily called at exit()? Isn't it a bit of a shame we can't combine this with the other exit()-time TLB flushing?

Also, from a high level, this does increase the overhead of KPTI in a non-trivial way, right? It costs us three more page table pages per process allocated at fork() and freed at exit() and a new TLB flush.

Quoted 3 lines

Also, from a high level, this does increase the overhead of KPTI in a non-trivial way, right? It costs us three more page table pages per process allocated at fork() and freed at exit() and a new TLB flush.

Yeah, but no one will care. modify_ldt() is used for DOSEMU, Wine, and really old 32-bit programs.

Quoted 5 lines · Andy Lutomirski

Quoted 3 lines

Also, from a high level, this does increase the overhead of KPTI in a non-trivial way, right? It costs us three more page table pages per process allocated at fork() and freed at exit() and a new TLB flush.

Yeah, but no one will care. modify_ldt() is used for DOSEMU, Wine, and really old 32-bit programs.

The heavyweight part of map_ldt_struct() (and unmap) looks to run whenever we have KPTI enabled. I'm missing how it gets avoided for the non-DOSEMU cases.

I thought there would be a "fast path" where we just use the normal clear_LDT() LDT from the cpu_entry_area and don't have to do any of this, but I'm missing where that happens. Do we need a check in (un)map_ldt_struct() for !mm->context.ldt?

Just to make sure I understand this: We now have two places that LDTs live in virtual space:

1. The "plain" one that we get from clear_LDT() which lives in the cpu_entry_area. (No additional overhead when doing this)
2. The new one under the special PGD that's only used for modify_ldt() and is fairly slow. (plenty of overhead, but nobody cares).

Quoted 4 lines

I thought there would be a "fast path" where we just use the normal clear_LDT() LDT from the cpu_entry_area and don't have to do any of this, but I'm missing where that happens. Do we need a check in (un)map_ldt_struct() for !mm->context.ldt?

I'm confused.

if (unlikely(ldt)) {
do something slowish;
} else {
clear_LD();
}

Quoted 11 lines · Andy Lutomirski

Quoted 4 lines

I thought there would be a "fast path" where we just use the normal clear_LDT() LDT from the cpu_entry_area and don't have to do any of this, but I'm missing where that happens. Do we need a check in (un)map_ldt_struct() for !mm->context.ldt?

I'm confused.

if (unlikely(ldt)) {
do something slowish;
} else {
clear_LD();
}

I was looking at the map/unmap paths. It looks to me like the cases where there is map/unmap overhead, we *are* doing checking against mm->context.ldt. It just wasn't visible from the patch context.

In any case, it would be really nice to call that out if you revise these in the patch description: none of these LDT acrobatics are used in the common case. Virtually every process uses the !ldt paths which don't do any of this.

I'm getting reasonably happy with this. It still needs more testing, but I want to get it out there.

The main things that need testing are the 5-level case for the both vsyscalls and the LDT. I'm getting a double-fault in ldt_gdt_64, but I haven't checked whether it's a bug in this series, and it kind of looks like it isn't. I'll figure it out in the morning. The docs also want updating for the 5 level case.

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 (6):
x86/pti: Vastly simplify pgd synchronization
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/pti: Put the LDT in its own PGD if PTI is on

 Documentation/x86/x86_64/mm.txt         |  11 ++-
 arch/x86/Kconfig                        |   8 --
 arch/x86/entry/vsyscall/vsyscall_64.c   |  37 ++++++++-
 arch/x86/include/asm/mmu_context.h      |  33 +++++++-
 arch/x86/include/asm/pgtable.h          |   6 +-
 arch/x86/include/asm/pgtable_64.h       |  77 ++++++++----------
 arch/x86/include/asm/pgtable_64_types.h |   2 +
 arch/x86/include/asm/processor.h        |  23 ++++--
 arch/x86/include/asm/vsyscall.h         |   1 +
 arch/x86/kernel/ldt.c                   | 138 +++++++++++++++++++++++++++++---
 arch/x86/mm/pti.c                       | 115 ++++++++++++++++----------
 11 files changed, 331 insertions(+), 120 deletions(-)

--
2.13.6

Quoted 9 lines · Andy Lutomirski

I'm getting reasonably happy with this. It still needs more testing, but I want to get it out there.

The main things that need testing are the 5-level case for the both vsyscalls and the LDT. I'm getting a double-fault in ldt_gdt_64, but I haven't checked whether it's a bug in this series, and it kind of looks like it isn't. I'll figure it out in the morning. The docs also want updating for the 5 level case.

The actual failure looks a lot like the ESPFIX stacks aren't mapped in the usermode tables, which reinforces my old belief that this bit of code is bogus:

/*
* 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

Of course, the comment is even more wrong with this series applied, but I think it's been wrong all along.

I'll fix it in the morning if no one beats me to it.

(Hint: this can be tested in QEMU with -machine accel=tcg -cpu qemu64,+la57)