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

[Zi Yan <ziy@nvidia.com>]

On 19 Nov 2025, at 9:09, David Hildenbrand (Red Hat) wrote:

> On 19.11.25 14:08, Zi Yan wrote:
>> On 19 Nov 2025, at 7:54, David Hildenbrand (Red Hat) wrote:
>>
>>>>
>>>>> 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?
>>>
>>>
>>> On upstream, I would do something like the following (untested):
>>>
>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>> index 2f2a521e5d683..33fc3590867e2 100644
>>> --- a/mm/huge_memory.c
>>> +++ b/mm/huge_memory.c
>>> @@ -3524,6 +3524,9 @@ bool non_uniform_split_supported(struct folio *folio, unsigned int new_order,
>>>                                  "Cannot split to order-1 folio");
>>>                  if (new_order == 1)
>>>                          return false;
>>> +       } else if (folio_test_swapcache(folio)) {
>>> +               /* TODO: support shmem folios that are in the swapcache. */
>>> +               return false;
>>>          } else if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
>>>              !mapping_large_folio_support(folio->mapping)) {
>>>                  /*
>>> @@ -3556,6 +3559,9 @@ bool uniform_split_supported(struct folio *folio, unsigned int new_order,
>>>                                  "Cannot split to order-1 folio");
>>>                  if (new_order == 1)
>>>                          return false;
>>> +       } else if (folio_test_swapcache(folio)) {
>>> +               /* TODO: support shmem folios that are in the swapcache. */
>>> +               return false;
>> You are splitting the truncate case into shmem one and page cache one.
>> This is only for shmem in the swap cache and ...
>>
>>>          } else  if (new_order) {
>>>                  if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
>>>                      !mapping_large_folio_support(folio->mapping)) {
>>> @@ -3619,6 +3625,15 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>>>          if (folio != page_folio(split_at) || folio != page_folio(lock_at))
>>>                  return -EINVAL;
>>>   +       /*
>>> +        * Folios that just got truncated cannot get split. Signal to the
>>> +        * caller that there was a race.
>>> +        *
>>> +        * TODO: support shmem folios that are in the swapcache.
>>
>> this is for page cache one. So this TODO is not needed.
>
> I added the TODO because we'd like to detect truncation there as well I think. Hm.

OK, got it. Here you mean shmem in the swapcache is not checked and
when shmem in the swapcache is supported, folio_test_swapcache() can be removed
here along with the TODO. Now it makes sense.

>>
>>> +        */
>>> +       if (!is_anon && !folio->mapping && !folio_test_swapcache(folio))
>>> +               return -EBUSY;
>>> +
>
> Given folio_test_swapcache() might have false positives,
> I assume we'd need a
>
> 	folio_test_swapbacked() && folio_test_swapcache(folio)
>
> To detect large large shmem folios in the swapcache in all cases here.
>
> Something like the following would hopefully do:
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 2f2a521e5d683..57aab66bedbea 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3515,6 +3515,13 @@ static int __split_unmapped_folio(struct folio *folio, int new_order,
>         return ret;
>  }
>  +static bool folio_test_shmem_swapcache(struct folio *folio)
> +{
> +       VM_WARN_ON_ONCE_FOLIO(folio_test_anon(folio), folio);
> +       /* These folios do not have folio->mapping set. */
> +       return folio_test_swapbacked(folio) && folio_test_swapcache(folio);
> +}
> +
>  bool non_uniform_split_supported(struct folio *folio, unsigned int new_order,
>                 bool warns)
>  {
> @@ -3524,6 +3531,9 @@ bool non_uniform_split_supported(struct folio *folio, unsigned int new_order,
>                                 "Cannot split to order-1 folio");
>                 if (new_order == 1)
>                         return false;
> +       } else if (folio_test_shmem_swapcache(folio)) {
> +               /* TODO: support shmem folios that are in the swapcache. */
> +               return false;

With this, truncated shmem returns -EINVALID instead of -EBUSY now.
Can s390_wiggle_split_folio() such folios?

>         } else if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
>             !mapping_large_folio_support(folio->mapping)) {
>                 /*
> @@ -3556,6 +3566,9 @@ bool uniform_split_supported(struct folio *folio, unsigned int new_order,
>                                 "Cannot split to order-1 folio");
>                 if (new_order == 1)
>                         return false;
> +       } else if (folio_test_shmem_swapcache(folio)) {
> +               /* TODO: support shmem folios that are in the swapcache. */
> +               return false;
>         } else  if (new_order) {
>                 if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
>                     !mapping_large_folio_support(folio->mapping)) {
> @@ -3619,6 +3632,13 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>         if (folio != page_folio(split_at) || folio != page_folio(lock_at))
>                 return -EINVAL;
>  +       /*
> +        * Folios that just got truncated cannot get split. Signal to the
> +        * caller that there was a race.
> +        */
> +       if (!is_anon && !folio->mapping && !folio_test_shmem_swapcache(folio))
> +               return -EBUSY;
> +
>         if (new_order >= folio_order(folio))
>                 return -EINVAL;
>  @@ -3659,17 +3679,7 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>                 gfp_t gfp;
>                  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.
> -                */
> -               if (!mapping) {
> -                       ret = -EBUSY;
> -                       goto out;
> -               }
> +               VM_WARN_ON_ONCE_FOLIO(!mapping, folio);
>                  min_order = mapping_min_folio_order(folio->mapping);
>                 if (new_order < min_order) {
>

I think it works if there is no impact on s390_wiggle_split_folio().
It also clarifies two truncated cases.

For backporting, maybe just move "if (!mapping)" up for simplicity?

>>>>
>>>>>
>>>>> 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
>>>
>>> As we will want to backport this patch, likely we want to have it apply on current master.
>>>
>>> Bur Andrew can comment what he prefers in this case of a stable fix.
>>
>> That could mess up with mm-new tree[1] based on Andrew's recent feedback.
>
> Right, a similar fix could be had against mm-new.
>
> -- 
> Cheers
>
> David


--
Best Regards,
Yan, Zi