Re: [PATCH RFC v10 13/22] mm: kpkeys: Protect early kernel page tables

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] mm: kpkeys: redundant KASAN shadow walks in
  protect_early_kernel_pgtables()
- [Medium] mm: kpkeys: repeated swapper_pg_dir walks in
  protect_early_kernel_pgtables()
--

commit 9b67b4dcc54e06a0681d8d0ff9c11cb3015125f0
Author: Kevin Brodsky <kevin.brodsky@arm.com>

mm: kpkeys: Protect early kernel page tables

This commit walks the installed kernel page tables during boot and sets the
direct-map pkey of their backing pages to KPKEYS_PKEY_PGTABLES when
kpkeys_hardened_pgtables is enabled.

> diff --git a/mm/kpkeys_hardened_pgtables.c b/mm/kpkeys_hardened_pgtables.c
> index 2311fd8da6bbb..f6825a0e0689d 100644
> --- a/mm/kpkeys_hardened_pgtables.c
> +++ b/mm/kpkeys_hardened_pgtables.c
> @@ -54,10 +56,130 @@ void kpkeys_pgtable_free(struct page *page, unsigned int order)
>  	__free_pages(page, order);
>  }
>  
> +static void __init protect_pgtable_page(void *page)
> +{
> +	int ret;
> +
> +	ret = set_direct_map_pkey((unsigned long)page, 1, KPKEYS_PKEY_PGTABLES);
> +	WARN_ON_ONCE(ret);
> +}
> +
> +static int __init protect_pte_table(pmd_t *pmd, unsigned long addr,
> +				    unsigned long next, struct mm_walk *walk)
> +{
> +	pmd_t entry = pmdp_get(pmd);
> +
> +	if (!pmd_present(entry) || pmd_leaf(entry))
> +		return 0;
> +
> +	protect_pgtable_page(pte_offset_kernel(pmd, 0));
> +
> +	return 0;
> +}
> +
> +static int __init protect_pmd_table(pud_t *pud, unsigned long addr,
> +				    unsigned long next, struct mm_walk *walk)
> +{
> +	pud_t entry;
> +
> +	if (mm_pmd_folded(walk->mm))
> +		return 0;
> +
> +	entry = pudp_get(pud);
> +	if (!pud_present(entry) || pud_leaf(entry))
> +		return 0;
> +
> +	protect_pgtable_page(pmd_offset(pud, 0));
> +
> +	return 0;
> +}
> +
> +static int __init protect_pud_table(p4d_t *p4d, unsigned long addr,
> +				    unsigned long next, struct mm_walk *walk)
> +{
> +	p4d_t entry;
> +
> +	if (mm_pud_folded(walk->mm))
> +		return 0;
> +
> +	entry = p4dp_get(p4d);
> +	if (!p4d_present(entry) || p4d_leaf(entry))
> +		return 0;
> +
> +	protect_pgtable_page(pud_offset(p4d, 0));
> +
> +	return 0;
> +}
> +
> +static int __init protect_p4d_table(pgd_t *pgd, unsigned long addr,
> +				    unsigned long next, struct mm_walk *walk)
> +{
> +	pgd_t entry;
> +
> +	if (mm_p4d_folded(walk->mm))
> +		return 0;
> +
> +	entry = pgdp_get(pgd);
> +	if (!pgd_present(entry) || pgd_leaf(entry))
> +		return 0;
> +
> +	protect_pgtable_page(p4d_offset(pgd, 0));
> +
> +	return 0;
> +}

[Severity: Medium]
When CONFIG_KASAN_GENERIC or CONFIG_KASAN_SW_TAGS is enabled, does this walk
and call set_direct_map_pkey() millions of times on the shared
kasan_early_shadow_{p4d,pud,pmd,pte} tables?

kasan_init() runs in setup_arch() before mm_core_init() calls
kpkeys_hardened_pgtables_init(), populating unbacked shadow ranges in
[PAGE_OFFSET, ULONG_MAX) with shared static tables where all 512 entries at
each level point to the same next-level kasan_early_shadow_* page.  Unlike
mm/ptdump.c, which detects those tables and sets walk->action =
ACTION_CONTINUE, the callbacks here descend through every entry and invoke
set_direct_map_pkey() (including a direct-map page table walk and broadcast
TLB flush) for every 2 MiB PMD slot across the multi-terabyte shadow range,
stalling boot.

[ ... ]

> +static void __init protect_early_kernel_pgtables(void)
> +{
> +	int ret;
> +
> +	mmap_write_lock(&init_mm);
> +	/*
> +	 * [PAGE_OFFSET, ULONG_MAX) covers all early kernel mappings on arm64
> +	 * and x86 (at least).
> +	 */
> +	ret = walk_kernel_page_table_range(PAGE_OFFSET, ULONG_MAX,
> +					   &protect_pgtable_ops, NULL, NULL);
> +	mmap_write_unlock(&init_mm);
> +	WARN_ON_ONCE(ret);
> +}

[Severity: Medium]
On arm64 with CONFIG_ARM64_LPA2=y running on hardware where the top page
table level is dynamically folded (vabits_actual < VA_BITS), does starting
at PAGE_OFFSET walk and re-protect swapper_pg_dir 16 or 32 times?

On arm64, PAGE_OFFSET is the compile-time _PAGE_OFFSET(VA_BITS) rather than
_PAGE_OFFSET(vabits_actual).  When !pgtable_l5_enabled() on 4K pages (or
!pgtable_l4_enabled() on 16K pages), pgd_none() is always false and
pgd_to_folded_p4d() (or p4d_to_folded_pud()) wraps each top-level step in
[PAGE_OFFSET, ULONG_MAX) back to the start of swapper_pg_dir:

arch/arm64/include/asm/pgtable.h:pgd_to_folded_p4d() {
	return (p4d_t *)PTR_ALIGN_DOWN(pgdp, PAGE_SIZE) + p4d_index(addr);
}

As a result, walk_kernel_page_table_range() traverses the entire kernel page
table hierarchy and flushes TLBs across all page table pages 16 times on 4K
pages (or 32 times on 16K pages) during boot.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-kpkeys-v10-0-aa6c4d9603bc@arm.com?part=13