arm64: mm: Map fixmap page tables read-only

From: Ard Biesheuvel <[email protected]>

Without physical KASLR, the fixmap page tables will appear at an a priori known offset in the physical address space, and due to the lack of randomization, the linear map carries a writeable alias of the fixmap PTE pages, which appears at an offset in the kernel VA space that is also predictable.

Given that the placement of the fixmap area is never randomized either, a single store to this linear alias region is sufficient to map any physical page with any permissions at a known offset in the kernel VA space, including on top of the PTI trampoline.

Avoid this, by remapping the fixmap PTE pages read-only in the linear map. This is possible because all updates to bm_pte[] occur via the mapping of the kernel image in the vmap area. A read-only mapping is still needed for things like ptdump that walk the page tables.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/include/asm/fixmap.h | 3 +++
 arch/arm64/mm/fixmap.c          | 8 +++++---
 arch/arm64/mm/mmu.c             | 8 ++++++++
 3 files changed, 16 insertions(+), 3 deletions(-)

arch/arm64/include/asm/fixmap.h +0 -0

index 170c3502d723..9191125738e9 100644

arch/arm64/include/asm/fixmap.h +1 -0

+extern const size_t fixmap_bm_pte_size;
[...]

From: Ard Biesheuvel <[email protected]>

Instead of checking the destination of a page table descriptor update against the virtual address of swapper_pg_dir in the kernel image, and taking a dedicated path to perform such updates via a fixmap alias, rely on a fault handler to do so if the plain store faulted.

This is more efficient for the general case, and makes it more straight-forward to extend this logic to other page tables that may have been allocated statically in the .rodata region of the kernel image.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/include/asm/fixmap.h  |  1 +
 arch/arm64/include/asm/pgtable.h | 49 +++++++++-----------
 arch/arm64/mm/mmu.c              | 18 +++----
 3 files changed, 32 insertions(+), 36 deletions(-)

arch/arm64/include/asm/fixmap.h +0 -0

index 9191125738e9..8db791dcb4cb 100644

arch/arm64/include/asm/fixmap.h +1 -0

+	FIX_PTVAL,
 
 	__end_of_fixed_addresses
 };

arch/arm64/include/asm/pgtable.h +22 -27

index 6000905a2e86..799cd9d52c28 100644
@@ -35,6 +35,7 @@
 #include <asm/cmpxchg.h>
 #include <asm/fixmap.h>
 #include <asm/por.h>
+#include <asm/uaccess.h>
 #include <linux/mmdebug.h>
 #include <linux/mm_types.h>
 #include <linux/sched.h>

@@ -814,7 +815,7 @@ extern pgd_t idmap_pg_dir[];
 extern pgd_t tramp_pg_dir[];
 extern pgd_t reserved_pg_dir[];
 
-extern void set_swapper_pgd(pgd_t *pgdp, pgd_t pgd);
+void try_set_readonly_ptval(ptval_t *ptvalp, ptval_t ptval);
 
 static inline bool in_swapper_pgdir(void *addr)
 {

@@ -824,17 +825,14 @@ static inline bool in_swapper_pgdir(void *addr)
 
 static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
 {
-#ifdef __PAGETABLE_PMD_FOLDED
-	if (in_swapper_pgdir(pmdp)) {
-		set_swapper_pgd((pgd_t *)pmdp, __pgd(pmd_val(pmd)));
-		return;
-	}
-#endif /* __PAGETABLE_PMD_FOLDED */
-
-	WRITE_ONCE(*pmdp, pmd);
+	__put_kernel_nofault(pmdp, &pmd, pmd_t, fault);
 
 	if (pmd_valid(pmd))
 		queue_pte_barriers();
+	return;
+
+fault:
+	try_set_readonly_ptval(&pmd_val(*pmdp), pmd_val(pmd));
 }
 
 static inline void pmd_clear(pmd_t *pmdp)

@@ -890,15 +888,14 @@ static inline bool pgtable_l4_enabled(void);
 
 static inline void set_pud(pud_t *pudp, pud_t pud)
 {
-	if (!pgtable_l4_enabled() && in_swapper_pgdir(pudp)) {
-		set_swapper_pgd((pgd_t *)pudp, __pgd(pud_val(pud)));
-		return;
-	}
-
-	WRITE_ONCE(*pudp, pud);
+	__put_kernel_nofault(pudp, &pud, pud_t, fault);
 
 	if (pud_valid(pud))
 		queue_pte_barriers();
+	return;
+
+fault:
+	try_set_readonly_ptval(&pud_val(*pudp), pud_val(pud));
 }
 
 static inline void pud_clear(pud_t *pudp)

@@ -971,13 +968,12 @@ static inline bool mm_pud_folded(const struct mm_struct *mm)
 
 static inline void set_p4d(p4d_t *p4dp, p4d_t p4d)
 {
-	if (in_swapper_pgdir(p4dp)) {
-		set_swapper_pgd((pgd_t *)p4dp, __pgd(p4d_val(p4d)));
-		return;
-	}
-
-	WRITE_ONCE(*p4dp, p4d);
+	__put_kernel_nofault(p4dp, &p4d, p4d_t, fault);
 	queue_pte_barriers();
+	return;
+
+fault:
+	try_set_readonly_ptval(&p4d_val(*p4dp), p4d_val(p4d));
 }
 
 static inline void p4d_clear(p4d_t *p4dp)

@@ -1099,13 +1095,12 @@ static inline bool mm_p4d_folded(const struct mm_struct *mm)
 
 static inline void set_pgd(pgd_t *pgdp, pgd_t pgd)
 {
-	if (in_swapper_pgdir(pgdp)) {
-		set_swapper_pgd(pgdp, __pgd(pgd_val(pgd)));
-		return;
-	}
-
-	WRITE_ONCE(*pgdp, pgd);
+	__put_kernel_nofault(pgdp, &pgd, pgd_t, fault);
 	queue_pte_barriers();
+	return;
+
+fault:
+	try_set_readonly_ptval(&pgd_val(*pgdp), pgd_val(pgd));
 }
 
 static inline void pgd_clear(pgd_t *pgdp)

arch/arm64/mm/mmu.c +0 -0

index 9c1aa838e9d5..b97e4bf99ca5 100644

arch/arm64/mm/mmu.c +1 -1

 static DEFINE_MUTEX(fixmap_lock);
 
-void noinstr set_swapper_pgd(pgd_t *pgdp, pgd_t pgd)
+void noinstr try_set_readonly_ptval(ptval_t *ptvalp, ptval_t ptval)
 {

arch/arm64/mm/mmu.c +5 -6

-		WRITE_ONCE(*pgdp, pgd);
+		WRITE_ONCE(*ptvalp, ptval);
 		dsb(ishst);
 		isb();
 		return;
 	}
 
-	spin_lock(&swapper_pgdir_lock);
-	fixmap_pgdp = pgd_set_fixmap(__pa_symbol(pgdp));
-	WRITE_ONCE(*fixmap_pgdp, pgd);
+	guard(spinlock)(&lock);
+	ptvalp = (ptval_t *)set_fixmap_offset(FIX_PTVAL, __pa_symbol(ptvalp));
+	WRITE_ONCE(*ptvalp, ptval);
 	/*
 	 * We need dsb(ishst) here to ensure the page-table-walker sees
 	 * our new entry before set_p?d() returns. The fixmap's
 	 * flush_tlb_kernel_range() via clear_fixmap() does this for us.
 	 */
-	pgd_clear_fixmap();
-	spin_unlock(&swapper_pgdir_lock);
+	clear_fixmap(FIX_PTVAL);
 }
 
 pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,

From: Ard Biesheuvel <[email protected]>

The fixmap intermediate page tables are allocated statically, are installed into the kernel's page table hierarchy early during boot, and control a slice of the kernel's virtual address space that is not subject to KASLR randomization.

Combined with the lack of randomization of the linear map, and the tendency of some Android bootloaders to place the kernel image at the base of DRAM in the physical space, the placement of these page tables produces a vulnerability that is comparatively easy to exploit.

Avoid this, by moving these intermediate page tables into .rodata, so that they cannot be manipulated directly via the linear map.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/include/asm/linkage.h | 1 +
 arch/arm64/mm/fixmap.c           | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

arch/arm64/include/asm/linkage.h +1 -0

index d1f7a16729d2..00963e11ebf0 100644
@@ -45,6 +45,7 @@
 
 #define _THIS_IP_ ({ unsigned long __ip; asm volatile("adr %0, ." : "=r" (__ip)); __ip; })
 
+#define __rodata_pgtbl __section(".pgtbl.ro_after_init") __aligned(PAGE_SIZE)
 #define __bss_pgtbl __section(".bss..pgtbl") __aligned(PAGE_SIZE)
 
 #endif

arch/arm64/mm/fixmap.c +2 -2

index 3a8cf6de6a7d..ab0f9ba7b712 100644
@@ -32,8 +32,8 @@ static_assert(NR_BM_PMD_TABLES == 1);
 #define BM_PTE_TABLE_IDX(addr)	__BM_TABLE_IDX(addr, PMD_SHIFT)
 
 pte_t fixmap_bm_pte[NR_BM_PTE_TABLES][PTRS_PER_PTE] __bss_pgtbl;
-static pmd_t bm_pmd[PTRS_PER_PMD] __bss_pgtbl __maybe_unused;
-static pud_t bm_pud[PTRS_PER_PUD] __bss_pgtbl __maybe_unused;
+static pmd_t bm_pmd[PTRS_PER_PMD] __rodata_pgtbl;
+static pud_t bm_pud[PTRS_PER_PUD] __rodata_pgtbl;
 
 const size_t fixmap_bm_pte_size = sizeof(fixmap_bm_pte);
 

From: Ard Biesheuvel <[email protected]>

This v2 now covers intermediate level page tables as well as the PTE level page table for the fixmap. The latter is a special case, as it

a) is only accessed via the kernel image's mapping, and never via the linear map (except for ptdump etc)

b) must be accessible via a read-write mapping, as all manipulation of read-only page table descriptors relies on the fixmap itself

and so it is treated separately. The intermediate page tables may be shared with other mappings in the upper kernel/vmalloc region, so they must be updatable using the ordinary APIs.

Build tested and boot tested on a Lenovo Yoga C630 using 16k pages.

v1: https://lore.kernel.org/all/[email protected]/

Cc Ryan Roberts <[email protected]>

Cc Anshuman Khandual <[email protected]>

Cc Kevin Brodsky <[email protected]>

Cc Liz Prucka <[email protected]>

Cc Seth Jenkins <[email protected]>

Cc Kees Cook <[email protected]>

Cc Jann Horn <[email protected]>

Cc [email protected]

Quoted 19 lines · Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

This v2 now covers intermediate level page tables as well as the PTE level page table for the fixmap. The latter is a special case, as it

a) is only accessed via the kernel image's mapping, and never via the linear map (except for ptdump etc)

b) must be accessible via a read-write mapping, as all manipulation of read-only page table descriptors relies on the fixmap itself

and so it is treated separately. The intermediate page tables may be shared with other mappings in the upper kernel/vmalloc region, so they must be updatable using the ordinary APIs.

Build tested and boot tested on a Lenovo Yoga C630 using 16k pages.

v1: https://lore.kernel.org/all/[email protected]/

Quoted 8 lines

Cc Ryan Roberts <[email protected]>

Cc Anshuman Khandual <[email protected]>

Cc Kevin Brodsky <[email protected]>

Cc Liz Prucka <[email protected]>

Cc Seth Jenkins <[email protected]>

Cc Kees Cook <[email protected]>

Cc Jann Horn <[email protected]>

Cc [email protected]

Looks like the Cc's didn't propagate to the actual patches, fortunately my lei filters did catch this series ;)

Either way I quite like this series, it's an elegant approach and it should increase security without overhead, what's not to like!

I also considered it from the perspective of kpkeys protection [1] and I think they should work together fine. The kpkeys series still allows page table setters to write to all page tables, so if we get a fault there it must be because the target is read only, and not because of a pkey fault.