Re: [PATCH 4.4 20/43] mm/vmalloc: add interfaces to free unmapped page table

[Dan Rue <dan.rue@linaro.org>]

On Tue, Mar 27, 2018 at 06:27:24PM +0200, Greg Kroah-Hartman wrote:
> 4.4-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Toshi Kani <toshi.kani@hpe.com>
> 
> commit b6bdb7517c3d3f41f20e5c2948d6bc3f8897394e upstream.
> 
> On architectures with CONFIG_HAVE_ARCH_HUGE_VMAP set, ioremap() may
> create pud/pmd mappings.  A kernel panic was observed on arm64 systems
> with Cortex-A75 in the following steps as described by Hanjun Guo.
> 
>  1. ioremap a 4K size, valid page table will build,
>  2. iounmap it, pte0 will set to 0;
>  3. ioremap the same address with 2M size, pgd/pmd is unchanged,
>     then set the a new value for pmd;
>  4. pte0 is leaked;
>  5. CPU may meet exception because the old pmd is still in TLB,
>     which will lead to kernel panic.
> 
> This panic is not reproducible on x86.  INVLPG, called from iounmap,
> purges all levels of entries associated with purged address on x86.  x86
> still has memory leak.
> 
> The patch changes the ioremap path to free unmapped page table(s) since
> doing so in the unmap path has the following issues:
> 
>  - The iounmap() path is shared with vunmap(). Since vmap() only
>    supports pte mappings, making vunmap() to free a pte page is an
>    overhead for regular vmap users as they do not need a pte page freed
>    up.
> 
>  - Checking if all entries in a pte page are cleared in the unmap path
>    is racy, and serializing this check is expensive.
> 
>  - The unmap path calls free_vmap_area_noflush() to do lazy TLB purges.
>    Clearing a pud/pmd entry before the lazy TLB purges needs extra TLB
>    purge.
> 
> Add two interfaces, pud_free_pmd_page() and pmd_free_pte_page(), which
> clear a given pud/pmd entry and free up a page for the lower level
> entries.
> 
> This patch implements their stub functions on x86 and arm64, which work
> as workaround.
> 
> [akpm@linux-foundation.org: fix typo in pmd_free_pte_page() stub]
> Link: http://lkml.kernel.org/r/20180314180155.19492-2-toshi.kani@hpe.com
> Fixes: e61ce6ade404e ("mm: change ioremap to set up huge I/O mappings")
> Reported-by: Lei Li <lious.lilei@hisilicon.com>
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Wang Xuefeng <wxf.wang@hisilicon.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Hanjun Guo <guohanjun@huawei.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Chintan Pandya <cpandya@codeaurora.org>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

This patch causes the following build error on 4.4 arm64:

$ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=build-arm64 defconfig
$ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=build-arm64

  CC      arch/arm64/mm/mmu.o
../arch/arm64/mm/mmu.c:701:5: error: redefinition of ‘pud_free_pmd_page’
 int pud_free_pmd_page(pud_t *pud)
     ^~~~~~~~~~~~~~~~~
In file included from ../arch/arm64/include/asm/pgtable.h:682:0,
                 from ../include/linux/mm.h:55,
                 from ../include/linux/mman.h:4,
                 from ../arch/arm64/mm/mmu.c:25:
../include/asm-generic/pgtable.h:777:19: note: previous definition of ‘pud_free_pmd_page’ was here
 static inline int pud_free_pmd_page(pud_t *pud)
                   ^~~~~~~~~~~~~~~~~
../arch/arm64/mm/mmu.c:706:5: error: redefinition of ‘pmd_free_pte_page’
 int pmd_free_pte_page(pmd_t *pmd)
     ^~~~~~~~~~~~~~~~~
In file included from ../arch/arm64/include/asm/pgtable.h:682:0,
                 from ../include/linux/mm.h:55,
                 from ../include/linux/mman.h:4,
                 from ../arch/arm64/mm/mmu.c:25:
../include/asm-generic/pgtable.h:781:19: note: previous definition of ‘pmd_free_pte_page’ was here
 static inline int pmd_free_pte_page(pmd_t *pmd)
                   ^~~~~~~~~~~~~~~~~
make[2]: *** [../scripts/Makefile.build:270: arch/arm64/mm/mmu.o] Error 1
make[1]: *** [/home/drue/src/linux/4.4-rc/Makefile:969: arch/arm64/mm] Error 2
make[1]: Leaving directory '/home/drue/src/linux/4.4-rc/build-arm64'
make: *** [Makefile:152: sub-make] Error 2


> 
> ---
>  arch/arm64/mm/mmu.c           |   10 ++++++++++
>  arch/x86/mm/pgtable.c         |   24 ++++++++++++++++++++++++
>  include/asm-generic/pgtable.h |   10 ++++++++++
>  lib/ioremap.c                 |    6 ++++--
>  4 files changed, 48 insertions(+), 2 deletions(-)
> 
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -697,3 +697,13 @@ void *__init fixmap_remap_fdt(phys_addr_
>  
>  	return dt_virt;
>  }
> +
> +int pud_free_pmd_page(pud_t *pud)
> +{
> +	return pud_none(*pud);
> +}
> +
> +int pmd_free_pte_page(pmd_t *pmd)
> +{
> +	return pmd_none(*pmd);
> +}
> --- a/arch/x86/mm/pgtable.c
> +++ b/arch/x86/mm/pgtable.c
> @@ -666,4 +666,28 @@ int pmd_clear_huge(pmd_t *pmd)
>  
>  	return 0;
>  }
> +
> +/**
> + * pud_free_pmd_page - Clear pud entry and free pmd page.
> + * @pud: Pointer to a PUD.
> + *
> + * Context: The pud range has been unmaped and TLB purged.
> + * Return: 1 if clearing the entry succeeded. 0 otherwise.
> + */
> +int pud_free_pmd_page(pud_t *pud)
> +{
> +	return pud_none(*pud);
> +}
> +
> +/**
> + * pmd_free_pte_page - Clear pmd entry and free pte page.
> + * @pmd: Pointer to a PMD.
> + *
> + * Context: The pmd range has been unmaped and TLB purged.
> + * Return: 1 if clearing the entry succeeded. 0 otherwise.
> + */
> +int pmd_free_pte_page(pmd_t *pmd)
> +{
> +	return pmd_none(*pmd);
> +}
>  #endif	/* CONFIG_HAVE_ARCH_HUGE_VMAP */
> --- a/include/asm-generic/pgtable.h
> +++ b/include/asm-generic/pgtable.h
> @@ -755,6 +755,8 @@ int pud_set_huge(pud_t *pud, phys_addr_t
>  int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot);
>  int pud_clear_huge(pud_t *pud);
>  int pmd_clear_huge(pmd_t *pmd);
> +int pud_free_pmd_page(pud_t *pud);
> +int pmd_free_pte_page(pmd_t *pmd);
>  #else	/* !CONFIG_HAVE_ARCH_HUGE_VMAP */
>  static inline int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
>  {
> @@ -772,6 +774,14 @@ static inline int pmd_clear_huge(pmd_t *
>  {
>  	return 0;
>  }
> +static inline int pud_free_pmd_page(pud_t *pud)
> +{
> +	return 0;
> +}
> +static inline int pmd_free_pte_page(pmd_t *pmd)
> +{
> +	return 0;
> +}
>  #endif	/* CONFIG_HAVE_ARCH_HUGE_VMAP */
>  
>  #endif /* !__ASSEMBLY__ */
> --- a/lib/ioremap.c
> +++ b/lib/ioremap.c
> @@ -83,7 +83,8 @@ static inline int ioremap_pmd_range(pud_
>  
>  		if (ioremap_pmd_enabled() &&
>  		    ((next - addr) == PMD_SIZE) &&
> -		    IS_ALIGNED(phys_addr + addr, PMD_SIZE)) {
> +		    IS_ALIGNED(phys_addr + addr, PMD_SIZE) &&
> +		    pmd_free_pte_page(pmd)) {
>  			if (pmd_set_huge(pmd, phys_addr + addr, prot))
>  				continue;
>  		}
> @@ -109,7 +110,8 @@ static inline int ioremap_pud_range(pgd_
>  
>  		if (ioremap_pud_enabled() &&
>  		    ((next - addr) == PUD_SIZE) &&
> -		    IS_ALIGNED(phys_addr + addr, PUD_SIZE)) {
> +		    IS_ALIGNED(phys_addr + addr, PUD_SIZE) &&
> +		    pud_free_pmd_page(pud)) {
>  			if (pud_set_huge(pud, phys_addr + addr, prot))
>  				continue;
>  		}
> 
>