On 05/23/2019 03:52 PM, Ard Biesheuvel wrote:
> Wire up the special helper functions to manipulate aliases of vmalloc
> regions in the linear map.
IMHO the commit message here could be bit more descriptive because of the
amount of changes this patch brings in.
>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/cacheflush.h | 3 ++
> arch/arm64/mm/pageattr.c | 48 ++++++++++++++++----
> mm/vmalloc.c | 11 -----
> 4 files changed, 44 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index ca9c175fb949..4ab32180eabd 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -26,6 +26,7 @@ config ARM64
> select ARCH_HAS_MEMBARRIER_SYNC_CORE
> select ARCH_HAS_PTE_SPECIAL
> select ARCH_HAS_SETUP_DMA_OPS
> + select ARCH_HAS_SET_DIRECT_MAP
> select ARCH_HAS_SET_MEMORY
> select ARCH_HAS_STRICT_KERNEL_RWX
> select ARCH_HAS_STRICT_MODULE_RWX
> diff --git a/arch/arm64/include/asm/cacheflush.h
> b/arch/arm64/include/asm/cacheflush.h
> index 19844211a4e6..b9ee5510067f 100644
> --- a/arch/arm64/include/asm/cacheflush.h
> +++ b/arch/arm64/include/asm/cacheflush.h
> @@ -187,4 +187,7 @@ static inline void flush_cache_vunmap(unsigned long
> start, unsigned long end)
>
> int set_memory_valid(unsigned long addr, int numpages, int enable);
>
> +int set_direct_map_invalid_noflush(struct page *page);
> +int set_direct_map_default_noflush(struct page *page);
> +
> #endif
> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> index 6cd645edcf35..9c6b9039ec8f 100644
> --- a/arch/arm64/mm/pageattr.c
> +++ b/arch/arm64/mm/pageattr.c
> @@ -159,17 +159,48 @@ int set_memory_valid(unsigned long addr, int numpages,
> int enable)
> __pgprot(PTE_VALID));
> }
>
> -#ifdef CONFIG_DEBUG_PAGEALLOC
> +int set_direct_map_invalid_noflush(struct page *page)
> +{
> + struct page_change_data data = {
> + .set_mask = __pgprot(0),
> + .clear_mask = __pgprot(PTE_VALID),
> + };
> +
> + if (!rodata_full)
> + return 0;
Why rodata_full needs to be probed here ? Should not we still require the
following
transition even if rodata_full is not enabled. Just wondering whether we can use
VM_FLUSH_RESET_PERMS feature without these required transitions.
/*
* Set direct map to something invalid so that it won't be cached if
* there are any accesses after the TLB flush, then flush the TLB and
* reset the direct map permissions to the default.
*/
set_area_direct_map(area, set_direct_map_invalid_noflush);
_vm_unmap_aliases(start, end, 1);
set_area_direct_map(area, set_direct_map_default_noflush);
> +
> + return apply_to_page_range(&init_mm,
> + (unsigned long)page_address(page),
> + PAGE_SIZE, change_page_range, &data);
> +}
> +
> +int set_direct_map_default_noflush(struct page *page)
> +{
> + struct page_change_data data = {
> + .set_mask = __pgprot(PTE_VALID | PTE_WRITE),
> + .clear_mask = __pgprot(PTE_RDONLY),
Replace __pgprot(PTE_VALID | PTE_WRITE) with PAGE_KERNEL instead !
> + };
> +
> + if (!rodata_full)
> + return 0;
> +
> + return apply_to_page_range(&init_mm,
> + (unsigned long)page_address(page),
> + PAGE_SIZE, change_page_range, &data);
> +}
> +
IIUC set_direct_map_invalid_noflush() and set_direct_map_default_noflush()
should set *appropriate* permissions as seen fit from platform perspective
to implement this transition.
In here set_direct_map_invalid_noflush() makes the entry invalid preventing
further MMU walks (hence new TLB entries). set_direct_map_default_noflush()
makes it a valid write entry. Though it looks similar to PAGE_KERNEL which
is the default permission for linear mapping on arm64 via __map_memblock().
Should not PAGE_KERNEL be used explicitly as suggested above.
> void __kernel_map_pages(struct page *page, int numpages, int enable)
> {
> + if (!debug_pagealloc_enabled() && !rodata_full)
> + return;
> +
I guess this is not related to CONFIG_ARCH_HAS_SET_DIRECT_MAP here and should
be a fix or an enhancement to CONFIG_DEBUG_PAGEALLOC implementation. Just
curious, !rodata_full check here to ensure that linear mapping does not have
block or contig mappings and should be backed by regular pages only ?
> set_memory_valid((unsigned long)page_address(page), numpages, enable);
> }
> -#ifdef CONFIG_HIBERNATION
> +
> /*
> - * When built with CONFIG_DEBUG_PAGEALLOC and CONFIG_HIBERNATION, this
> function
> - * is used to determine if a linear map page has been marked as not-valid by
> - * CONFIG_DEBUG_PAGEALLOC. Walk the page table and check the PTE_VALID bit.
> - * This is based on kern_addr_valid(), which almost does what we need.
> + * This function is used to determine if a linear map page has been marked as
> + * not-valid. Walk the page table and check the PTE_VALID bit. This is based
> + * on kern_addr_valid(), which almost does what we need.
> *
> * Because this is only called on the kernel linear map, p?d_sect() implies
> * p?d_present(). When debug_pagealloc is enabled, sections mappings are
> @@ -183,6 +214,9 @@ bool kernel_page_present(struct page *page)
> pte_t *ptep;
> unsigned long addr = (unsigned long)page_address(page);
>
> + if (!debug_pagealloc_enabled() && !rodata_full)
> + return true;
> +
Ditto, not related to CONFIG_ARCH_HAS_SET_DIRECT_MAP.
