Re: [PATCH v2] mm/huge_memory: fix a folio_split() race condition with folio_try_get()

[Zi Yan <ziy@nvidia.com>]

On 3 Mar 2026, at 3:48, David Hildenbrand (Arm) wrote:

> On 3/2/26 21:31, Zi Yan wrote:
>> During a pagecache folio split, the values in the related xarray should not
>> be changed from the original folio at xarray split time until all
>> after-split folios are well formed and stored in the xarray. Current use
>> of xas_try_split() in __split_unmapped_folio() lets some after-split folios
>> show up at wrong indices in the xarray. When these misplaced after-split
>> folios are unfrozen, before correct folios are stored via __xa_store(), and
>> grabbed by folio_try_get(), they are returned to userspace at wrong file
>> indices, causing data corruption. More detailed explanation is at the
>> bottom.
>>
>> The reproducer is at: https://github.com/dfinity/thp-madv-remove-test
>> It
>> 1. creates a memfd,
>> 2. forks,
>> 3. in the child process, maps the file with large folios (via shmem code
>>    path) and reads the mapped file continuously with 16 threads,
>> 4. in the parent process, uses madvise(MADV_REMOVE) to punch poles in the
>>    large folio.
>>
>> Data corruption can be observed without the fix. Basically, data from a
>> wrong page->index is returned.
>>
>> Fix it by using the original folio in xas_try_split() calls, so that
>> folio_try_get() can get the right after-split folios after the original
>> folio is unfrozen.
>>
>> Uniform split, split_huge_page*(), is not affected, since it uses
>> xas_split_alloc() and xas_split() only once and stores the original folio
>> in the xarray. Change xas_split() used in uniform split branch to use
>> the original folio to avoid confusion.
>>
>> Fixes below points to the commit introduces the code, but folio_split() is
>> used in a later commit 7460b470a131f ("mm/truncate: use folio_split() in
>> truncate operation").
>>
>> More details:
>>
>> For example, a folio f is split non-uniformly into f, f2, f3, f4 like
>> below:
>> +----------------+---------+----+----+
>> |       f        |    f2   | f3 | f4 |
>> +----------------+---------+----+----+
>> but the xarray would look like below after __split_unmapped_folio() is
>> done:
>> +----------------+---------+----+----+
>> |       f        |    f2   | f3 | f3 |
>> +----------------+---------+----+----+
>>
>> After __split_unmapped_folio(), the code changes the xarray and unfreezes
>> after-split folios:
>>
>> 1. unfreezes f2, __xa_store(f2)
>> 2. unfreezes f3, __xa_store(f3)
>> 3. unfreezes f4, __xa_store(f4), which overwrites the second f3 to f4.
>> 4. unfreezes f.
>>
>> Meanwhile, a parallel filemap_get_entry() can read the second f3 from the
>> xarray and use folio_try_get() on it at step 2 when f3 is unfrozen. Then,
>> f3 is wrongly returned to user.
>>
>> After the fix, the xarray looks like below after __split_unmapped_folio():
>> +----------------+---------+----+----+
>> |       f        |    f    | f  | f  |
>> +----------------+---------+----+----+
>> so that the race window no longer exists.
>>
>> Fixes: 00527733d0dc8 ("mm/huge_memory: add two new (not yet used) functions for folio_split()")
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> Reported-by: Bas van Dijk <bas@dfinity.org>
>> Closes: https://lore.kernel.org/all/CAKNNEtw5_kZomhkugedKMPOG-sxs5Q5OLumWJdiWXv+C9Yct0w@mail.gmail.com/
>> Tested-by: Lance Yang <lance.yang@linux.dev>
>> Cc: <stable@vger.kernel.org>
>> ---
>>  mm/huge_memory.c | 15 +++++++++++----
>>  1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 56db54fa48181..f0bdac3f270b5 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -3647,6 +3647,7 @@ static int __split_unmapped_folio(struct folio *folio, int new_order,
>>  	const bool is_anon = folio_test_anon(folio);
>>  	int old_order = folio_order(folio);
>>  	int start_order = split_type == SPLIT_TYPE_UNIFORM ? new_order : old_order - 1;
>> +	struct folio *old_folio = folio;
>>  	int split_order;
>>
>>  	/*
>> @@ -3668,11 +3669,17 @@ static int __split_unmapped_folio(struct folio *folio, int new_order,
>>  			 * irq is disabled to allocate enough memory, whereas
>>  			 * non-uniform split can handle ENOMEM.
>>  			 */
>> -			if (split_type == SPLIT_TYPE_UNIFORM)
>> -				xas_split(xas, folio, old_order);
>> -			else {
>
> Just wondering whether we should no move the comment over here now, so
> it just covers both cases.
>
>> +			if (split_type == SPLIT_TYPE_UNIFORM) {
>> +				xas_split(xas, old_folio, old_order);
>> +			} else {
>>  				xas_set_order(xas, folio->index, split_order);
>> -				xas_try_split(xas, folio, old_order);
>> +				/*
>> +				 * use the to-be-split folio, so that a parallel
>> +				 * folio_try_get() waits on it until xarray is
>> +				 * updated with after-split folios and
>> +				 * the original one is unfrozen.
>> +				 */
>> +				xas_try_split(xas, old_folio, old_order);
>>  				if (xas_error(xas))
>>  					return xas_error(xas);
>>  			}
Sure.

Hi Andrew,

Do you mind applying the fixup below? Thanks.

From fe94203b814a7fb11035c5b720a5e798ec2bcbb5 Mon Sep 17 00:00:00 2001
From: Zi Yan <ziy@nvidia.com>
Date: Tue, 3 Mar 2026 11:26:41 -0500
Subject: [PATCH] move the comment.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 mm/huge_memory.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index f0bdac3f270b5..6d3bdde334126 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3668,17 +3668,16 @@ static int __split_unmapped_folio(struct folio *folio, int new_order,
 			 * uniform split has xas_split_alloc() called before
 			 * irq is disabled to allocate enough memory, whereas
 			 * non-uniform split can handle ENOMEM.
+			 *
+			 * use the to-be-split folio in xas_split() or
+			 * xas_try_split(), so that a parallel folio_try_get()
+			 * waits on it until xarray is updated with after-split
+			 * folios and the original one is unfrozen.
 			 */
 			if (split_type == SPLIT_TYPE_UNIFORM) {
 				xas_split(xas, old_folio, old_order);
 			} else {
 				xas_set_order(xas, folio->index, split_order);
-				/*
-				 * use the to-be-split folio, so that a parallel
-				 * folio_try_get() waits on it until xarray is
-				 * updated with after-split folios and
-				 * the original one is unfrozen.
-				 */
 				xas_try_split(xas, old_folio, old_order);
 				if (xas_error(xas))
 					return xas_error(xas);
-- 
2.51.0



--
Best Regards,
Yan, Zi