All of lore.kernel.org
 help / color / mirror / Atom feed
From: toshi.kani@hpe.com (Kani, Toshi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] mm/vmalloc: Add interfaces to free unused page table
Date: Wed, 7 Mar 2018 23:02:12 +0000	[thread overview]
Message-ID: <1520466429.2693.43.camel@hpe.com> (raw)
In-Reply-To: <20180307145454.d3df4bed6d6431c52bcf271e@linux-foundation.org>

On Wed, 2018-03-07 at 14:54 -0800, Andrew Morton wrote:
> On Wed,  7 Mar 2018 11:32:26 -0700 Toshi Kani <toshi.kani@hpe.com> wrote:
> 
> > On architectures with CONFIG_HAVE_ARCH_HUGE_VMAP set, ioremap()
> > may create pud/pmd mappings.  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.
> > 
> > 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.
> > 
> > index 004abf9ebf12..942f4fa341f1 100644
> > --- a/arch/x86/mm/pgtable.c
> > +++ b/arch/x86/mm/pgtable.c
> > @@ -702,4 +702,24 @@ int pmd_clear_huge(pmd_t *pmd)
> >  
> >  	return 0;
> >  }
> > +
> > +/**
> > + * pud_free_pmd_page - clear pud entry and free pmd page
> > + *
> > + * Returns 1 on success and 0 on failure (pud not cleared).
> > + */
> > +int pud_free_pmd_page(pud_t *pud)
> > +{
> > +	return pud_none(*pud);
> > +}
> > +
> > +/**
> > + * pmd_free_pte_page - clear pmd entry and free pte page
> > + *
> > + * Returns 1 on success and 0 on failure (pmd not cleared).
> > + */
> > +int pmd_free_pte_page(pmd_t *pmd)
> > +{
> > +	return pmd_none(*pmd);
> > +}
> 
> Are these functions well named?  I mean, the comment says "clear pud
> entry and free pmd page" but the implementatin does neither of those
> things.  The name implies that the function frees a pmd_page but the
> callsites use the function as a way of querying state.

This patch 1/2 only implements stubs.  Patch 2/2 implements what is
described here.

> Also, as this fixes an arm64 kernel panic, should we be using
> cc:stable?

Right.

Thanks,
-Toshi

WARNING: multiple messages have this Message-ID (diff)
From: "Kani, Toshi" <toshi.kani@hpe.com>
To: "akpm@linux-foundation.org" <akpm@linux-foundation.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"bp@suse.de" <bp@suse.de>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"guohanjun@huawei.com" <guohanjun@huawei.com>,
	"wxf.wang@hisilicon.com" <wxf.wang@hisilicon.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"x86@kernel.org" <x86@kernel.org>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"will.deacon@arm.com" <will.deacon@arm.com>,
	"catalin.marinas@arm.com" <catalin.marinas@arm.com>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"Hocko, Michal" <mhocko@suse.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/2] mm/vmalloc: Add interfaces to free unused page table
Date: Wed, 7 Mar 2018 23:02:12 +0000	[thread overview]
Message-ID: <1520466429.2693.43.camel@hpe.com> (raw)
In-Reply-To: <20180307145454.d3df4bed6d6431c52bcf271e@linux-foundation.org>

On Wed, 2018-03-07 at 14:54 -0800, Andrew Morton wrote:
> On Wed,  7 Mar 2018 11:32:26 -0700 Toshi Kani <toshi.kani@hpe.com> wrote:
> 
> > On architectures with CONFIG_HAVE_ARCH_HUGE_VMAP set, ioremap()
> > may create pud/pmd mappings.  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.
> > 
> > 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.
> > 
> > index 004abf9ebf12..942f4fa341f1 100644
> > --- a/arch/x86/mm/pgtable.c
> > +++ b/arch/x86/mm/pgtable.c
> > @@ -702,4 +702,24 @@ int pmd_clear_huge(pmd_t *pmd)
> >  
> >  	return 0;
> >  }
> > +
> > +/**
> > + * pud_free_pmd_page - clear pud entry and free pmd page
> > + *
> > + * Returns 1 on success and 0 on failure (pud not cleared).
> > + */
> > +int pud_free_pmd_page(pud_t *pud)
> > +{
> > +	return pud_none(*pud);
> > +}
> > +
> > +/**
> > + * pmd_free_pte_page - clear pmd entry and free pte page
> > + *
> > + * Returns 1 on success and 0 on failure (pmd not cleared).
> > + */
> > +int pmd_free_pte_page(pmd_t *pmd)
> > +{
> > +	return pmd_none(*pmd);
> > +}
> 
> Are these functions well named?  I mean, the comment says "clear pud
> entry and free pmd page" but the implementatin does neither of those
> things.  The name implies that the function frees a pmd_page but the
> callsites use the function as a way of querying state.

This patch 1/2 only implements stubs.  Patch 2/2 implements what is
described here.

> Also, as this fixes an arm64 kernel panic, should we be using
> cc:stable?

Right.

Thanks,
-Toshi

  reply	other threads:[~2018-03-07 23:02 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-07 18:32 [PATCH 0/2] fix memory leak / panic in ioremap huge pages Toshi Kani
2018-03-07 18:32 ` Toshi Kani
2018-03-07 18:32 ` Toshi Kani
2018-03-07 18:32 ` [PATCH 1/2] mm/vmalloc: Add interfaces to free unused page table Toshi Kani
2018-03-07 18:32   ` Toshi Kani
2018-03-07 18:32   ` Toshi Kani
2018-03-07 22:54   ` Andrew Morton
2018-03-07 22:54     ` Andrew Morton
2018-03-07 23:02     ` Kani, Toshi [this message]
2018-03-07 23:02       ` Kani, Toshi
2018-03-07 22:55   ` Andrew Morton
2018-03-07 22:55     ` Andrew Morton
2018-03-08  4:00   ` Matthew Wilcox
2018-03-08  4:00     ` Matthew Wilcox
2018-03-08 15:56     ` Kani, Toshi
2018-03-08 15:56       ` Kani, Toshi
2018-03-08 22:07       ` Matthew Wilcox
2018-03-08 22:07         ` Matthew Wilcox
2018-03-08 23:27         ` Kani, Toshi
2018-03-08 23:27           ` Kani, Toshi
2018-03-08  8:08   ` Ingo Molnar
2018-03-08  8:08     ` Ingo Molnar
2018-03-08 18:04   ` Will Deacon
2018-03-08 18:04     ` Will Deacon
2018-03-08 19:30     ` Kani, Toshi
2018-03-08 19:30       ` Kani, Toshi
2018-03-07 18:32 ` [PATCH 2/2] x86/mm: implement free pmd/pte page interfaces Toshi Kani
2018-03-07 18:32   ` Toshi Kani
2018-03-07 18:32   ` Toshi Kani
2018-03-07 23:01   ` Andrew Morton
2018-03-07 23:01     ` Andrew Morton
2018-03-07 23:22     ` Kani, Toshi
2018-03-07 23:22       ` Kani, Toshi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1520466429.2693.43.camel@hpe.com \
    --to=toshi.kani@hpe.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.