linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Wei Yang <richard.weiyang@gmail.com>
To: "David Hildenbrand (Red Hat)" <david@kernel.org>
Cc: Wei Yang <richard.weiyang@gmail.com>,
	akpm@linux-foundation.org, lorenzo.stoakes@oracle.com,
	ziy@nvidia.com, baolin.wang@linux.alibaba.com,
	Liam.Howlett@oracle.com, npache@redhat.com, ryan.roberts@arm.com,
	dev.jain@arm.com, baohua@kernel.org, lance.yang@linux.dev,
	linux-mm@kvack.org, stable@vger.kernel.org
Subject: Re: [PATCH] mm/huge_memory: fix NULL pointer deference when splitting shmem folio in swap cache
Date: Wed, 19 Nov 2025 13:14:31 +0000	[thread overview]
Message-ID: <20251119131431.gzr77o24cnnt3o34@master> (raw)
In-Reply-To: <59b1d49f-42f5-4e7e-ae23-7d96cff5b035@kernel.org>

On Wed, Nov 19, 2025 at 01:54:45PM +0100, 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;
>        } 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.
>+        */
>+       if (!is_anon && !folio->mapping && !folio_test_swapcache(folio))
>+               return -EBUSY;
>+
>        if (new_order >= folio_order(folio))
>                return -EINVAL;
>@@ -3659,17 +3674,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) {
>
>
>So rule out the truncated case earlier, leaving only the swapcache check to be handled
>later.
>
>Thoughts?
>

Cleaner, will test this first.

>> 
>> > 
>> > 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.
>

Yep, I will prepare patch both for current master and current mm-new.

And wait for Andrew's order.

>-- 
>Cheers
>
>David

-- 
Wei Yang
Help you, Help me


  parent reply	other threads:[~2025-11-19 13:14 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-19  1:26 Wei Yang
2025-11-19  2:32 ` Zi Yan
2025-11-19  2:56   ` Wei Yang
2025-11-19  8:57 ` David Hildenbrand (Red Hat)
2025-11-19 12:23   ` Wei Yang
2025-11-19 12:54     ` David Hildenbrand (Red Hat)
2025-11-19 13:08       ` Zi Yan
2025-11-19 13:41         ` Wei Yang
2025-11-19 13:58           ` David Hildenbrand (Red Hat)
2025-11-19 14:09         ` David Hildenbrand (Red Hat)
2025-11-19 14:29           ` Zi Yan
2025-11-19 14:37             ` David Hildenbrand (Red Hat)
2025-11-19 14:46               ` David Hildenbrand (Red Hat)
2025-11-19 14:48                 ` Zi Yan
2025-11-19 14:50                   ` David Hildenbrand (Red Hat)
2025-11-19 23:18                 ` Wei Yang
2025-11-20  0:47                 ` Wei Yang
2025-11-20  3:00                   ` Zi Yan
2025-11-19 14:47               ` Zi Yan
2025-11-19 13:14       ` Wei Yang [this message]
2025-11-19 12:42   ` Wei Yang
2025-11-19 14:13     ` David Hildenbrand (Red Hat)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251119131431.gzr77o24cnnt3o34@master \
    --to=richard.weiyang@gmail.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@kernel.org \
    --cc=dev.jain@arm.com \
    --cc=lance.yang@linux.dev \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=npache@redhat.com \
    --cc=ryan.roberts@arm.com \
    --cc=stable@vger.kernel.org \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox