mm/rmap: fix potential out-of-bounds page table access during batched unmap

PATCH Applied Lance Yang 6 participants 8 messages Jul 1, 2025 – Jul 16, 2025

Subsystems (4)

Stable branch Memory management RISC-V MM rmap

lore

Share & tools

lore

Lance Yang

author

2025-07-01 14:33 UTC #0 lore raw

PATCH v4 1/1 mm/rmap: fix potential out-of-bounds page table access during batched unmap

From: Lance Yang <[email protected]>

As pointed out by David[1], the batched unmap logic in try_to_unmap_one() may read past the end of a PTE table when a large folio's PTE mappings are not fully contained within a single page table.

While this scenario might be rare, an issue triggerable from userspace must be fixed regardless of its likelihood. This patch fixes the out-of-bounds access by refactoring the logic into a new helper, folio_unmap_pte_batch().

The new helper correctly calculates the safe batch size by capping the scan at both the VMA and PMD boundaries. To simplify the code, it also supports partial batching (i.e., any number of pages from 1 up to the calculated safe maximum), as there is no strong reason to special-case for fully mapped folios.

[1] https://lore.kernel.org/linux-mm/[email protected]

Cc <[email protected]>

Reported-by David Hildenbrand <[email protected]>

Closes https://lore.kernel.org/linux-mm/[email protected] open here

Fixes 354dffd29575 ("mm: support batched unmap for lazyfree large folios during reclamation")

Acked-by Barry Song <[email protected]>

Reviewed-by Lorenzo Stoakes <[email protected]>

Acked-by David Hildenbrand <[email protected]>

Signed-off-by Lance Yang <[email protected]>

---
v3 -> v4:
- Add Reported-by + Closes tags (per David)
- Pick RB from Lorenzo - thanks!
- Pick AB from David - thanks!
- https://lore.kernel.org/linux-mm/[email protected]

v2 -> v3:
- Tweak changelog (per Barry and David)
- Pick AB from Barry - thanks!
- https://lore.kernel.org/linux-mm/[email protected]

v1 -> v2:
- Update subject and changelog (per Barry)
- https://lore.kernel.org/linux-mm/[email protected]

 mm/rmap.c | 46 ++++++++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 18 deletions(-)

mm/rmap.c +28 ~~-18~~

index fb63d9256f09..1320b88fab74 100644
@@ -1845,23 +1845,32 @@ void folio_remove_rmap_pud(struct folio *folio, struct page *page,
 #endif
 }
 
-/* We support batch unmapping of PTEs for lazyfree large folios */
-static inline bool can_batch_unmap_folio_ptes(unsigned long addr,
-			struct folio *folio, pte_t *ptep)
+static inline unsigned int folio_unmap_pte_batch(struct folio *folio,
+			struct page_vma_mapped_walk *pvmw,
+			enum ttu_flags flags, pte_t pte)
 {
 	const fpb_t fpb_flags = FPB_IGNORE_DIRTY | FPB_IGNORE_SOFT_DIRTY;
-	int max_nr = folio_nr_pages(folio);
-	pte_t pte = ptep_get(ptep);
+	unsigned long end_addr, addr = pvmw->address;
+	struct vm_area_struct *vma = pvmw->vma;
+	unsigned int max_nr;
+
+	if (flags & TTU_HWPOISON)
+		return 1;
+	if (!folio_test_large(folio))
+		return 1;
 
+	/* We may only batch within a single VMA and a single page table. */
+	end_addr = pmd_addr_end(addr, vma->vm_end);
+	max_nr = (end_addr - addr) >> PAGE_SHIFT;
+
+	/* We only support lazyfree batching for now ... */
 	if (!folio_test_anon(folio) || folio_test_swapbacked(folio))
-		return false;
+		return 1;
 	if (pte_unused(pte))
-		return false;
-	if (pte_pfn(pte) != folio_pfn(folio))
-		return false;
+		return 1;
 
-	return folio_pte_batch(folio, addr, ptep, pte, max_nr, fpb_flags, NULL,
-			       NULL, NULL) == max_nr;
+	return folio_pte_batch(folio, addr, pvmw->pte, pte, max_nr, fpb_flags,
+			       NULL, NULL, NULL);
 }
 
 /*

@@ -2024,9 +2033,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
 			if (pte_dirty(pteval))
 				folio_mark_dirty(folio);
 		} else if (likely(pte_present(pteval))) {
-			if (folio_test_large(folio) && !(flags & TTU_HWPOISON) &&
-			    can_batch_unmap_folio_ptes(address, folio, pvmw.pte))
-				nr_pages = folio_nr_pages(folio);
+			nr_pages = folio_unmap_pte_batch(folio, &pvmw, flags, pteval);
 			end_addr = address + nr_pages * PAGE_SIZE;
 			flush_cache_range(vma, address, end_addr);

@@ -2206,13 +2213,16 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
 			hugetlb_remove_rmap(folio);
 		} else {
 			folio_remove_rmap_ptes(folio, subpage, nr_pages, vma);
-			folio_ref_sub(folio, nr_pages - 1);
 		}
 		if (vma->vm_flags & VM_LOCKED)
 			mlock_drain_local();
-		folio_put(folio);
-		/* We have already batched the entire folio */
-		if (nr_pages > 1)
+		folio_put_refs(folio, nr_pages);
+
+		/*
+		 * If we are sure that we batched the entire folio and cleared
+		 * all PTEs, we can just optimize and stop right here.
+		 */
+		if (nr_pages == folio_nr_pages(folio))
 			goto walk_done;
 		continue;
 walk_abort:

Signature · 1 line

2.49.0

Andrew Morton

maintainer

2025-07-01 21:17 UTC #1 lore raw

Quoted 3 lines · On Tue, 1 Jul 2025 22:31:00 +0800 Lance Yang <[email protected]> wrote:

- Add Reported-by + Closes tags (per David)
- Pick RB from Lorenzo - thanks!
- Pick AB from David - thanks!

It generally isn't necessary to resend a patch to add these things - I update the changelog in place as they come in.

In this case I'll grab that Reported-by: and Closes:, thanks.

Lance Yang

reviewer

2025-07-02 01:29 UTC #2 lore raw

Quoted 10 lines · Andrew Morton

Quoted 3 lines · On Tue, 1 Jul 2025 22:31:00 +0800 Lance Yang <[email protected]> wrote:
- Add Reported-by + Closes tags (per David)
- Pick RB from Lorenzo - thanks!
- Pick AB from David - thanks!
It generally isn't necessary to resend a patch to add these things - I update the changelog in place as they come in.
In this case I'll grab that Reported-by: and Closes:, thanks.

Ah, good to know that. Thanks for adding these tags!

Harry Yoo

Reviewed-by on v4 1/1

2025-07-07 05:40 UTC #3 lore raw

In reply to Lance Yang

Quoted 27 lines · Lance Yang

From: Lance Yang <[email protected]>
As pointed out by David[1], the batched unmap logic in try_to_unmap_one() may read past the end of a PTE table when a large folio's PTE mappings are not fully contained within a single page table.
While this scenario might be rare, an issue triggerable from userspace must be fixed regardless of its likelihood. This patch fixes the out-of-bounds access by refactoring the logic into a new helper, folio_unmap_pte_batch().
The new helper correctly calculates the safe batch size by capping the scan at both the VMA and PMD boundaries. To simplify the code, it also supports partial batching (i.e., any number of pages from 1 up to the calculated safe maximum), as there is no strong reason to special-case for fully mapped folios.
[1] https://lore.kernel.org/linux-mm/[email protected]
Cc <[email protected]>
Reported-by David Hildenbrand <[email protected]>
Closes https://lore.kernel.org/linux-mm/[email protected] open here
Fixes 354dffd29575 ("mm: support batched unmap for lazyfree large folios during reclamation")
Suggested-by Barry Song <[email protected]>
Acked-by Barry Song <[email protected]>
Reviewed-by Lorenzo Stoakes <[email protected]>
Acked-by David Hildenbrand <[email protected]>
Signed-off-by Lance Yang <[email protected]>

Quoted 1 line

---

LGTM,

Reviewed-by Harry Yoo <[email protected]>

With a minor comment below.

Quoted 13 lines

mm/rmap.c +0 -2

index fb63d9256f09..1320b88fab74 100644
@@ -2206,13 +2213,16 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
 			hugetlb_remove_rmap(folio);
 		} else {
 			folio_remove_rmap_ptes(folio, subpage, nr_pages, vma);
-			folio_ref_sub(folio, nr_pages - 1);
 		}
 		if (vma->vm_flags & VM_LOCKED)
 			mlock_drain_local();
-		folio_put(folio);

mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2226

Quoted 10 lines

mm/rmap.c +7 -2

-		/* We have already batched the entire folio */
-		if (nr_pages > 1)
+		folio_put_refs(folio, nr_pages);
+
+		/*
+		 * If we are sure that we batched the entire folio and cleared
+		 * all PTEs, we can just optimize and stop right here.
+		 */
+		if (nr_pages == folio_nr_pages(folio))
 			goto walk_done;

Just a minor comment.

We should probably teach page_vma_mapped_walk() to skip nr_pages pages, or just rely on next_pte: do { ... } while (pte_none(ptep_get(pvmw->pte))) loop in page_vma_mapped_walk() to skip those ptes?

Taking different paths depending on (nr_pages == folio_nr_pages(folio)) doesn't seem sensible.

Quoted 1 line

mm/rmap.c +0 -0
 		continue;

Signature · 2 lines

Cheers,
Harry / Hyeonggon

Lance Yang

reviewer

2025-07-07 09:13 UTC #4 lore raw

Quoted 30 lines · Harry Yoo

Quoted 27 lines · Lance Yang
From: Lance Yang <[email protected]>
As pointed out by David[1], the batched unmap logic in try_to_unmap_one() may read past the end of a PTE table when a large folio's PTE mappings are not fully contained within a single page table.
While this scenario might be rare, an issue triggerable from userspace must be fixed regardless of its likelihood. This patch fixes the out-of-bounds access by refactoring the logic into a new helper, folio_unmap_pte_batch().
The new helper correctly calculates the safe batch size by capping the scan at both the VMA and PMD boundaries. To simplify the code, it also supports partial batching (i.e., any number of pages from 1 up to the calculated safe maximum), as there is no strong reason to special-case for fully mapped folios.
[1] https://lore.kernel.org/linux-mm/[email protected]
Cc <[email protected]>
Reported-by David Hildenbrand <[email protected]>
Closes https://lore.kernel.org/linux-mm/[email protected] open here
Fixes 354dffd29575 ("mm: support batched unmap for lazyfree large folios during reclamation")
Suggested-by Barry Song <[email protected]>
Acked-by Barry Song <[email protected]>
Reviewed-by Lorenzo Stoakes <[email protected]>
Acked-by David Hildenbrand <[email protected]>
Signed-off-by Lance Yang <[email protected]>
Quoted 1 line
---

Quoted 2 lines

LGTM,
Reviewed-by Harry Yoo <[email protected]>

Hi Harry,

Thanks for taking time to review!

Quoted 29 lines

With a minor comment below.

Quoted 13 lines

mm/rmap.c +0 -2

index fb63d9256f09..1320b88fab74 100644
@@ -2206,13 +2213,16 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
  			hugetlb_remove_rmap(folio);
  		} else {
  			folio_remove_rmap_ptes(folio, subpage, nr_pages, vma);
-			folio_ref_sub(folio, nr_pages - 1);
  		}
  		if (vma->vm_flags & VM_LOCKED)
  			mlock_drain_local();
-		folio_put(folio);

mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2226

Quoted 10 lines

mm/rmap.c +7 -2

-		/* We have already batched the entire folio */
-		if (nr_pages > 1)
+		folio_put_refs(folio, nr_pages);
+
+		/*
+		 * If we are sure that we batched the entire folio and cleared
+		 * all PTEs, we can just optimize and stop right here.
+		 */
+		if (nr_pages == folio_nr_pages(folio))
  			goto walk_done;

Just a minor comment.

Quoted 3 lines

We should probably teach page_vma_mapped_walk() to skip nr_pages pages, or just rely on next_pte: do { ... } while (pte_none(ptep_get(pvmw->pte))) loop in page_vma_mapped_walk() to skip those ptes?

Good point. We handle partially-mapped folios by relying on the "next_pte" loop to skip those ptes. The common case we expect to handle is fully-mapped folios.

Quoted 3 lines

Taking different paths depending on (nr_pages == folio_nr_pages(folio)) doesn't seem sensible.

Adding more logic to page_vma_mapped_walk() for the rare partial-folio case seems like an over-optimization that would complicate the walker.

So, I'd prefer to keep it as is for now ;)

Barry Song

2025-07-07 15:41 UTC #5 lore raw

In reply to Harry Yoo

Quoted 62 lines · Harry Yoo

Quoted 27 lines · Lance Yang
From: Lance Yang <[email protected]>
As pointed out by David[1], the batched unmap logic in try_to_unmap_one() may read past the end of a PTE table when a large folio's PTE mappings are not fully contained within a single page table.
While this scenario might be rare, an issue triggerable from userspace must be fixed regardless of its likelihood. This patch fixes the out-of-bounds access by refactoring the logic into a new helper, folio_unmap_pte_batch().
The new helper correctly calculates the safe batch size by capping the scan at both the VMA and PMD boundaries. To simplify the code, it also supports partial batching (i.e., any number of pages from 1 up to the calculated safe maximum), as there is no strong reason to special-case for fully mapped folios.
[1] https://lore.kernel.org/linux-mm/[email protected]
Cc <[email protected]>
Reported-by David Hildenbrand <[email protected]>
Closes https://lore.kernel.org/linux-mm/[email protected] open here
Fixes 354dffd29575 ("mm: support batched unmap for lazyfree large folios during reclamation")
Suggested-by Barry Song <[email protected]>
Acked-by Barry Song <[email protected]>
Reviewed-by Lorenzo Stoakes <[email protected]>
Acked-by David Hildenbrand <[email protected]>
Signed-off-by Lance Yang <[email protected]>
Quoted 1 line
---
LGTM,
Reviewed-by Harry Yoo <[email protected]>
With a minor comment below.
Quoted 13 lines
mm/rmap.c +0 -2
index fb63d9256f09..1320b88fab74 100644
@@ -2206,13 +2213,16 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
                      hugetlb_remove_rmap(folio);
              } else {
                      folio_remove_rmap_ptes(folio, subpage, nr_pages, vma);
-                     folio_ref_sub(folio, nr_pages - 1);
              }
              if (vma->vm_flags & VM_LOCKED)
                      mlock_drain_local();
-             folio_put(folio);
mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2226
Quoted 10 lines
mm/rmap.c +7 -2
-             /* We have already batched the entire folio */
-             if (nr_pages > 1)
+             folio_put_refs(folio, nr_pages);
+
+             /*
+              * If we are sure that we batched the entire folio and cleared
+              * all PTEs, we can just optimize and stop right here.
+              */
+             if (nr_pages == folio_nr_pages(folio))
                      goto walk_done;
Just a minor comment.

Quoted 6 lines

We should probably teachhttps://lore.kernel.org/linux-mm/[email protected]/() to skip nr_pages pages, or just rely on next_pte: do { ... } while (pte_none(ptep_get(pvmw->pte)))
loop in page_vma_mapped_walk() to skip those ptes?
Taking different paths depending on (nr_pages == folio_nr_pages(folio)) doesn't seem sensible.

Hi Harry,

I believe we've already had this discussion here:
https://lore.kernel.org/linux-mm/[email protected]/

My main point is that nr_pages = folio_nr_pages(folio) is the typical/common case.
Also, modifying page_vma_mapped_walk() feels like a layering violation.

Quoted 6 lines

mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2227
Quoted 2 lines
mm/rmap.c +0 -0
              continue;
--
Cheers,
Harry / Hyeonggon

Thanks
Barry

Harry Yoo

2025-07-08 08:20 UTC #6 lore raw

Quoted 63 lines · Barry Song

Quoted 62 lines · Harry Yoo
Quoted 27 lines · Lance Yang
From: Lance Yang <[email protected]>
As pointed out by David[1], the batched unmap logic in try_to_unmap_one() may read past the end of a PTE table when a large folio's PTE mappings are not fully contained within a single page table.
While this scenario might be rare, an issue triggerable from userspace must be fixed regardless of its likelihood. This patch fixes the out-of-bounds access by refactoring the logic into a new helper, folio_unmap_pte_batch().
The new helper correctly calculates the safe batch size by capping the scan at both the VMA and PMD boundaries. To simplify the code, it also supports partial batching (i.e., any number of pages from 1 up to the calculated safe maximum), as there is no strong reason to special-case for fully mapped folios.
[1] https://lore.kernel.org/linux-mm/[email protected]
Cc <[email protected]>
Reported-by David Hildenbrand <[email protected]>
Closes https://lore.kernel.org/linux-mm/[email protected] open here
Fixes 354dffd29575 ("mm: support batched unmap for lazyfree large folios during reclamation")
Suggested-by Barry Song <[email protected]>
Acked-by Barry Song <[email protected]>
Reviewed-by Lorenzo Stoakes <[email protected]>
Acked-by David Hildenbrand <[email protected]>
Signed-off-by Lance Yang <[email protected]>
Quoted 1 line
---
LGTM,
Reviewed-by Harry Yoo <[email protected]>
With a minor comment below.
Quoted 13 lines
mm/rmap.c +0 -2
index fb63d9256f09..1320b88fab74 100644
@@ -2206,13 +2213,16 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
                      hugetlb_remove_rmap(folio);
              } else {
                      folio_remove_rmap_ptes(folio, subpage, nr_pages, vma);
-                     folio_ref_sub(folio, nr_pages - 1);
              }
              if (vma->vm_flags & VM_LOCKED)
                      mlock_drain_local();
-             folio_put(folio);
mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2226
Quoted 10 lines
mm/rmap.c +7 -2
-             /* We have already batched the entire folio */
-             if (nr_pages > 1)
+             folio_put_refs(folio, nr_pages);
+
+             /*
+              * If we are sure that we batched the entire folio and cleared
+              * all PTEs, we can just optimize and stop right here.
+              */
+             if (nr_pages == folio_nr_pages(folio))
                      goto walk_done;
Just a minor comment.

Quoted 8 lines

Quoted 6 lines
We should probably teachhttps://lore.kernel.org/linux-mm/[email protected]/() to skip nr_pages pages, or just rely on next_pte: do { ... } while (pte_none(ptep_get(pvmw->pte)))
loop in page_vma_mapped_walk() to skip those ptes?
Taking different paths depending on (nr_pages == folio_nr_pages(folio)) doesn't seem sensible.
Hi Harry,

Hi Lance and Barry.

Quoted 6 lines

I believe we've already had this discussion here:
https://lore.kernel.org/linux-mm/[email protected]/
My main point is that nr_pages = folio_nr_pages(folio) is the typical/common case.
Also, modifying page_vma_mapped_walk() feels like a layering violation.

Agreed. Perhaps it's not worth the trouble, nevermind :)

The patch looks good to me as-is.

Signature · 2 lines

Cheers,
Harry / Hyeonggon

[email protected]

CI Applied

2025-07-16 15:21 UTC #7 lore raw

In reply to Lance Yang

This patch was applied to riscv/linux.git (fixes) by Andrew Morton <[email protected]>:

Hello:

This patch was applied to riscv/linux.git (fixes)
by Andrew Morton <[email protected]>:

Quoted 11 lines · On Tue, 1 Jul 2025 22:31:00 +0800 you wrote:

From: Lance Yang <[email protected]>
As pointed out by David[1], the batched unmap logic in try_to_unmap_one() may read past the end of a PTE table when a large folio's PTE mappings are not fully contained within a single page table.
While this scenario might be rare, an issue triggerable from userspace must be fixed regardless of its likelihood. This patch fixes the out-of-bounds access by refactoring the logic into a new helper, folio_unmap_pte_batch().
[...]

Here is the summary with links:
- [v4,1/1] mm/rmap: fix potential out-of-bounds page table access during batched unmap

    https://git.kernel.org/riscv/c/ddd05742b45b

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html