Re: [PATCH] mm/huge_memory: fix NULL pointer deference when splitting shmem folio in swap cache

[Wei Yang <richard.weiyang@gmail.com>]

On Wed, Nov 19, 2025 at 09:57:58AM +0100, David Hildenbrand (Red Hat) wrote:
>On 19.11.25 02:26, Wei Yang wrote:
>> Commit c010d47f107f ("mm: thp: split huge page to any lower order
>> pages") introduced an early check on the folio's order via
>> mapping->flags before proceeding with the split work.
>> 
>> This check introduced a bug: for shmem folios in the swap cache, the
>> mapping pointer can be NULL. Accessing mapping->flags in this state
>> leads directly to a NULL pointer dereference.
>
>Under which circumstances would that be the case? Only for large shmem folios
>in the swapcache or also for truncated folios? So I'd assume this
>would also affect truncated folios and we should spell that out here?
>

Sorry I don't mention it for my uncertainty.

Will add it.

>> 
>> This commit fixes the issue by moving the check for mapping != NULL
>> before any attempt to access mapping->flags.
>> 
>> This fix necessarily changes the return value from -EBUSY to -EINVAL
>> when mapping is NULL. After reviewing current callers, they do not
>> differentiate between these two error codes, making this change safe.
>
>The doc of __split_huge_page_to_list_to_order() would now be outdated and has
>to be updated.
>
>Also, take a look at s390_wiggle_split_folio(): returning -EINVAL instead of
>-EBUSY will make a difference on concurrent truncation. -EINVAL will be
>propagated and make the operation fail, while -EBUSY will be translated to
>-EAGAIN and the caller will simply lookup the folio again and retry.
>

Oops, I missed this case.

I will rescan users.

>So I think we should try to keep truncation return -EBUSY. For the shmem
>case, I think it's ok to return -EINVAL. I guess we can identify such folios
>by checking for folio_test_swapcache().
>

Hmm... Don't get how to do this nicely.

Looks we can't do it in folio_split_supported().

Or change folio_split_supported() return error code directly?

>
>Probably worth mentioning that this was identified by code inspection?
>

Agree.

>> 
>> Fixes: c010d47f107f ("mm: thp: split huge page to any lower order pages")
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> Cc: Zi Yan <ziy@nvidia.com>
>> Cc: <stable@vger.kernel.org>
>
>Hmm, what would this patch look like when based on current upstream? We'd
>likely want to get that upstream asap.
>

This depends whether we want it on top of [1].

Current upstream doesn't have it [1] and need to fix it in two places.

Andrew mention prefer a fixup version in [2]. 

[1]: lkml.kernel.org/r/20251106034155.21398-1-richard.weiyang@gmail.com
[2]: lkml.kernel.org/r/20251118140658.9078de6aab719b2308996387@linux-foundation.org

>> 
>> ---
>> 
>> This patch is based on current mm-new, latest commit:
>> 
>>      056b93566a35 mm/vmalloc: warn only once when vmalloc detect invalid gfp flags
>> 
>> Backport note:
>> 
>> Current code evolved from original commit with following four changes.
>> We should do proper adjustment respectively on backporting.
>> 
>> commit c010d47f107f609b9f4d6a103b6dfc53889049e9
>> Author: Zi Yan <ziy@nvidia.com>
>> Date:   Mon Feb 26 15:55:33 2024 -0500
>> 
>>      mm: thp: split huge page to any lower order pages
>> 
>> commit 6a50c9b512f7734bc356f4bd47885a6f7c98491a
>> Author: Ran Xiaokai <ran.xiaokai@zte.com.cn>
>> Date:   Fri Jun 7 17:40:48 2024 +0800
>> 
>>      mm: huge_memory: fix misused mapping_large_folio_support() for anon folios
>> 
>> commit 9b2f764933eb5e3ac9ebba26e3341529219c4401
>> Author: Zi Yan <ziy@nvidia.com>
>> Date:   Wed Jan 22 11:19:27 2025 -0500
>> 
>>      mm/huge_memory: allow split shmem large folio to any lower order
>> 
>> commit 58729c04cf1092b87aeef0bf0998c9e2e4771133
>> Author: Zi Yan <ziy@nvidia.com>
>> Date:   Fri Mar 7 12:39:57 2025 -0500
>> 
>>      mm/huge_memory: add buddy allocator like (non-uniform) folio_split()
>> ---
>>   mm/huge_memory.c | 68 +++++++++++++++++++++++++-----------------------
>>   1 file changed, 35 insertions(+), 33 deletions(-)
>> 
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 7c69572b6c3f..8701c3eef05f 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -3696,29 +3696,42 @@ bool folio_split_supported(struct folio *folio, unsigned int new_order,
>>   				"Cannot split to order-1 folio");
>>   		if (new_order == 1)
>>   			return false;
>> -	} else if (split_type == SPLIT_TYPE_NON_UNIFORM || new_order) {
>> -		if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
>> -		    !mapping_large_folio_support(folio->mapping)) {
>> -			/*
>> -			 * We can always split a folio down to a single page
>> -			 * (new_order == 0) uniformly.
>> -			 *
>> -			 * For any other scenario
>> -			 *   a) uniform split targeting a large folio
>> -			 *      (new_order > 0)
>> -			 *   b) any non-uniform split
>> -			 * we must confirm that the file system supports large
>> -			 * folios.
>> -			 *
>> -			 * Note that we might still have THPs in such
>> -			 * mappings, which is created from khugepaged when
>> -			 * CONFIG_READ_ONLY_THP_FOR_FS is enabled. But in that
>> -			 * case, the mapping does not actually support large
>> -			 * folios properly.
>> -			 */
>> -			VM_WARN_ONCE(warns,
>> -				"Cannot split file folio to non-0 order");
>> +	} else {
>
>
>Why not simply
>
>} else if (!folio->mapping) {
>	/*
>	 * Truncated?
>	...
>	return false;
>} else if (split_type == SPLIT_TYPE_NON_UNIFORM || new_order) {
>...
>

Ah, just not spot this :-(

>> +		const struct address_space *mapping = folio->mapping;
>> +
>> +		/* Truncated ? */
>> +		/*
>> +		 * TODO: add support for large shmem folio in swap cache.
>> +		 * When shmem is in swap cache, mapping is NULL and
>> +		 * folio_test_swapcache() is true.
>> +		 */
>
>While at it, can we merge the two comments into something, like:
>
>/*
> * If there is no mapping that the folio was truncated and we cannot
> * split.
> *
> * TODO: large shmem folio in the swap cache also don't currently have a
> * mapping but folio_test_swapcache() is true for them.
> */
>

Sure.

>-- 
>Cheers
>
>David

-- 
Wei Yang
Help you, Help me