Re: Linux 6.11-rc1

[Peter Zijlstra <peterz@infradead.org>]

On Wed, Jul 31, 2024 at 07:26:04PM +0200, Thomas Gleixner wrote:
> On Wed, Jul 31 2024 at 18:51, Peter Zijlstra wrote:
> > On Wed, Jul 31, 2024 at 06:31:05PM +0200, Peter Zijlstra wrote:
> > Thomas, this all still relies on the full text section being PMD mapped,
> > and since we don't have ALIGN_ENTRY_TEXT_END and _etext has PAGE_SIZE
> > alignment, can't have a PAGE mapped tail which then doesn't get cloned?
> >
> > Do we want to make pto_clone_entry_text() use PTI_LEVEL_KERNEL_IMAGE
> > such that it will clone whatever it has?
> 
> Yes, I think so.

The alternative is ripping that level thing out entirely, and simply
duplicate anything we find in the page-tables.

We could add something like:

	WARN_ON_ONCE(IS_ENABLED(CONFIG_X86_64));

in the PTE path, but do we really care?

---
--- a/arch/x86/mm/pti.c
+++ b/arch/x86/mm/pti.c
@@ -47,16 +47,6 @@
 #define __GFP_NOTRACK	0
 #endif
 
-/*
- * Define the page-table levels we clone for user-space on 32
- * and 64 bit.
- */
-#ifdef CONFIG_X86_64
-#define	PTI_LEVEL_KERNEL_IMAGE	PTI_CLONE_PMD
-#else
-#define	PTI_LEVEL_KERNEL_IMAGE	PTI_CLONE_PTE
-#endif
-
 static void __init pti_print_if_insecure(const char *reason)
 {
 	if (boot_cpu_has_bug(X86_BUG_CPU_MELTDOWN))
@@ -294,14 +284,7 @@ static void __init pti_setup_vsyscall(vo
 static void __init pti_setup_vsyscall(void) { }
 #endif
 
-enum pti_clone_level {
-	PTI_CLONE_PMD,
-	PTI_CLONE_PTE,
-};
-
-static void
-pti_clone_pgtable(unsigned long start, unsigned long end,
-		  enum pti_clone_level level)
+static void pti_clone_pgtable(unsigned long start, unsigned long end)
 {
 	unsigned long addr;
 
@@ -341,7 +324,7 @@ pti_clone_pgtable(unsigned long start, u
 			continue;
 		}
 
-		if (pmd_leaf(*pmd) || level == PTI_CLONE_PMD) {
+		if (pmd_leaf(*pmd)) {
 			target_pmd = pti_user_pagetable_walk_pmd(addr);
 			if (WARN_ON(!target_pmd))
 				return;
@@ -375,37 +358,33 @@ pti_clone_pgtable(unsigned long start, u
 			*target_pmd = *pmd;
 
 			addr = round_up(addr + 1, PMD_SIZE);
+			continue;
+		}
 
-		} else if (level == PTI_CLONE_PTE) {
-
-			/* Walk the page-table down to the pte level */
-			pte = pte_offset_kernel(pmd, addr);
-			if (pte_none(*pte)) {
-				addr = round_up(addr + 1, PAGE_SIZE);
-				continue;
-			}
-
-			/* Only clone present PTEs */
-			if (WARN_ON(!(pte_flags(*pte) & _PAGE_PRESENT)))
-				return;
+		/* Walk the page-table down to the pte level */
+		pte = pte_offset_kernel(pmd, addr);
+		if (pte_none(*pte)) {
+			addr = round_up(addr + 1, PAGE_SIZE);
+			continue;
+		}
 
-			/* Allocate PTE in the user page-table */
-			target_pte = pti_user_pagetable_walk_pte(addr);
-			if (WARN_ON(!target_pte))
-				return;
+		/* Only clone present PTEs */
+		if (WARN_ON(!(pte_flags(*pte) & _PAGE_PRESENT)))
+			return;
 
-			/* Set GLOBAL bit in both PTEs */
-			if (boot_cpu_has(X86_FEATURE_PGE))
-				*pte = pte_set_flags(*pte, _PAGE_GLOBAL);
+		/* Allocate PTE in the user page-table */
+		target_pte = pti_user_pagetable_walk_pte(addr);
+		if (WARN_ON(!target_pte))
+			return;
 
-			/* Clone the PTE */
-			*target_pte = *pte;
+		/* Set GLOBAL bit in both PTEs */
+		if (boot_cpu_has(X86_FEATURE_PGE))
+			*pte = pte_set_flags(*pte, _PAGE_GLOBAL);
 
-			addr = round_up(addr + 1, PAGE_SIZE);
+		/* Clone the PTE */
+		*target_pte = *pte;
 
-		} else {
-			BUG();
-		}
+		addr = round_up(addr + 1, PAGE_SIZE);
 	}
 }
 
@@ -475,7 +454,7 @@ static void __init pti_clone_user_shared
 	start = CPU_ENTRY_AREA_BASE;
 	end   = start + (PAGE_SIZE * CPU_ENTRY_AREA_PAGES);
 
-	pti_clone_pgtable(start, end, PTI_CLONE_PMD);
+	pti_clone_pgtable(start, end);
 }
 #endif /* CONFIG_X86_64 */
 
@@ -495,8 +474,7 @@ static void __init pti_setup_espfix64(vo
 static void pti_clone_entry_text(void)
 {
 	pti_clone_pgtable((unsigned long) __entry_text_start,
-			  (unsigned long) __entry_text_end,
-			  PTI_CLONE_PMD);
+			  (unsigned long) __entry_text_end);
 }
 
 /*
@@ -571,7 +549,7 @@ static void pti_clone_kernel_text(void)
 	 * pti_set_kernel_image_nonglobal() did to clear the
 	 * global bit.
 	 */
-	pti_clone_pgtable(start, end_clone, PTI_LEVEL_KERNEL_IMAGE);
+	pti_clone_pgtable(start, end_clone);
 
 	/*
 	 * pti_clone_pgtable() will set the global bit in any PMDs