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

[David Hildenbrand (Arm)]

On 8/26/26 10:09, Lorenzo Stoakes (ARM) wrote:
> On Mon, Aug 24, 2026 at 05:29:35PM +0800, Vernon Yang wrote:
>> From: Vernon Yang <yanglincheng@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxxxx
>> Signed-off-by: Vernon Yang <yanglincheng@xxxxxxxxxx>
>> ---
>>  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 fa828967e1fb..5fb4d92cfd84 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 4e0fca5942dd..24347f1a94ae 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -2254,6 +2254,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;
> 
> Nitty but:
> 
> Could we use pfn_t? I don't love that we are inconsistent with that.

Note that the monstrosity we called pfn_t no longer exits. :)

-- 
Cheers,

David