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?
>
>> 
>> 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.
>
>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().
>

I come up a draft:

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 7c69572b6c3f..3e140fa1ca13 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3696,6 +3696,18 @@ bool folio_split_supported(struct folio *folio, unsigned int new_order,
                                "Cannot split to order-1 folio");
                if (new_order == 1)
                        return -EINVAL;
+       } else if (!folio->mapping) {
+               /*
+                * 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.
+                */
+               if (folio_test_swapcache(folio))
+                       return -EINVAL;
+               return -EBUSY;
        } 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)) {
@@ -3931,8 +3943,9 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
        if (new_order >= old_order)
                return -EINVAL;
 
-       if (!folio_split_supported(folio, new_order, split_type, /* warn = */ true))
-               return -EINVAL;
+       ret = folio_split_supported((folio, new_order, split_type, /* warn = */ true));
+       if (ret)
+               return ret;
 
        is_hzp = is_huge_zero_folio(folio);
        if (is_hzp) {

Not sure I get your point correctly.

-- 
Wei Yang
Help you, Help me