linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Ryan Roberts <ryan.roberts@arm.com>
To: Andrew Morton <akpm@linux-foundation.org>,
	Matthew Wilcox <willy@infradead.org>,
	Yin Fengwei <fengwei.yin@intel.com>,
	David Hildenbrand <david@redhat.com>, Yu Zhao <yuzhao@google.com>,
	Yang Shi <shy828301@gmail.com>,
	"Huang, Ying" <ying.huang@intel.com>, Zi Yan <ziy@nvidia.com>,
	Nathan Chancellor <nathan@kernel.org>
Cc: Ryan Roberts <ryan.roberts@arm.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: [PATCH v1] mm: Fix use-after-free for MMU_GATHER_NO_GATHER
Date: Thu, 27 Jul 2023 12:02:24 +0100	[thread overview]
Message-ID: <20230727110224.3333682-1-ryan.roberts@arm.com> (raw)

The recent change to batch-zap anonymous ptes did not take into account
that for platforms where MMU_GATHER_NO_GATHER is enabled (e.g. s390),
__tlb_remove_page() drops a reference to the page. This means that the
folio reference count can drop to zero while still in use (i.e. before
folio_remove_rmap_range() is called). This does not happen on other
platforms because the actual page freeing is deferred.

Solve this by appropriately getting/putting the folio to guarrantee it
does not get freed early.

Given the new need to get/put the folio in the batch path, let's stick
to the non-batched path if the folio is not large. In this case batching
is not helpful since the batch size is 1.

Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
Fixes: 904d9713b3b0 ("mm: batch-zap large anonymous folio PTE mappings")
Reported-by: Nathan Chancellor <nathan@kernel.org>
Link: https://lore.kernel.org/linux-mm/20230726161942.GA1123863@dev-arch.thelio-3990X/
---

Hi Andrew,

This fixes patch 3 in the series at [1], which is currently in mm-unstable. I'm
not sure whether you want to take the fix or whether I should re-post the entire
series?

Thanks,
Ryan


 mm/memory.c | 42 +++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 2130bad76eb1..808f6408a570 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1456,6 +1456,9 @@ static unsigned long try_zap_anon_pte_range(struct mmu_gather *tlb,
 	bool full;
 	int i;

+	/* __tlb_remove_page drops a ref; prevent it going to 0 while in use. */
+	folio_get(folio);
+
 	for (i = 0; i < nr_pages;) {
 		ptent = ptep_get_and_clear_full(mm, addr, pte, tlb->fullmm);
 		tlb_remove_tlb_entry(tlb, pte, addr);
@@ -1476,6 +1479,8 @@ static unsigned long try_zap_anon_pte_range(struct mmu_gather *tlb,

 	folio_remove_rmap_range(folio, page - i, i, vma);

+	folio_put(folio);
+
 	return i;
 }

@@ -1526,26 +1531,33 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
 			 */
 			if (page && PageAnon(page)) {
 				struct folio *folio = page_folio(page);
-				int nr_pages_req, nr_pages;

-				nr_pages_req = folio_nr_pages_cont_mapped(
-						folio, page, pte, addr, end);
+				if (folio_test_large(folio)) {
+					int nr_pages_req, nr_pages;
+					int counter = mm_counter(page);

-				nr_pages = try_zap_anon_pte_range(tlb, vma,
-						folio, page, pte, addr,
-						nr_pages_req, details);
+					nr_pages_req = folio_nr_pages_cont_mapped(
+							folio, page, pte, addr,
+							end);

-				rss[mm_counter(page)] -= nr_pages;
-				nr_pages--;
-				pte += nr_pages;
-				addr += nr_pages << PAGE_SHIFT;
+					/* folio may be freed on return. */
+					nr_pages = try_zap_anon_pte_range(
+							tlb, vma, folio, page,
+							pte, addr, nr_pages_req,
+							details);

-				if (unlikely(nr_pages < nr_pages_req)) {
-					force_flush = 1;
-					addr += PAGE_SIZE;
-					break;
+					rss[counter] -= nr_pages;
+					nr_pages--;
+					pte += nr_pages;
+					addr += nr_pages << PAGE_SHIFT;
+
+					if (unlikely(nr_pages < nr_pages_req)) {
+						force_flush = 1;
+						addr += PAGE_SIZE;
+						break;
+					}
+					continue;
 				}
-				continue;
 			}

 			ptent = ptep_get_and_clear_full(mm, addr, pte,
--
2.25.1



             reply	other threads:[~2023-07-27 11:02 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-27 11:02 Ryan Roberts [this message]
2023-07-27 11:15 ` David Hildenbrand
2023-07-27 11:35   ` Sven Schnelle

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=20230727110224.3333682-1-ryan.roberts@arm.com \
    --to=ryan.roberts@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=fengwei.yin@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=nathan@kernel.org \
    --cc=shy828301@gmail.com \
    --cc=willy@infradead.org \
    --cc=ying.huang@intel.com \
    --cc=yuzhao@google.com \
    --cc=ziy@nvidia.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox