[PATCH RFC v9 12/25] mm: kpkeys: Protect regular page tables

[Mike Rapoport]

Hi Kevin,

> If the kpkeys_hardened_pgtables feature is enabled, page table pages
> (PTPs) should be protected by modifying the linear mapping to map
> them with a privileged pkey (KPKEYS_PKEY_PGTABLES). This patch
> introduces a new page allocator for that purpose:
> 
> * kpkeys_pgtable_alloc() allocates a new PTP and sets the linear
>   mapping to KPKEYS_PKEY_PGTABLES for that page
> 
> * kpkeys_pgtable_free() frees such a PTP and restores the linear
>   mapping to the default pkey
> 
> This interface is then hooked into pagetable_alloc() and
> pagetable_free(), protecting all page tables created once the buddy
> allocator is available. Early page tables are allocated in other
> ways and will be protected in subsequent patches.
> 
> This implementation of kpkeys_pgtable_{alloc,free}() is minimal and
> relies on the linear map being fully PTE-mapped - otherwise
> calling set_memory_pkey() on a single page may result in splitting a
> block mapping, which in turn requires allocating a new PTP. A more
> elaborate implementation could be added later to handle this
> situation.
> 
> Signed-off-by: Kevin Brodsky <kevin.brodsky at arm.com>
>
> diff --git a/include/linux/kpkeys.h b/include/linux/kpkeys.h
> index 23ae4ed512f52..288c8853eed60 100644
> --- a/include/linux/kpkeys.h
> +++ b/include/linux/kpkeys.h
> @@ -116,6 +116,9 @@ static inline bool kpkeys_hardened_pgtables_early_enabled(void)
>  	return arch_supports_kpkeys_early();
>  }
>  
> +struct page *kpkeys_pgtable_alloc(gfp_t gfp, unsigned int order);
> +void kpkeys_pgtable_free(struct page *page, unsigned int order);
> +
>  /*
>   * Should be called from mem_init(): as soon as the buddy allocator becomes
>   * available and before any call to pagetable_alloc().
> @@ -134,6 +137,13 @@ static inline bool kpkeys_hardened_pgtables_early_enabled(void)
>  	return false;
>  }
>  
> +static inline struct page *kpkeys_pgtable_alloc(gfp_t gfp, unsigned int order)
> +{
> +	return NULL;
> +}
> +
> +static inline void kpkeys_pgtable_free(struct page *page, unsigned int order) {}
> +
>  static inline void kpkeys_hardened_pgtables_init(void) {}
>  
>  #endif /* CONFIG_KPKEYS_HARDENED_PGTABLES */
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 485df9c2dbddb..eea3131786b35 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -37,6 +37,7 @@
>  #include <linux/bitmap.h>
>  #include <linux/bitops.h>
>  #include <linux/iommu-debug-pagealloc.h>
> +#include <linux/kpkeys.h>
>  
>  struct mempolicy;
>  struct anon_vma;
> @@ -3682,7 +3683,12 @@ static inline bool ptdesc_test_kernel(const struct ptdesc *ptdesc)
>   */
>  static inline struct ptdesc *pagetable_alloc_noprof(gfp_t gfp, unsigned int order)
>  {
> -	struct page *page = alloc_pages_noprof(gfp | __GFP_COMP, order);
> +	struct page *page;
> +
> +	if (kpkeys_hardened_pgtables_enabled())
> +		page = kpkeys_pgtable_alloc(gfp | __GFP_COMP, order);
> +	else
> +		page = alloc_pages_noprof(gfp | __GFP_COMP, order);

Can we make it a sequence rahter than a branch?

kpkeys_pgtable_alloc() does alloc_pages and then sets their pkeys, so I
think something like this should work:

	page = alloc_pages_noprof(gfp | __GFP_COMP, order);
	if (!page)
		return NULL;
	err = kpkeys_pgtable_alloc(page);
	if (err) {
		__free_pages(page, order);
		return NULL;

with if (kpkeys_hardened_pgtables_enabled()) folded into
kpkeys_pgtable_alloc().

>  
>  	return page_ptdesc(page);
>  }
> @@ -3691,8 +3697,12 @@ static inline struct ptdesc *pagetable_alloc_noprof(gfp_t gfp, unsigned int orde
>  static inline void __pagetable_free(struct ptdesc *pt)
>  {
>  	struct page *page = ptdesc_page(pt);
> +	unsigned int order = compound_order(page);
>  
> -	__free_pages(page, compound_order(page));
> +	if (kpkeys_hardened_pgtables_enabled())
> +		kpkeys_pgtable_free(page, order);
> +	else
> +		__free_pages(page, order);

And the same logic seem to apply here.

-- 
Sincerely yours,
Mike.