mm/khugepaged: guard is_zero_pfn() calls with pte_present()

PATCH Applied Lance Yang 3 participants 3 messages Oct 20, 2025

Subsystems (2)

Memory management THP

lore

Share & tools

lore

Lance Yang

author reviewer

2025-10-20 15:11 UTC #0 lore raw

PATCH v3 1/1 mm/khugepaged: guard is_zero_pfn() calls with pte_present()

From: Lance Yang <[email protected]>

A non-present entry, like a swap PTE, contains completely different data (swap type and offset). pte_pfn() doesn't know this, so if we feed it a non-present entry, it will spit out a junk PFN.

What if that junk PFN happens to match the zeropage's PFN by sheer chance? While really unlikely, this would be really bad if it did.

So, let's fix this potential bug by ensuring all calls to is_zero_pfn() in khugepaged.c are properly guarded by a pte_present() check.

Reviewed-by Nico Pache <[email protected]>

Reviewed-by Dev Jain <[email protected]>

Reviewed-by Baolin Wang <[email protected]>

Reviewed-by Wei Yang <[email protected]>

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

---
Applies against commit a61ca1246ad3 in mm-new.

v2 -> v3:
- Collect Reviewed-by from Nico - thanks!
- Add a VM_WARN_ON_ONCE() for unexpected PTEs (per David)
- Introduce a pte_is_none_or_zero() helper to reduce duplication (per David and Lorenzo)
- https://lore.kernel.org/linux-mm/[email protected]/

v1 -> v2:
- Collect Reviewed-by from Dev, Wei and Baolin - thanks!
- Reduce a level of indentation (per Dev)
- https://lore.kernel.org/linux-mm/[email protected]/

 mm/khugepaged.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

mm/khugepaged.c +21 ~~-14~~

index d635d821f611..6f2ae2238b5b 100644
@@ -337,6 +337,13 @@ struct attribute_group khugepaged_attr_group = {
 };
 #endif /* CONFIG_SYSFS */
 
+static bool pte_none_or_zero(pte_t pte)
+{
+	if (pte_none(pte))
+		return true;
+	return pte_present(pte) && is_zero_pfn(pte_pfn(pte));
+}
+
 int hugepage_madvise(struct vm_area_struct *vma,
 		     vm_flags_t *vm_flags, int advice)
 {

@@ -518,6 +525,7 @@ static void release_pte_pages(pte_t *pte, pte_t *_pte,
 
 		if (pte_none(pteval))
 			continue;
+		VM_WARN_ON_ONCE(!pte_present(pteval));
 		pfn = pte_pfn(pteval);
 		if (is_zero_pfn(pfn))
 			continue;

@@ -548,8 +556,7 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 	for (_pte = pte; _pte < pte + HPAGE_PMD_NR;
 	     _pte++, addr += PAGE_SIZE) {
 		pte_t pteval = ptep_get(_pte);
-		if (pte_none(pteval) || (pte_present(pteval) &&
-				is_zero_pfn(pte_pfn(pteval)))) {
+		if (pte_none_or_zero(pteval)) {
 			++none_or_zero;
 			if (!userfaultfd_armed(vma) &&
 			    (!cc->is_khugepaged ||

@@ -690,17 +697,17 @@ static void __collapse_huge_page_copy_succeeded(pte_t *pte,
 	     address += nr_ptes * PAGE_SIZE) {
 		nr_ptes = 1;
 		pteval = ptep_get(_pte);
-		if (pte_none(pteval) || is_zero_pfn(pte_pfn(pteval))) {
+		if (pte_none_or_zero(pteval)) {
 			add_mm_counter(vma->vm_mm, MM_ANONPAGES, 1);
-			if (is_zero_pfn(pte_pfn(pteval))) {
-				/*
-				 * ptl mostly unnecessary.
-				 */
-				spin_lock(ptl);
-				ptep_clear(vma->vm_mm, address, _pte);
-				spin_unlock(ptl);
-				ksm_might_unmap_zero_page(vma->vm_mm, pteval);
-			}
+			if (pte_none(pteval))
+				continue;
+			/*
+			 * ptl mostly unnecessary.
+			 */
+			spin_lock(ptl);
+			ptep_clear(vma->vm_mm, address, _pte);
+			spin_unlock(ptl);
+			ksm_might_unmap_zero_page(vma->vm_mm, pteval);
 		} else {
 			struct page *src_page = pte_page(pteval);

@@ -794,7 +801,7 @@ static int __collapse_huge_page_copy(pte_t *pte, struct folio *folio,
 		unsigned long src_addr = address + i * PAGE_SIZE;
 		struct page *src_page;
 
-		if (pte_none(pteval) || is_zero_pfn(pte_pfn(pteval))) {
+		if (pte_none_or_zero(pteval)) {
 			clear_user_highpage(page, src_addr);
 			continue;
 		}

@@ -1294,7 +1301,7 @@ static int hpage_collapse_scan_pmd(struct mm_struct *mm,
 				goto out_unmap;
 			}
 		}
-		if (pte_none(pteval) || is_zero_pfn(pte_pfn(pteval))) {
+		if (pte_none_or_zero(pteval)) {
 			++none_or_zero;
 			if (!userfaultfd_armed(vma) &&
 			    (!cc->is_khugepaged ||

Signature · 1 line

2.49.0

David Hildenbrand

Acked-by on v3 1/1

2025-10-20 17:14 UTC #1 lore raw

Quoted 12 lines · Lance Yang

From: Lance Yang <[email protected]>
A non-present entry, like a swap PTE, contains completely different data (swap type and offset). pte_pfn() doesn't know this, so if we feed it a non-present entry, it will spit out a junk PFN.
What if that junk PFN happens to match the zeropage's PFN by sheer chance? While really unlikely, this would be really bad if it did.
So, let's fix this potential bug by ensuring all calls to is_zero_pfn() in khugepaged.c are properly guarded by a pte_present() check.

Quoted 7 lines

Suggested-by Lorenzo Stoakes <[email protected]>
Reviewed-by Nico Pache <[email protected]>
Reviewed-by Dev Jain <[email protected]>
Reviewed-by Baolin Wang <[email protected]>
Reviewed-by Wei Yang <[email protected]>
Signed-off-by Lance Yang <[email protected]>
---

Works for me!

Acked-by David Hildenbrand <[email protected]>

Signature · 3 lines

Cheers

David / dhildenb

Lorenzo Stoakes

Reviewed-by on v3 1/1

2025-10-20 17:46 UTC #2 lore raw

In reply to Lance Yang

Quoted 12 lines · Lance Yang

From: Lance Yang <[email protected]>
A non-present entry, like a swap PTE, contains completely different data (swap type and offset). pte_pfn() doesn't know this, so if we feed it a non-present entry, it will spit out a junk PFN.
What if that junk PFN happens to match the zeropage's PFN by sheer chance? While really unlikely, this would be really bad if it did.
So, let's fix this potential bug by ensuring all calls to is_zero_pfn() in khugepaged.c are properly guarded by a pte_present() check.

Quoted 6 lines

Suggested-by Lorenzo Stoakes <[email protected]>
Reviewed-by Nico Pache <[email protected]>
Reviewed-by Dev Jain <[email protected]>
Reviewed-by Baolin Wang <[email protected]>
Reviewed-by Wei Yang <[email protected]>
Signed-off-by Lance Yang <[email protected]>

This LGTM thanks for this, so:

Reviewed-by Lorenzo Stoakes <[email protected]>

Quoted 8 lines

---
Applies against commit a61ca1246ad3 in mm-new.
v2 -> v3:
- Collect Reviewed-by from Nico - thanks!
- Add a VM_WARN_ON_ONCE() for unexpected PTEs (per David)
- Introduce a pte_is_none_or_zero() helper to reduce duplication (per David and Lorenzo)

Wow I hadn't realised David had suggested that too, that was actually both of us doing that independently by chance lol.

I guess we agree then :)

Quoted 95 lines

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

v1 -> v2:
- Collect Reviewed-by from Dev, Wei and Baolin - thanks!
- Reduce a level of indentation (per Dev)
- https://lore.kernel.org/linux-mm/[email protected]/

 mm/khugepaged.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

mm/khugepaged.c +21 ~~-15~~

index d635d821f611..6f2ae2238b5b 100644
@@ -337,6 +337,13 @@ struct attribute_group khugepaged_attr_group = {
 };
 #endif /* CONFIG_SYSFS */
 
+static bool pte_none_or_zero(pte_t pte)
+{
+	if (pte_none(pte))
+		return true;
+	return pte_present(pte) && is_zero_pfn(pte_pfn(pte));
+}
+
 int hugepage_madvise(struct vm_area_struct *vma,
 		     vm_flags_t *vm_flags, int advice)
 {

@@ -518,6 +525,7 @@ static void release_pte_pages(pte_t *pte, pte_t *_pte,
 
 		if (pte_none(pteval))
 			continue;
+		VM_WARN_ON_ONCE(!pte_present(pteval));
 		pfn = pte_pfn(pteval);
 		if (is_zero_pfn(pfn))
 			continue;

@@ -548,8 +556,7 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 	for (_pte = pte; _pte < pte + HPAGE_PMD_NR;
 	     _pte++, addr += PAGE_SIZE) {
 		pte_t pteval = ptep_get(_pte);
-		if (pte_none(pteval) || (pte_present(pteval) &&
-				is_zero_pfn(pte_pfn(pteval)))) {
+		if (pte_none_or_zero(pteval)) {
 			++none_or_zero;
 			if (!userfaultfd_armed(vma) &&
 			    (!cc->is_khugepaged ||

@@ -690,17 +697,17 @@ static void __collapse_huge_page_copy_succeeded(pte_t *pte,
 	     address += nr_ptes * PAGE_SIZE) {
 		nr_ptes = 1;
 		pteval = ptep_get(_pte);
-		if (pte_none(pteval) || is_zero_pfn(pte_pfn(pteval))) {
+		if (pte_none_or_zero(pteval)) {
 			add_mm_counter(vma->vm_mm, MM_ANONPAGES, 1);
-			if (is_zero_pfn(pte_pfn(pteval))) {
-				/*
-				 * ptl mostly unnecessary.
-				 */
-				spin_lock(ptl);
-				ptep_clear(vma->vm_mm, address, _pte);
-				spin_unlock(ptl);
-				ksm_might_unmap_zero_page(vma->vm_mm, pteval);
-			}
+			if (pte_none(pteval))
+				continue;
+			/*
+			 * ptl mostly unnecessary.
+			 */
+			spin_lock(ptl);
+			ptep_clear(vma->vm_mm, address, _pte);
+			spin_unlock(ptl);
+			ksm_might_unmap_zero_page(vma->vm_mm, pteval);
 		} else {
 			struct page *src_page = pte_page(pteval);

@@ -794,7 +801,7 @@ static int __collapse_huge_page_copy(pte_t *pte, struct folio *folio,
 		unsigned long src_addr = address + i * PAGE_SIZE;
 		struct page *src_page;
 
-		if (pte_none(pteval) || is_zero_pfn(pte_pfn(pteval))) {
+		if (pte_none_or_zero(pteval)) {
 			clear_user_highpage(page, src_addr);
 			continue;
 		}

@@ -1294,7 +1301,7 @@ static int hpage_collapse_scan_pmd(struct mm_struct *mm,
 				goto out_unmap;
 			}
 		}
-		if (pte_none(pteval) || is_zero_pfn(pte_pfn(pteval))) {
+		if (pte_none_or_zero(pteval)) {
 			++none_or_zero;
 			if (!userfaultfd_armed(vma) &&
 			    (!cc->is_khugepaged ||
--

2.49.0