[PATCH v2 0/3] mm: khugepaged: fix tracepoint UAF

[PATCH v2 0/3] mm: khugepaged: fix tracepoint UAF

[Vernon Yang]

From: Vernon Yang <yanglincheng@kylinos.cn>

The khugepaged tracepoints take a folio pointer and call folio_pfn(),
but by then the folio may no longer be valid: freed after folio_put(),
folio_unlock() or pte_unmap_unlock(), or not a folio at all but an
xarray-encoded swap entry. On classic SPARSEMEM, dereferencing it oopses
khugepaged as soon as the trace event is enabled; on other memory models
it merely prints a bogus pfn.

Pass the pfn to the tracepoints directly, captured while the folio is
still pinned, closing the use-after-free windows in
mm_khugepaged_scan_file(), mm_khugepaged_scan_pmd() and
mm_khugepaged_collapse_file().

V1 -> V2:
- Instead of passing the folio, just pass the pfn directly.
- Using the folio_pfn() before dropping the reference or the page table
  lock.

V1 : https://lore.kernel.org/linux-mm/20260811133655.267739-1-vernon2gm@gmail.com/

Vernon Yang (3):
  mm: khugepaged: fix swap entry value to folio_pfn()
  mm: khugepaged: fix folio is used after pte_unmap_unlock()
  mm: khugepaged: fix folio is used after folio_put/unlock()

 include/trace/events/huge_memory.h | 18 +++++++++---------
 mm/khugepaged.c                    | 23 ++++++++++++++++-------
 2 files changed, 25 insertions(+), 16 deletions(-)


base-commit: 075b74841bd0065a3bda3440873c747938e69b68
--
2.53.0

[PATCH v2 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[Vernon Yang]

From: Vernon Yang <yanglincheng@kylinos.cn>

When the swap entries found exceed max_ptes_swap, the loop is left via
break with folio still holding the xarray value that encodes the swap
entry, not valid folio pointer.

That value is passed to trace_mm_khugepaged_scan_file(), which feeds it
to folio_pfn(). On FLATMEM and SPARSEMEM_VMEMMAP, the page_to_pfn() is
plain pointer arithmetic, so the trace event merely prints bogus
scan_pfn. On classic SPARSEMEM, the page_to_pfn() reads page->flags,
dereferencing the tiny encoded integer and oopsing khugepaged whenever
the trace event is enabled.

So when folio is the swap entry value, simply set pfn to -1, just like
exhausted scan naturally.

And the folio_put() has maybe dropped the last reference of folio. The
trace_mm_khugepaged_scan_file() is left with a dangling folio pointer.
so using the folio_pfn() before dropping the reference, closing
use-after-free window.

Fixes: d41fd2016ed0 ("mm/khugepaged: add tracepoint to hpage_collapse_scan_file()")
Cc: stable@vger.kernel.org
Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
---
 include/trace/events/huge_memory.h |  6 +++---
 mm/khugepaged.c                    | 14 +++++++++-----
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
index 291fae364c62..d3572d4ef453 100644
--- a/include/trace/events/huge_memory.h
+++ b/include/trace/events/huge_memory.h
@@ -178,10 +178,10 @@ TRACE_EVENT(mm_collapse_huge_page_swapin,
 
 TRACE_EVENT(mm_khugepaged_scan_file,
 
-	TP_PROTO(struct mm_struct *mm, struct folio *folio, struct file *file,
+	TP_PROTO(struct mm_struct *mm, unsigned long pfn, struct file *file,
 		 int present, int swap, int result),
 
-	TP_ARGS(mm, folio, file, present, swap, result),
+	TP_ARGS(mm, pfn, file, present, swap, result),
 
 	TP_STRUCT__entry(
 		__field(struct mm_struct *, mm)
@@ -194,7 +194,7 @@ TRACE_EVENT(mm_khugepaged_scan_file,
 
 	TP_fast_assign(
 		__entry->mm = mm;
-		__entry->pfn = folio ? folio_pfn(folio) : -1;
+		__entry->pfn = pfn;
 		__assign_str(filename);
 		__entry->present = present;
 		__entry->swap = swap;
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 617bca76db49..e7830761d3a2 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -2683,6 +2683,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
 	int present, swap;
 	int node = NUMA_NO_NODE;
 	enum scan_result result = SCAN_SUCCEED;
+	unsigned long pfn;
 
 	present = 0;
 	swap = 0;
@@ -2720,27 +2721,23 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
 			 * PMD-sized THP implies that we can only try
 			 * retracting the PTE table.
 			 */
-			folio_put(folio);
 			break;
 		}
 
 		node = folio_nid(folio);
 		if (collapse_scan_abort(node, cc)) {
 			result = SCAN_SCAN_ABORT;
-			folio_put(folio);
 			break;
 		}
 		cc->node_load[node]++;
 
 		if (!folio_test_lru(folio)) {
 			result = SCAN_PAGE_LRU;
-			folio_put(folio);
 			break;
 		}
 
 		if (folio_expected_ref_count(folio) + 1 != folio_ref_count(folio)) {
 			result = SCAN_PAGE_COUNT;
-			folio_put(folio);
 			break;
 		}
 
@@ -2759,7 +2756,14 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
 			cond_resched_rcu();
 		}
 	}
+	if (!folio || xa_is_value(folio)) {
+		pfn = -1;
+	} else {
+		pfn = folio_pfn(folio);
+		folio_put(folio);
+	}
 	rcu_read_unlock();
+
 	if (result == SCAN_PTE_MAPPED_HUGEPAGE)
 		cc->progress++;
 	else
@@ -2774,7 +2778,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
 		}
 	}
 
-	trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result);
+	trace_mm_khugepaged_scan_file(mm, pfn, file, present, swap, result);
 	return result;
 }
 
-- 
2.53.0

[PATCH v2 2/3] mm: khugepaged: fix folio is used after pte_unmap_unlock()

[Vernon Yang]

From: Vernon Yang <yanglincheng@kylinos.cn>

After the page table lock has dropped, the folio can be freed
concurrently. The trace_mm_khugepaged_scan_pmd() is left with
a dangling folio pointer.

So using the folio_pfn() before dropping the page table lock,
closing use-after-free window.

Fixes: 7d2eba0557c1 ("mm: add tracepoint for scanning pages")
Cc: stable@vger.kernel.org
Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
---
 include/trace/events/huge_memory.h | 6 +++---
 mm/khugepaged.c                    | 4 +++-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
index d3572d4ef453..5dc71d292f47 100644
--- a/include/trace/events/huge_memory.h
+++ b/include/trace/events/huge_memory.h
@@ -55,10 +55,10 @@ SCAN_STATUS
 
 TRACE_EVENT(mm_khugepaged_scan_pmd,
 
-	TP_PROTO(struct mm_struct *mm, struct folio *folio,
+	TP_PROTO(struct mm_struct *mm, unsigned long pfn,
 		 int referenced, int none_or_zero, int status, int unmapped),
 
-	TP_ARGS(mm, folio, referenced, none_or_zero, status, unmapped),
+	TP_ARGS(mm, pfn, referenced, none_or_zero, status, unmapped),
 
 	TP_STRUCT__entry(
 		__field(struct mm_struct *, mm)
@@ -71,7 +71,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
 
 	TP_fast_assign(
 		__entry->mm = mm;
-		__entry->pfn = folio ? folio_pfn(folio) : -1;
+		__entry->pfn = pfn;
 		__entry->referenced = referenced;
 		__entry->none_or_zero = none_or_zero;
 		__entry->status = status;
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index e7830761d3a2..7c8c48577408 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1603,6 +1603,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
 	enum scan_result result = SCAN_FAIL;
 	struct page *page = NULL;
 	struct folio *folio = NULL;
+	unsigned long pfn = -1;
 	unsigned long addr;
 	unsigned long enabled_orders;
 	spinlock_t *ptl;
@@ -1778,6 +1779,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
 		result = SCAN_SUCCEED;
 	}
 out_unmap:
+	pfn = folio ? folio_pfn(folio) : -1;
 	pte_unmap_unlock(pte, ptl);
 	if (result == SCAN_SUCCEED) {
 		/* collapse_huge_page expects the lock to be dropped before calling */
@@ -1788,7 +1790,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
 		*lock_dropped = true;
 	}
 out:
-	trace_mm_khugepaged_scan_pmd(mm, folio, referenced,
+	trace_mm_khugepaged_scan_pmd(mm, pfn, referenced,
 				     none_or_zero, result, unmapped);
 	return result;
 }
-- 
2.53.0

[PATCH v2 3/3] mm: khugepaged: fix folio is used after folio_put/unlock()

[Vernon Yang]

From: Vernon Yang <yanglincheng@kylinos.cn>

On the rollback path, folio_put() has already dropped the last reference
of new_folio. On the success path, new_folio is already unlocked and can
be freed concurrently. The trace_mm_khugepaged_collapse_file() is left
with a dangling folio pointer.

So using the folio_pfn() before dropping the reference, closing
use-after-free window.

Fixes: 4c9473e87e75 ("mm/khugepaged: add tracepoint to collapse_file()")
Cc: stable@vger.kernel.org
Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
---
 include/trace/events/huge_memory.h | 6 +++---
 mm/khugepaged.c                    | 5 ++++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
index 5dc71d292f47..1b301d4e2dba 100644
--- a/include/trace/events/huge_memory.h
+++ b/include/trace/events/huge_memory.h
@@ -211,10 +211,10 @@ TRACE_EVENT(mm_khugepaged_scan_file,
 );
 
 TRACE_EVENT(mm_khugepaged_collapse_file,
-	TP_PROTO(struct mm_struct *mm, struct folio *new_folio, pgoff_t index,
+	TP_PROTO(struct mm_struct *mm, unsigned long new_pfn, pgoff_t index,
 			unsigned long addr, bool is_shmem, struct file *file,
 			int nr, int result),
-	TP_ARGS(mm, new_folio, index, addr, is_shmem, file, nr, result),
+	TP_ARGS(mm, new_pfn, index, addr, is_shmem, file, nr, result),
 	TP_STRUCT__entry(
 		__field(struct mm_struct *, mm)
 		__field(unsigned long, hpfn)
@@ -228,7 +228,7 @@ TRACE_EVENT(mm_khugepaged_collapse_file,
 
 	TP_fast_assign(
 		__entry->mm = mm;
-		__entry->hpfn = new_folio ? folio_pfn(new_folio) : -1;
+		__entry->hpfn = new_pfn;
 		__entry->index = index;
 		__entry->addr = addr;
 		__entry->is_shmem = is_shmem;
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 7c8c48577408..7df0fd2d2fdc 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -2247,6 +2247,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
 	struct address_space *mapping = file->f_mapping;
 	struct page *dst;
 	struct folio *folio, *tmp, *new_folio;
+	unsigned long new_pfn = -1;
 	pgoff_t index = 0, end = start + HPAGE_PMD_NR;
 	LIST_HEAD(pagelist);
 	XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER);
@@ -2626,6 +2627,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
 	retract_page_tables(mapping, start);
 	if (cc && !cc->is_khugepaged)
 		result = SCAN_PTE_MAPPED_HUGEPAGE;
+	new_pfn = folio_pfn(new_folio);
 	folio_unlock(new_folio);
 
 	/*
@@ -2664,12 +2666,13 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
 	}
 
 	new_folio->mapping = NULL;
+	new_pfn = folio_pfn(new_folio);
 
 	folio_unlock(new_folio);
 	folio_put(new_folio);
 out:
 	VM_BUG_ON(!list_empty(&pagelist));
-	trace_mm_khugepaged_collapse_file(mm, new_folio, index, addr, is_shmem, file, HPAGE_PMD_NR, result);
+	trace_mm_khugepaged_collapse_file(mm, new_pfn, index, addr, is_shmem, file, HPAGE_PMD_NR, result);
 	return result;
 }
 
-- 
2.53.0

Re: [PATCH v2 0/3] mm: khugepaged: fix tracepoint UAF

[Lance Yang]

On Sat, Aug 15, 2026 at 01:19:21PM +0800, Vernon Yang wrote:
>From: Vernon Yang <yanglincheng@kylinos.cn>
>
>The khugepaged tracepoints take a folio pointer and call folio_pfn(),
>but by then the folio may no longer be valid: freed after folio_put(),
>folio_unlock() or pte_unmap_unlock(), or not a folio at all but an
>xarray-encoded swap entry. On classic SPARSEMEM, dereferencing it oopses
>khugepaged as soon as the trace event is enabled; on other memory models
>it merely prints a bogus pfn.
>
>Pass the pfn to the tracepoints directly, captured while the folio is
>still pinned, closing the use-after-free windows in
>mm_khugepaged_scan_file(), mm_khugepaged_scan_pmd() and
>mm_khugepaged_collapse_file().

Well spotted!

Gave the series a run on x86_64 (KVM), all good (only classic SPARSEMEM
untested) :)

Tested-by: Lance Yang <lance.yang@linux.dev>

Re: [PATCH v2 0/3] mm: khugepaged: fix tracepoint UAF

[Lance Yang]

+Cc Baolin

On Sun, Aug 16, 2026 at 01:44:44AM +0800, Lance Yang wrote:
>
>On Sat, Aug 15, 2026 at 01:19:21PM +0800, Vernon Yang wrote:
>>From: Vernon Yang <yanglincheng@kylinos.cn>
>>
>>The khugepaged tracepoints take a folio pointer and call folio_pfn(),
>>but by then the folio may no longer be valid: freed after folio_put(),
>>folio_unlock() or pte_unmap_unlock(), or not a folio at all but an
>>xarray-encoded swap entry. On classic SPARSEMEM, dereferencing it oopses
>>khugepaged as soon as the trace event is enabled; on other memory models
>>it merely prints a bogus pfn.
>>
>>Pass the pfn to the tracepoints directly, captured while the folio is
>>still pinned, closing the use-after-free windows in
>>mm_khugepaged_scan_file(), mm_khugepaged_scan_pmd() and
>>mm_khugepaged_collapse_file().
>
>Well spotted!
>
>Gave the series a run on x86_64 (KVM), all good (only classic SPARSEMEM
>untested) :)

Hmm ... stumbled over something else while testing this ...

With tmpfs mounted huge=advise, one MADV_HUGEPAGE isn't enough to get
an unregistered mm onto khugepaged's list. Do it twice, and khugepaged
starts scanning right away.

The pending flags make it into khugepaged just fine:

int hugepage_madvise(struct vm_area_struct *vma,
		     vm_flags_t *vm_flags, int advice)
{
	switch (advice) {
	case MADV_HUGEPAGE:
		*vm_flags &= ~VM_NOHUGEPAGE;
		*vm_flags |= VM_HUGEPAGE;
...
		khugepaged_enter_vma(vma, *vm_flags);
		break;
...
	}

	return 0;
}

and survive the common eligibility check:

unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
					 vm_flags_t vm_flags,
					 enum tva_type type,
					 unsigned long orders)
{
...
	/*
	 * Enabled via shmem mount options or sysfs settings.
	 * Must be done before hugepage flags check since shmem has its
	 * own flags.
	 */
	if (!in_pf && shmem_file(vma->vm_file))
		return orders & shmem_allowable_huge_orders(file_inode(vma->vm_file),
						   vma, vma_start_pgoff(vma), 0,
						   forced_collapse);
...
}

But then the shmem helper reads them back from the VMA:

unsigned long shmem_allowable_huge_orders(struct inode *inode,
				struct vm_area_struct *vma, pgoff_t index,
				loff_t write_end, bool shmem_huge_force)
{
...
	vm_flags_t vm_flags = vma ? vma->vm_flags : 0;
...
}

At that point vma->vm_flags still has the old value, so huge=advise
quietly gives us no allowable order.

First madvise still succeeds, of course. The second one works because it
finds VM_HUGEPAGE already installed by the first call.

Looked at history too ... we've been here before. 2cf1338454a8 ("mm: fix
khugepaged with shmem_enabled=advise") fixed this exact ordering bug and
tagged cd89fb065099 as the culprit. Then 6beeab870e70 was meant to be
"No functional changes", but moving shmem_huge_global_enabled() into
shmem_allowable_huge_orders() seems to have wired the stale vma->vm_flags
read back in.

So AFAICT, this regressed in v6.12 with 6beeab870e70.

@Baolin, does that ring a bell? Any reason shmem_allowable_huge_orders()
can't just take the pending vm_flags as well?

Cheers, Lance

Re: [PATCH v2 0/3] mm: khugepaged: fix tracepoint UAF

[Baolin Wang]

Hi Lance,

On 8/16/26 2:16 AM, Lance Yang wrote:
> +Cc Baolin
> 
> On Sun, Aug 16, 2026 at 01:44:44AM +0800, Lance Yang wrote:
>>
>> On Sat, Aug 15, 2026 at 01:19:21PM +0800, Vernon Yang wrote:
>>> From: Vernon Yang <yanglincheng@kylinos.cn>
>>>
>>> The khugepaged tracepoints take a folio pointer and call folio_pfn(),
>>> but by then the folio may no longer be valid: freed after folio_put(),
>>> folio_unlock() or pte_unmap_unlock(), or not a folio at all but an
>>> xarray-encoded swap entry. On classic SPARSEMEM, dereferencing it oopses
>>> khugepaged as soon as the trace event is enabled; on other memory models
>>> it merely prints a bogus pfn.
>>>
>>> Pass the pfn to the tracepoints directly, captured while the folio is
>>> still pinned, closing the use-after-free windows in
>>> mm_khugepaged_scan_file(), mm_khugepaged_scan_pmd() and
>>> mm_khugepaged_collapse_file().
>>
>> Well spotted!
>>
>> Gave the series a run on x86_64 (KVM), all good (only classic SPARSEMEM
>> untested) :)
> 
> Hmm ... stumbled over something else while testing this ...
> 
> With tmpfs mounted huge=advise, one MADV_HUGEPAGE isn't enough to get
> an unregistered mm onto khugepaged's list. Do it twice, and khugepaged
> starts scanning right away.
> 
> The pending flags make it into khugepaged just fine:
> 
> int hugepage_madvise(struct vm_area_struct *vma,
> 		     vm_flags_t *vm_flags, int advice)
> {
> 	switch (advice) {
> 	case MADV_HUGEPAGE:
> 		*vm_flags &= ~VM_NOHUGEPAGE;
> 		*vm_flags |= VM_HUGEPAGE;
> ...
> 		khugepaged_enter_vma(vma, *vm_flags);
> 		break;
> ...
> 	}
> 
> 	return 0;
> }
> 
> and survive the common eligibility check:
> 
> unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
> 					 vm_flags_t vm_flags,
> 					 enum tva_type type,
> 					 unsigned long orders)
> {
> ...
> 	/*
> 	 * Enabled via shmem mount options or sysfs settings.
> 	 * Must be done before hugepage flags check since shmem has its
> 	 * own flags.
> 	 */
> 	if (!in_pf && shmem_file(vma->vm_file))
> 		return orders & shmem_allowable_huge_orders(file_inode(vma->vm_file),
> 						   vma, vma_start_pgoff(vma), 0,
> 						   forced_collapse);
> ...
> }
> 
> But then the shmem helper reads them back from the VMA:
> 
> unsigned long shmem_allowable_huge_orders(struct inode *inode,
> 				struct vm_area_struct *vma, pgoff_t index,
> 				loff_t write_end, bool shmem_huge_force)
> {
> ...
> 	vm_flags_t vm_flags = vma ? vma->vm_flags : 0;
> ...
> }
> 
> At that point vma->vm_flags still has the old value, so huge=advise
> quietly gives us no allowable order.
> 
> First madvise still succeeds, of course. The second one works because it
> finds VM_HUGEPAGE already installed by the first call.
> 
> Looked at history too ... we've been here before. 2cf1338454a8 ("mm: fix
> khugepaged with shmem_enabled=advise") fixed this exact ordering bug and
> tagged cd89fb065099 as the culprit. Then 6beeab870e70 was meant to be
> "No functional changes", but moving shmem_huge_global_enabled() into
> shmem_allowable_huge_orders() seems to have wired the stale vma->vm_flags
> read back in.
> 
> So AFAICT, this regressed in v6.12 with 6beeab870e70.
> 
> @Baolin, does that ring a bell? Any reason shmem_allowable_huge_orders()
> can't just take the pending vm_flags as well?

Good catch. Sorry for my mistake. Would you like to send a fix? 
Otherwise, I can fix it. Thanks for your report and analysis.

Re: [PATCH v2 0/3] mm: khugepaged: fix tracepoint UAF

[Lance Yang]

On 2026/8/17 10:25, Baolin Wang wrote:
> Hi Lance,
> 
> On 8/16/26 2:16 AM, Lance Yang wrote:
>> +Cc Baolin
>>
>> On Sun, Aug 16, 2026 at 01:44:44AM +0800, Lance Yang wrote:
>>>
>>> On Sat, Aug 15, 2026 at 01:19:21PM +0800, Vernon Yang wrote:
[...]
> Otherwise, I can fix it. Thanks for your report and analysis.

Yeah, please go ahead :) Thanks, Baolin!

Re: [PATCH v2 0/3] mm: khugepaged: fix tracepoint UAF

[Baolin Wang]

On 8/17/26 10:53 AM, Lance Yang wrote:
> 
> 
> On 2026/8/17 10:25, Baolin Wang wrote:
>> Hi Lance,
>>
>> On 8/16/26 2:16 AM, Lance Yang wrote:
>>> +Cc Baolin
>>>
>>> On Sun, Aug 16, 2026 at 01:44:44AM +0800, Lance Yang wrote:
>>>>
>>>> On Sat, Aug 15, 2026 at 01:19:21PM +0800, Vernon Yang wrote:
> [...]
>> Otherwise, I can fix it. Thanks for your report and analysis.
> 
> Yeah, please go ahead :) Thanks, Baolin!

FYI: I've posted the fix and verified it resolves the issue from my testing.

https://lore.kernel.org/all/ed34ca03ae7d65e89467fb87bc961f5497049c00.1786948410.git.baolin.wang@linux.alibaba.com/

Re: [PATCH v2 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[Lorenzo Stoakes (ARM)]

On Sat, Aug 15, 2026 at 01:19:22PM +0800, Vernon Yang wrote:
> From: Vernon Yang <yanglincheng@kylinos.cn>
>
> When the swap entries found exceed max_ptes_swap, the loop is left via
> break with folio still holding the xarray value that encodes the swap
> entry, not valid folio pointer.
>
> That value is passed to trace_mm_khugepaged_scan_file(), which feeds it
> to folio_pfn(). On FLATMEM and SPARSEMEM_VMEMMAP, the page_to_pfn() is
> plain pointer arithmetic, so the trace event merely prints bogus
> scan_pfn. On classic SPARSEMEM, the page_to_pfn() reads page->flags,
> dereferencing the tiny encoded integer and oopsing khugepaged whenever
> the trace event is enabled.

Hmm I seem to recall that we were no longer spporting !VMEMMAP SPARSEMEM, but
maybe I was imagining that :)

But this needs fixing in general anyway.

>
> So when folio is the swap entry value, simply set pfn to -1, just like
> exhausted scan naturally.
>
> And the folio_put() has maybe dropped the last reference of folio. The
> trace_mm_khugepaged_scan_file() is left with a dangling folio pointer.
> so using the folio_pfn() before dropping the reference, closing
> use-after-free window.
>
> Fixes: d41fd2016ed0 ("mm/khugepaged: add tracepoint to hpage_collapse_scan_file()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> ---
>  include/trace/events/huge_memory.h |  6 +++---
>  mm/khugepaged.c                    | 14 +++++++++-----
>  2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> index 291fae364c62..d3572d4ef453 100644
> --- a/include/trace/events/huge_memory.h
> +++ b/include/trace/events/huge_memory.h
> @@ -178,10 +178,10 @@ TRACE_EVENT(mm_collapse_huge_page_swapin,
>
>  TRACE_EVENT(mm_khugepaged_scan_file,
>
> -	TP_PROTO(struct mm_struct *mm, struct folio *folio, struct file *file,
> +	TP_PROTO(struct mm_struct *mm, unsigned long pfn, struct file *file,
>  		 int present, int swap, int result),
>
> -	TP_ARGS(mm, folio, file, present, swap, result),
> +	TP_ARGS(mm, pfn, file, present, swap, result),
>
>  	TP_STRUCT__entry(
>  		__field(struct mm_struct *, mm)
> @@ -194,7 +194,7 @@ TRACE_EVENT(mm_khugepaged_scan_file,
>
>  	TP_fast_assign(
>  		__entry->mm = mm;
> -		__entry->pfn = folio ? folio_pfn(folio) : -1;
> +		__entry->pfn = pfn;

I wonder how easy it is for people to interpret that this is a PFN of something
only in the case of an early exit.

It's a bit of a mess that we're exposing internal implementation details like
this, quite honestly.

I hope there is no expectation of this being there indefinitely.

>  		__assign_str(filename);
>  		__entry->present = present;
>  		__entry->swap = swap;
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 617bca76db49..e7830761d3a2 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2683,6 +2683,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
>  	int present, swap;
>  	int node = NUMA_NO_NODE;
>  	enum scan_result result = SCAN_SUCCEED;
> +	unsigned long pfn;

See below I think we should drop it.

>
>  	present = 0;
>  	swap = 0;
> @@ -2720,27 +2721,23 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
>  			 * PMD-sized THP implies that we can only try
>  			 * retracting the PTE table.
>  			 */
> -			folio_put(folio);
>  			break;
>  		}
>
>  		node = folio_nid(folio);
>  		if (collapse_scan_abort(node, cc)) {
>  			result = SCAN_SCAN_ABORT;
> -			folio_put(folio);
>  			break;
>  		}
>  		cc->node_load[node]++;
>
>  		if (!folio_test_lru(folio)) {
>  			result = SCAN_PAGE_LRU;
> -			folio_put(folio);
>  			break;
>  		}
>
>  		if (folio_expected_ref_count(folio) + 1 != folio_ref_count(folio)) {
>  			result = SCAN_PAGE_COUNT;
> -			folio_put(folio);
>  			break;
>  		}
>
> @@ -2759,7 +2756,14 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
>  			cond_resched_rcu();
>  		}
>  	}
> +	if (!folio || xa_is_value(folio)) {
> +		pfn = -1;
> +	} else {
> +		pfn = folio_pfn(folio);
> +		folio_put(folio);
> +	}

Hmm kinda hate this you're now mixing clean up logic with some trace logic.

You have to go check out xas_for_each() and see that xas_next_entry() sets folio
to NULL at the end for the normal case to know this is OK and then figure out
that this is just for the break cases.

That's not intuitive in already not intuitive code.

And this is a patch you're wanting to backport to fix a bug too... :)

I think you should keep the folio_put()'s where they are for now and limit this
fix to the trace code please.

I think you should drop the pfn var altogether and do this far more simply, see
below.

>  	rcu_read_unlock();
> +

Stray newline added, let's not do that for a backported fix please :)

>  	if (result == SCAN_PTE_MAPPED_HUGEPAGE)
>  		cc->progress++;
>  	else
> @@ -2774,7 +2778,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
>  		}
>  	}
>
> -	trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result);
> +	trace_mm_khugepaged_scan_file(mm, pfn, file, present, swap, result);

I mean maybe better to just reduce this to something like:

	/* folio is NULL unless exited early. */
	trace_mm_khugepaged_scan_file(mm,
			(!folio || xa_is_value(folio)) ? -1 : folio_pfn(folio),
			file, present, swap, result);

As the least invasive way of fixing the trace + limiting the contageon?

>  	return result;
>  }
>
> --
> 2.53.0
>

--
Cheers, Lorenzo

Re: [PATCH v2 2/3] mm: khugepaged: fix folio is used after pte_unmap_unlock()

[Lorenzo Stoakes (ARM)]

On Sat, Aug 15, 2026 at 01:19:23PM +0800, Vernon Yang wrote:
> From: Vernon Yang <yanglincheng@kylinos.cn>
>
> After the page table lock has dropped, the folio can be freed
> concurrently. The trace_mm_khugepaged_scan_pmd() is left with
> a dangling folio pointer.
>
> So using the folio_pfn() before dropping the page table lock,
> closing use-after-free window.
>
> Fixes: 7d2eba0557c1 ("mm: add tracepoint for scanning pages")
> Cc: stable@vger.kernel.org
> Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> ---
>  include/trace/events/huge_memory.h | 6 +++---
>  mm/khugepaged.c                    | 4 +++-
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> index d3572d4ef453..5dc71d292f47 100644
> --- a/include/trace/events/huge_memory.h
> +++ b/include/trace/events/huge_memory.h
> @@ -55,10 +55,10 @@ SCAN_STATUS
>
>  TRACE_EVENT(mm_khugepaged_scan_pmd,
>
> -	TP_PROTO(struct mm_struct *mm, struct folio *folio,
> +	TP_PROTO(struct mm_struct *mm, unsigned long pfn,
>  		 int referenced, int none_or_zero, int status, int unmapped),
>
> -	TP_ARGS(mm, folio, referenced, none_or_zero, status, unmapped),
> +	TP_ARGS(mm, pfn, referenced, none_or_zero, status, unmapped),
>
>  	TP_STRUCT__entry(
>  		__field(struct mm_struct *, mm)
> @@ -71,7 +71,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
>
>  	TP_fast_assign(
>  		__entry->mm = mm;
> -		__entry->pfn = folio ? folio_pfn(folio) : -1;
> +		__entry->pfn = pfn;
>  		__entry->referenced = referenced;
>  		__entry->none_or_zero = none_or_zero;
>  		__entry->status = status;
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index e7830761d3a2..7c8c48577408 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -1603,6 +1603,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
>  	enum scan_result result = SCAN_FAIL;
>  	struct page *page = NULL;
>  	struct folio *folio = NULL;
> +	unsigned long pfn = -1;
>  	unsigned long addr;
>  	unsigned long enabled_orders;
>  	spinlock_t *ptl;
> @@ -1778,6 +1779,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
>  		result = SCAN_SUCCEED;
>  	}
>  out_unmap:
> +	pfn = folio ? folio_pfn(folio) : -1;
>  	pte_unmap_unlock(pte, ptl);
>  	if (result == SCAN_SUCCEED) {
>  		/* collapse_huge_page expects the lock to be dropped before calling */
> @@ -1788,7 +1790,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
>  		*lock_dropped = true;
>  	}
>  out:
> -	trace_mm_khugepaged_scan_pmd(mm, folio, referenced,
> +	trace_mm_khugepaged_scan_pmd(mm, pfn, referenced,

Same comment as 1/3 I don't see why we should be storing a pfn value used
nowhere else just for tracing.

>  				     none_or_zero, result, unmapped);
>  	return result;
>  }
> --
> 2.53.0
>

--
Cheers, Lorenzo

Re: [PATCH v2 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[David Hildenbrand (Arm)]

> I mean maybe better to just reduce this to something like:
> 
> 	/* folio is NULL unless exited early. */
> 	trace_mm_khugepaged_scan_file(mm,
> 			(!folio || xa_is_value(folio)) ? -1 : folio_pfn(folio),
> 			file, present, swap, result);
> 
> As the least invasive way of fixing the trace + limiting the contageon?

I raised that using a folio after dropping relevant refs+locks is in general an
anti-pattern (IOW wrong if folios can just get split afterwards and are suddenly
no longer folios).

I'd like us to avoid that where possible (and suspect Willy will thank us later ;) )

Isn't there some minimal way to just calculate the pfn once we obtain a folio? I
was assuming that there should be an easier way to do that.

-- 
Cheers,

David

Re: [PATCH v2 2/3] mm: khugepaged: fix folio is used after pte_unmap_unlock()

[David Hildenbrand (Arm)]

On 8/17/26 18:11, Lorenzo Stoakes (ARM) wrote:
> On Sat, Aug 15, 2026 at 01:19:23PM +0800, Vernon Yang wrote:
>> From: Vernon Yang <yanglincheng@kylinos.cn>
>>
>> After the page table lock has dropped, the folio can be freed
>> concurrently. The trace_mm_khugepaged_scan_pmd() is left with
>> a dangling folio pointer.
>>
>> So using the folio_pfn() before dropping the page table lock,
>> closing use-after-free window.
>>
>> Fixes: 7d2eba0557c1 ("mm: add tracepoint for scanning pages")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
>> ---
>>  include/trace/events/huge_memory.h | 6 +++---
>>  mm/khugepaged.c                    | 4 +++-
>>  2 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
>> index d3572d4ef453..5dc71d292f47 100644
>> --- a/include/trace/events/huge_memory.h
>> +++ b/include/trace/events/huge_memory.h
>> @@ -55,10 +55,10 @@ SCAN_STATUS
>>
>>  TRACE_EVENT(mm_khugepaged_scan_pmd,
>>
>> -	TP_PROTO(struct mm_struct *mm, struct folio *folio,
>> +	TP_PROTO(struct mm_struct *mm, unsigned long pfn,
>>  		 int referenced, int none_or_zero, int status, int unmapped),
>>
>> -	TP_ARGS(mm, folio, referenced, none_or_zero, status, unmapped),
>> +	TP_ARGS(mm, pfn, referenced, none_or_zero, status, unmapped),
>>
>>  	TP_STRUCT__entry(
>>  		__field(struct mm_struct *, mm)
>> @@ -71,7 +71,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
>>
>>  	TP_fast_assign(
>>  		__entry->mm = mm;
>> -		__entry->pfn = folio ? folio_pfn(folio) : -1;
>> +		__entry->pfn = pfn;
>>  		__entry->referenced = referenced;
>>  		__entry->none_or_zero = none_or_zero;
>>  		__entry->status = status;
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index e7830761d3a2..7c8c48577408 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -1603,6 +1603,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
>>  	enum scan_result result = SCAN_FAIL;
>>  	struct page *page = NULL;
>>  	struct folio *folio = NULL;
>> +	unsigned long pfn = -1;
>>  	unsigned long addr;
>>  	unsigned long enabled_orders;
>>  	spinlock_t *ptl;
>> @@ -1778,6 +1779,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
>>  		result = SCAN_SUCCEED;
>>  	}
>>  out_unmap:
>> +	pfn = folio ? folio_pfn(folio) : -1;
>>  	pte_unmap_unlock(pte, ptl);
>>  	if (result == SCAN_SUCCEED) {
>>  		/* collapse_huge_page expects the lock to be dropped before calling */
>> @@ -1788,7 +1790,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
>>  		*lock_dropped = true;
>>  	}
>>  out:
>> -	trace_mm_khugepaged_scan_pmd(mm, folio, referenced,
>> +	trace_mm_khugepaged_scan_pmd(mm, pfn, referenced,
> 
> Same comment as 1/3 I don't see why we should be storing a pfn value used
> nowhere else just for tracing.

I prefer it that way. Even if just for tracing. :)

-- 
Cheers,

David

Re: [PATCH v2 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[Lorenzo Stoakes (ARM)]

On Mon, Aug 17, 2026 at 05:10:39PM +0100, Lorenzo Stoakes (ARM) wrote:
> On Sat, Aug 15, 2026 at 01:19:22PM +0800, Vernon Yang wrote:
> > From: Vernon Yang <yanglincheng@kylinos.cn>
> >
> > When the swap entries found exceed max_ptes_swap, the loop is left via
> > break with folio still holding the xarray value that encodes the swap
> > entry, not valid folio pointer.
> >
> > That value is passed to trace_mm_khugepaged_scan_file(), which feeds it
> > to folio_pfn(). On FLATMEM and SPARSEMEM_VMEMMAP, the page_to_pfn() is
> > plain pointer arithmetic, so the trace event merely prints bogus
> > scan_pfn. On classic SPARSEMEM, the page_to_pfn() reads page->flags,
> > dereferencing the tiny encoded integer and oopsing khugepaged whenever
> > the trace event is enabled.
>
> Hmm I seem to recall that we were no longer spporting !VMEMMAP SPARSEMEM, but
> maybe I was imagining that :)

OK seems some museum piece architectures are causing us issues again, fun times
(parisc and some mips).

>
> But this needs fixing in general anyway.
>
> >
> > So when folio is the swap entry value, simply set pfn to -1, just like
> > exhausted scan naturally.
> >
> > And the folio_put() has maybe dropped the last reference of folio. The
> > trace_mm_khugepaged_scan_file() is left with a dangling folio pointer.
> > so using the folio_pfn() before dropping the reference, closing
> > use-after-free window.
> >
> > Fixes: d41fd2016ed0 ("mm/khugepaged: add tracepoint to hpage_collapse_scan_file()")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> > ---
> >  include/trace/events/huge_memory.h |  6 +++---
> >  mm/khugepaged.c                    | 14 +++++++++-----
> >  2 files changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> > index 291fae364c62..d3572d4ef453 100644
> > --- a/include/trace/events/huge_memory.h
> > +++ b/include/trace/events/huge_memory.h
> > @@ -178,10 +178,10 @@ TRACE_EVENT(mm_collapse_huge_page_swapin,
> >
> >  TRACE_EVENT(mm_khugepaged_scan_file,
> >
> > -	TP_PROTO(struct mm_struct *mm, struct folio *folio, struct file *file,
> > +	TP_PROTO(struct mm_struct *mm, unsigned long pfn, struct file *file,
> >  		 int present, int swap, int result),
> >
> > -	TP_ARGS(mm, folio, file, present, swap, result),
> > +	TP_ARGS(mm, pfn, file, present, swap, result),
> >
> >  	TP_STRUCT__entry(
> >  		__field(struct mm_struct *, mm)
> > @@ -194,7 +194,7 @@ TRACE_EVENT(mm_khugepaged_scan_file,
> >
> >  	TP_fast_assign(
> >  		__entry->mm = mm;
> > -		__entry->pfn = folio ? folio_pfn(folio) : -1;
> > +		__entry->pfn = pfn;
>
> I wonder how easy it is for people to interpret that this is a PFN of something
> only in the case of an early exit.
>
> It's a bit of a mess that we're exposing internal implementation details like
> this, quite honestly.
>
> I hope there is no expectation of this being there indefinitely.
>
> >  		__assign_str(filename);
> >  		__entry->present = present;
> >  		__entry->swap = swap;
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 617bca76db49..e7830761d3a2 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -2683,6 +2683,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
> >  	int present, swap;
> >  	int node = NUMA_NO_NODE;
> >  	enum scan_result result = SCAN_SUCCEED;
> > +	unsigned long pfn;
>
> See below I think we should drop it.

OK we can't because of museum piece architectures that will actually deref the
folio to get the PFN...

I'd startr by initialising this to -1 then since I guess in theory it's possible
xas_for_each() could just not do anything is it? (folio is being set to NULL so
that suggests so).

>
> >
> >  	present = 0;
> >  	swap = 0;
> > @@ -2720,27 +2721,23 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
> >  			 * PMD-sized THP implies that we can only try
> >  			 * retracting the PTE table.
> >  			 */
> > -			folio_put(folio);

I still hate the idea of moving these outside of the loop, it's just asking for trouble.

So I guess instead:

	xas_for_each(&xas, folio, start + HPAGE_PMD_NR - 1) {
		pfn = -1;
		...
		if (xa_is_value(folio)) {
			...
		}
		pfn = folio_pfn(folio);
		...
	}
	...
	trace_mm_khugepaged_scan_file(mm, pfn, file, present, swap, result);

That way you avoid the horror or moving the folio put somewhere deeply
unintuitive and inconsistent, and maintain pfn lifetime for some arches nobody
uses.


> >  			break;
> >  		}
> >
> >  		node = folio_nid(folio);
> >  		if (collapse_scan_abort(node, cc)) {
> >  			result = SCAN_SCAN_ABORT;
> > -			folio_put(folio);
> >  			break;
> >  		}
> >  		cc->node_load[node]++;
> >
> >  		if (!folio_test_lru(folio)) {
> >  			result = SCAN_PAGE_LRU;
> > -			folio_put(folio);
> >  			break;
> >  		}
> >
> >  		if (folio_expected_ref_count(folio) + 1 != folio_ref_count(folio)) {
> >  			result = SCAN_PAGE_COUNT;
> > -			folio_put(folio);
> >  			break;
> >  		}
> >
> > @@ -2759,7 +2756,14 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
> >  			cond_resched_rcu();
> >  		}
> >  	}
> > +	if (!folio || xa_is_value(folio)) {
> > +		pfn = -1;
> > +	} else {
> > +		pfn = folio_pfn(folio);
> > +		folio_put(folio);
> > +	}
>
> Hmm kinda hate this you're now mixing clean up logic with some trace logic.
>
> You have to go check out xas_for_each() and see that xas_next_entry() sets folio
> to NULL at the end for the normal case to know this is OK and then figure out
> that this is just for the break cases.
>
> That's not intuitive in already not intuitive code.
>
> And this is a patch you're wanting to backport to fix a bug too... :)
>
> I think you should keep the folio_put()'s where they are for now and limit this
> fix to the trace code please.
>
> I think you should drop the pfn var altogether and do this far more simply, see
> below.
>
> >  	rcu_read_unlock();
> > +
>
> Stray newline added, let's not do that for a backported fix please :)
>
> >  	if (result == SCAN_PTE_MAPPED_HUGEPAGE)
> >  		cc->progress++;
> >  	else
> > @@ -2774,7 +2778,7 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm,
> >  		}
> >  	}
> >
> > -	trace_mm_khugepaged_scan_file(mm, folio, file, present, swap, result);
> > +	trace_mm_khugepaged_scan_file(mm, pfn, file, present, swap, result);
>
> I mean maybe better to just reduce this to something like:
>
> 	/* folio is NULL unless exited early. */
> 	trace_mm_khugepaged_scan_file(mm,
> 			(!folio || xa_is_value(folio)) ? -1 : folio_pfn(folio),
> 			file, present, swap, result);
>
> As the least invasive way of fixing the trace + limiting the contageon?

Yeah as above this isn't right.

>
> >  	return result;
> >  }
> >
> > --
> > 2.53.0
> >
>
> --
> Cheers, Lorenzo

--
Cheers, Lorenzo

Re: [PATCH v2 1/3] mm: khugepaged: fix swap entry value to folio_pfn()

[Lorenzo Stoakes (ARM)]

On Mon, Aug 17, 2026 at 06:19:16PM +0200, David Hildenbrand (Arm) wrote:
>
> > I mean maybe better to just reduce this to something like:
> >
> > 	/* folio is NULL unless exited early. */
> > 	trace_mm_khugepaged_scan_file(mm,
> > 			(!folio || xa_is_value(folio)) ? -1 : folio_pfn(folio),
> > 			file, present, swap, result);
> >
> > As the least invasive way of fixing the trace + limiting the contageon?
>
> I raised that using a folio after dropping relevant refs+locks is in general an
> anti-pattern (IOW wrong if folios can just get split afterwards and are suddenly
> no longer folios).
>
> I'd like us to avoid that where possible (and suspect Willy will thank us later ;) )
>
> Isn't there some minimal way to just calculate the pfn once we obtain a folio? I
> was assuming that there should be an easier way to do that.

See my reply-to-self. My initial reply was wrong because parisc + mips do
actually do classic sparsemem (sigh).

I suggest what you're saying here basically.

(Moving folio_put()'s for the break cases out of the loop is nasty)

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo

Re: [PATCH v2 2/3] mm: khugepaged: fix folio is used after pte_unmap_unlock()

[Lorenzo Stoakes (ARM)]

On Mon, Aug 17, 2026 at 06:20:02PM +0200, David Hildenbrand (Arm) wrote:
> On 8/17/26 18:11, Lorenzo Stoakes (ARM) wrote:
> > Same comment as 1/3 I don't see why we should be storing a pfn value used
> > nowhere else just for tracing.
>
> I prefer it that way. Even if just for tracing. :)

This is moot because you have to store the PFN for lifetime for museum-piece
architectures.

But anyway FWIW:

Yes I agree accessing a now-invalid folio isn't great, but the reason I dislike
this pattern is that you're tracking this state in a different way that now has
maintenance overhead (if anybody reassigns folio or does weird control flow
etc. they must now update 2 things and etc.)

For the sake of CONFIG_SPARSEMEM and tracing for a value that is not at all
intuitive as to what it means, that's not great.

Let me go back and review this again accounting for the lifetime issue that I
missed first time.

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo

Re: [PATCH v2 2/3] mm: khugepaged: fix folio is used after pte_unmap_unlock()

[Lorenzo Stoakes]

On Mon, Aug 17, 2026 at 05:12:01PM +0100, Lorenzo Stoakes (ARM) wrote:
> On Sat, Aug 15, 2026 at 01:19:23PM +0800, Vernon Yang wrote:
> > From: Vernon Yang <yanglincheng@kylinos.cn>
> >
> > After the page table lock has dropped, the folio can be freed
> > concurrently. The trace_mm_khugepaged_scan_pmd() is left with
> > a dangling folio pointer.
> >
> > So using the folio_pfn() before dropping the page table lock,
> > closing use-after-free window.
> >
> > Fixes: 7d2eba0557c1 ("mm: add tracepoint for scanning pages")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Vernon Yang <yanglincheng@kylinos.cn>
> > ---
> >  include/trace/events/huge_memory.h | 6 +++---
> >  mm/khugepaged.c                    | 4 +++-
> >  2 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> > index d3572d4ef453..5dc71d292f47 100644
> > --- a/include/trace/events/huge_memory.h
> > +++ b/include/trace/events/huge_memory.h
> > @@ -55,10 +55,10 @@ SCAN_STATUS
> >
> >  TRACE_EVENT(mm_khugepaged_scan_pmd,
> >
> > -	TP_PROTO(struct mm_struct *mm, struct folio *folio,
> > +	TP_PROTO(struct mm_struct *mm, unsigned long pfn,
> >  		 int referenced, int none_or_zero, int status, int unmapped),
> >
> > -	TP_ARGS(mm, folio, referenced, none_or_zero, status, unmapped),
> > +	TP_ARGS(mm, pfn, referenced, none_or_zero, status, unmapped),
> >
> >  	TP_STRUCT__entry(
> >  		__field(struct mm_struct *, mm)
> > @@ -71,7 +71,7 @@ TRACE_EVENT(mm_khugepaged_scan_pmd,
> >
> >  	TP_fast_assign(
> >  		__entry->mm = mm;
> > -		__entry->pfn = folio ? folio_pfn(folio) : -1;
> > +		__entry->pfn = pfn;
> >  		__entry->referenced = referenced;
> >  		__entry->none_or_zero = none_or_zero;
> >  		__entry->status = status;
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index e7830761d3a2..7c8c48577408 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -1603,6 +1603,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> >  	enum scan_result result = SCAN_FAIL;
> >  	struct page *page = NULL;
> >  	struct folio *folio = NULL;
> > +	unsigned long pfn = -1;
> >  	unsigned long addr;
> >  	unsigned long enabled_orders;
> >  	spinlock_t *ptl;
> > @@ -1778,6 +1779,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> >  		result = SCAN_SUCCEED;
> >  	}
> >  out_unmap:
> > +	pfn = folio ? folio_pfn(folio) : -1;

You already defaulted the value to -1, better as:

	if (folio)
		pfn = folio_pfn(folio);

> >  	pte_unmap_unlock(pte, ptl);
> >  	if (result == SCAN_SUCCEED) {
> >  		/* collapse_huge_page expects the lock to be dropped before calling */
> > @@ -1788,7 +1790,7 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> >  		*lock_dropped = true;
> >  	}
> >  out:
> > -	trace_mm_khugepaged_scan_pmd(mm, folio, referenced,
> > +	trace_mm_khugepaged_scan_pmd(mm, pfn, referenced,
>
> Same comment as 1/3 I don't see why we should be storing a pfn value used
> nowhere else just for tracing.

Disregard, I didn't account for lifetime.

>
> >  				     none_or_zero, result, unmapped);
> >  	return result;
> >  }
> > --
> > 2.53.0
> >
>
> --
> Cheers, Lorenzo

--
Cheers, Lorenzo