Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 18 Jan 2018 18:40:26 +0300
From:   "Kirill A. Shutemov" <kirill@shutemov.name>
To:     Dave Hansen <dave.hansen@linux.intel.com>
Cc:     Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
        torvalds@linux-foundation.org, kirill.shutemov@linux.intel.com,
        akpm@linux-foundation.org, hannes@cmpxchg.org,
        iamjoonsoo.kim@lge.com, mgorman@techsingularity.net,
        tony.luck@intel.com, vbabka@suse.cz, mhocko@kernel.org,
        aarcange@redhat.com, hillf.zj@alibaba-inc.com, hughd@google.com,
        oleg@redhat.com, peterz@infradead.org, riel@redhat.com,
        srikar@linux.vnet.ibm.com, vdavydov.dev@gmail.com,
        mingo@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        x86@kernel.org
Subject: Re: [mm 4.15-rc8] Random oopses under memory pressure.
Message-ID: <20180118154026.jzdgdhkcxiliaulp@node.shutemov.name>
References: <201801160115.w0G1FOIG057203@www262.sakura.ne.jp>
 <CA+55aFxOn5n4O2JNaivi8rhDmeFhTQxEHD4xE33J9xOrFu=7kQ@mail.gmail.com>
 <201801170233.JDG21842.OFOJMQSHtOFFLV@I-love.SAKURA.ne.jp>
 <CA+55aFyxyjN0Mqnz66B4a0R+uR8DdfxdMhcg5rJVi8LwnpSRfA@mail.gmail.com>
 <201801172008.CHH39543.FFtMHOOVSQJLFO@I-love.SAKURA.ne.jp>
 <201801181712.BFD13039.LtHOSVMFJQFOFO@I-love.SAKURA.ne.jp>
 <20180118122550.2lhsjx7hg5drcjo4@node.shutemov.name>
 <d8347087-18a6-1709-8aa8-3c6f2d16aa94@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <d8347087-18a6-1709-8aa8-3c6f2d16aa94@linux.intel.com>
User-Agent: NeoMutt/20171215
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Jan 18, 2018 at 06:45:00AM -0800, Dave Hansen wrote:
> On 01/18/2018 04:25 AM, Kirill A. Shutemov wrote:
> > [   10.084024] diff: -858690919
> > [   10.084258] hpage_nr_pages: 1
> > [   10.084386] check1: 0
> > [   10.084478] check2: 0
> ...
> > diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> > index d22b84310f6d..57b4397f1ea5 100644
> > --- a/mm/page_vma_mapped.c
> > +++ b/mm/page_vma_mapped.c
> > @@ -70,6 +70,14 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
> >  		}
> >  		if (pte_page(*pvmw->pte) < pvmw->page)
> >  			return false;
> > +
> > +		if (pte_page(*pvmw->pte) - pvmw->page) {
> > +			printk("diff: %d\n", pte_page(*pvmw->pte) - pvmw->page);
> > +			printk("hpage_nr_pages: %d\n", hpage_nr_pages(pvmw->page));
> > +			printk("check1: %d\n", pte_page(*pvmw->pte) - pvmw->page < 0);
> > +			printk("check2: %d\n", pte_page(*pvmw->pte) - pvmw->page >= hpage_nr_pages(pvmw->page));
> > +			BUG();
> > +		}
> 
> This says that pte_page(*pvmw->pte) and pvmw->page are roughly 4GB away
> from each other (858690919*4=0xccba559c0).  That's not the compiler
> being wonky, it just means that the virtual addresses of the memory
> sections are that far apart.
> 
> This won't happen when you have vmemmap or flatmem because the mem_map[]
> is virtually contiguous and pointer arithmetic just works against all
> 'struct page' pointers.  But with classic sparsemem, it doesn't.
> 
> You need to make sure that the PFNs are in the same section before you
> can do the math that you want to do here.

Something like this?


From 251e124630da82482e8b320c73162ce89af04d5d Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Date: Thu, 18 Jan 2018 18:24:07 +0300
Subject: [PATCH] mm, page_vma_mapped: Fix pointer arithmetics in check_pte()

Tetsuo reported random crashes under memory pressure on 32-bit x86
system and tracked down to change that introduced
page_vma_mapped_walk().

The root cause of the issue is the faulty pointer math in check_pte().
As ->pte may point to an arbitrary page we have to check that they are
belong to the section before doing math. Otherwise it may lead to weird
results.

It wasn't noticed until now as mem_map[] is virtually contiguous on flatmem or
vmemmap sparsemem. Pointer arithmetic just works against all 'struct page'
pointers. But with classic sparsemem, it doesn't.

Let's restructure code a bit and add necessary check.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reported-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Fixes: ace71a19cec5 ("mm: introduce page_vma_mapped_walk()")
Cc: stable@vger.kernel.org
---
 mm/page_vma_mapped.c | 66 +++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 21 deletions(-)

diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
index d22b84310f6d..de195dcdfbd8 100644
--- a/mm/page_vma_mapped.c
+++ b/mm/page_vma_mapped.c
@@ -30,8 +30,28 @@ static bool map_pte(struct page_vma_mapped_walk *pvmw)
 	return true;
 }
 
+/**
+ * check_pte - check if @pvmw->page is mapped at the @pvmw->pte
+ *
+ * page_vma_mapped_walk() found a place where @pvmw->page is *potentially*
+ * mapped. check_pte() has to validate this.
+ *
+ * @pvmw->pte may point to empty PTE, swap PTE or PTE pointing to arbitrary
+ * page.
+ *
+ * If PVMW_MIGRATION flag is set, returns true if @pvmw->pte contains migration
+ * entry that points to @pvmw->page or any subpage in case of THP.
+ *
+ * If PVMW_MIGRATION flag is not set, returns true if @pvmw->pte points to
+ * @pvmw->page or any subpage in case of THP.
+ *
+ * Otherwise, return false.
+ *
+ */
 static bool check_pte(struct page_vma_mapped_walk *pvmw)
 {
+	struct page *page;
+
 	if (pvmw->flags & PVMW_MIGRATION) {
 #ifdef CONFIG_MIGRATION
 		swp_entry_t entry;
@@ -41,37 +61,41 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)
 
 		if (!is_migration_entry(entry))
 			return false;
-		if (migration_entry_to_page(entry) - pvmw->page >=
-				hpage_nr_pages(pvmw->page)) {
-			return false;
-		}
-		if (migration_entry_to_page(entry) < pvmw->page)
-			return false;
+
+		page = migration_entry_to_page(entry);
 #else
 		WARN_ON_ONCE(1);
 #endif
-	} else {
-		if (is_swap_pte(*pvmw->pte)) {
-			swp_entry_t entry;
+	} else if (is_swap_pte(*pvmw->pte)) {
+		swp_entry_t entry;
 
-			entry = pte_to_swp_entry(*pvmw->pte);
-			if (is_device_private_entry(entry) &&
-			    device_private_entry_to_page(entry) == pvmw->page)
-				return true;
-		}
+		/* Handle un-addressable ZONE_DEVICE memory */
+		entry = pte_to_swp_entry(*pvmw->pte);
+		if (!is_device_private_entry(entry))
+			return false;
 
+		page = device_private_entry_to_page(entry);
+	} else {
 		if (!pte_present(*pvmw->pte))
 			return false;
 
-		/* THP can be referenced by any subpage */
-		if (pte_page(*pvmw->pte) - pvmw->page >=
-				hpage_nr_pages(pvmw->page)) {
-			return false;
-		}
-		if (pte_page(*pvmw->pte) < pvmw->page)
-			return false;
+		page = pte_page(*pvmw->pte);
 	}
 
+	/*
+	 * Make sure that pages are in the same section before doing pointer
+	 * arithmetics.
+	 */
+	if (page_to_section(pvmw->page) != page_to_section(page))
+		return false;
+
+	if (page < pvmw->page)
+		return false;
+
+	/* THP can be referenced by any subpage */
+	if (page - pvmw->page >= hpage_nr_pages(pvmw->page))
+		return false;
+
 	return true;
 }
 
-- 
 Kirill A. Shutemov