[PATCH v3] arm: Fix memory attribute inconsistencies when using fixmap

[Jon Medhurst (Tixy)]

On Tue, 2017-04-04 at 08:31 +0100, Ard Biesheuvel wrote:
> From: Jon Medhurst <tixy at linaro.org>
> 
> To cope with the variety in ARM architectures and configurations, the
> pagetable attributes for kernel memory are generated at runtime to match
> the system the kernel finds itself on. This calculated value is stored
> in pgprot_kernel.
> 
> However, when early fixmap support was added for ARM (commit
> a5f4c561b3b1) the attributes used for mappings were hard coded because
> pgprot_kernel is not set up early enough. Unfortunately, when fixmap is
> used after early boot this means the memory being mapped can have
> different attributes to existing mappings, potentially leading to
> unpredictable behaviour. A specific problem also exists due to the hard
> coded values not include the 'shareable' attribute which means on
> systems where this matters (e.g. those with multiple CPU clusters) the
> cache contents for a memory location can become inconsistent between
> CPUs.
> 
> To resolve these issues we change fixmap to use the same memory
> attributes (from pgprot_kernel) that the rest of the kernel uses. To
> enable this we need to refactor the initialisation code so
> build_mem_type_table() is called early enough. Note, that relies on early
> param parsing for memory type overrides passed via the kernel command
> line, so we need to make sure this call is still after
> parse_early_params().
> 
> Fixes: a5f4c561b3b1 ("ARM: 8415/1: early fixmap support for earlycon")
> Cc: <stable at vger.kernel.org> # v4.3+
> Signed-off-by: Jon Medhurst <tixy at linaro.org>
> [ardb: keep early_fixmap_init() before param parsing, for earlycon]
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
[...]
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 4e016d7f37b3..ec81a30479aa 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -414,6 +414,11 @@ void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot)
>  		     FIXADDR_END);
>  	BUG_ON(idx >= __end_of_fixed_addresses);
>  
> +	/* we only support device mappings until pgprot_kernel has been set */
> +	if (WARN_ON(pgprot_val(prot) != pgprot_val(FIXMAP_PAGE_IO) &&
> +		    pgprot_val(pgprot_kernel) == 0))
> +		return;
> +

Hmm, on return from this function, isn't the caller then going to try
and access the memory at the fixmap address we didn't map, and so get a
page fault? If so, would a BUG_ON here be better?

>  	if (pgprot_val(prot))
>  		set_pte_at(NULL, vaddr, pte,
>  			pfn_pte(phys >> PAGE_SHIFT, prot));
> @@ -1616,7 +1621,6 @@ void __init paging_init(const struct machine_desc *mdesc)
>  {
>  	void *zero_page;
>  
> -	build_mem_type_table();
>  	prepare_page_table();
>  	map_lowmem();
>  	memblock_set_current_limit(arm_lowmem_limit);
> @@ -1636,3 +1640,9 @@ void __init paging_init(const struct machine_desc *mdesc)
>  	empty_zero_page = virt_to_page(zero_page);
>  	__flush_dcache_page(NULL, empty_zero_page);
>  }
> +
> +void __init early_mm_init(const struct machine_desc *mdesc)
> +{
> +	build_mem_type_table();
> +	early_paging_init(mdesc);
> +}

I tested this change with kprobes tests on TC2 (the setup that showed
the original bug) and compile tested for a nommu device (I will boot
test that in a bit as that's not quick to setup).

Also, with this patch we can now make early_paging_init() static now,
and remove the extern... 

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index f5db5548ad42..8ad1e4414024 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -80,7 +80,6 @@ __setup("fpe=", fpe_setup);
 
 extern void init_default_cache_policy(unsigned long);
 extern void paging_init(const struct machine_desc *desc);
-extern void early_paging_init(const struct machine_desc *);
 extern void adjust_lowmem_bounds(void);
 extern enum reboot_mode reboot_mode;
 extern void setup_dma_zone(const struct machine_desc *desc);
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index ec81a30479aa..347cca965783 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1497,7 +1497,7 @@ pgtables_remap lpae_pgtables_remap_asm;
  * early_paging_init() recreates boot time page table setup, allowing machines
  * to switch over to a high (>4G) address space on LPAE systems
  */
-void __init early_paging_init(const struct machine_desc *mdesc)
+static void __init early_paging_init(const struct machine_desc *mdesc)
 {
 	pgtables_remap *lpae_pgtables_remap;
 	unsigned long pa_pgd;
@@ -1565,7 +1565,7 @@ void __init early_paging_init(const struct machine_desc *mdesc)
 
 #else
 
-void __init early_paging_init(const struct machine_desc *mdesc)
+static void __init early_paging_init(const struct machine_desc *mdesc)
 {
 	long long offset;