[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180327201700.xmgzgqox3sz3z32r@xps>
Date: Tue, 27 Mar 2018 15:17:00 -0500
From: Dan Rue <dan.rue@...aro.org>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: linux-kernel@...r.kernel.org, stable@...r.kernel.org,
Lei Li <lious.lilei@...ilicon.com>,
Toshi Kani <toshi.kani@....com>,
Catalin Marinas <catalin.marinas@....com>,
Wang Xuefeng <wxf.wang@...ilicon.com>,
Will Deacon <will.deacon@....com>,
Hanjun Guo <guohanjun@...wei.com>,
Michal Hocko <mhocko@...e.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...e.de>,
Matthew Wilcox <willy@...radead.org>,
Chintan Pandya <cpandya@...eaurora.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 4.4 20/43] mm/vmalloc: add interfaces to free unmapped
page table
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@....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@...ux-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@...ilicon.com>
> Signed-off-by: Toshi Kani <toshi.kani@....com>
> Cc: Catalin Marinas <catalin.marinas@....com>
> Cc: Wang Xuefeng <wxf.wang@...ilicon.com>
> Cc: Will Deacon <will.deacon@....com>
> Cc: Hanjun Guo <guohanjun@...wei.com>
> Cc: Michal Hocko <mhocko@...e.com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: "H. Peter Anvin" <hpa@...or.com>
> Cc: Borislav Petkov <bp@...e.de>
> Cc: Matthew Wilcox <willy@...radead.org>
> Cc: Chintan Pandya <cpandya@...eaurora.org>
> Cc: <stable@...r.kernel.org>
> Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.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;
> }
>
>
Powered by blists - more mailing lists