Re: [PATCH] mm/swap: fix swap_cache_get_folio() returning a folio that left the swap cache

[Andrew Morton <akpm@linux-foundation.org>]

On Fri, 16 Oct 2026 10:22:20 +0800 Kairui Song <ryncsn@gmail.com> wrote:

> On Fri, Oct 16, 2026 at 10:16 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
> >
> > Hi Andrew,
> >
> > On 10/16/26 9:40 AM, Andrew Morton wrote:
> > > On Thu, 15 Oct 2026 14:05:27 +0800 Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
> > >
> > [...]
> 
> Hi all,
> 
> > >
> > > So CPU 0 writes to a folio it doesn't own: it may clear PG_readahead and
> > > takes the folio lock of someone else's page, before
> > > folio_matches_swap_entry() finds out the folio is not ours.
> > >
> > > Recheck the slot after folio_try_get() and retry if it no longer holds
> > > this folio.
> > > Thanks.
> > >
> > > As always, when fixing a bug please fully describe the worst-case
> > > userspace-visible runtime effects of that bug.
> > >
> > > I asked $LLM for this and was told
> > >
> > > : When the race occurs, a swap fault can acquire a reference to a
> > > : physical page that has been freed and reused for an unrelated purpose,
> > > : potentially by another process.  The fault handler may then clear that
> > > : page's PG_readahead/PG_reclaim flag, wait on or acquire its folio lock,
> > > : and potentially attempt to remove it from an unrelated swap cache.
> > > :
> > > : These operations can interfere with unrelated workloads and may cause
> > > : unexpected stalls, impaired reclaim or writeback, and potentially
> > > : system hangs.  The subsequent swap-entry validation normally prevents
> >
> >
> > I don't see a way to a system hang: the folio lock waiter is woken when
> > the real owner unlocks, so the worst case is a stall. I suggest
> > dropping "and potentially system hangs".

Yes, I think it's assuming the other-owned folio never gets unlocked.

> >
> > Writeback itself is not affected, only the PG_reclaim hint on where to
> > put the folio after writeback. So "impaired reclaim or writeback" ->
> > "impaired reclaim".
> >
> > So I'd suggest:
> >
> >    : When the race occurs, a swap fault can acquire a reference to a
> >    : physical page that has been freed and reused for an unrelated purpose,
> >    : potentially by another process.  The fault handler may then clear that
> >    : page's PG_readahead/PG_reclaim flag, wait on or acquire its folio lock,
> >    : and potentially attempt to remove it from an unrelated swap cache.
> >    :
> >    : These operations can interfere with unrelated workloads and may cause
> >    : unexpected stalls and impaired reclaim.  The subsequent swap-entry
> >    : validation normally prevents the wrong folio from being mapped into
> 
> It's documented that caller always double checks the folio so this
> part is harmless from the aspect of data corruption or crash.

OK, thanks, adjustment made.

> > >> Fixes: 8578e0c00dcf ("mm, swap: use the swap table for the swap cache and switch API")
> > > Given the above, I suggest adding cc:stable.
> > >
> > > Do you and maintainers agree?
> >
> > Agreed for me.
> >
> 
> This is more like a optimization than a bug fix, but it's clean and
> easy to follow, Cc'ing stable is fine to me.

Was that an ack ;)


Here's what I now have:

: swap_cache_get_folio() reads the folio from the swap table and takes a
: reference with folio_try_get(), without any lock.  If the folio is removed
: from the swap cache and freed in between, and its memory gets reused,
: folio_try_get() still succeeds and we return a page that has nothing to do
: with the swap entry:
: 
:   CPU 0                                  CPU 1
:   do_swap_page()
:     swap_cache_get_folio()
:       swp_tb = swap_table_get()
:       folio = swp_tb_to_folio(swp_tb)
:                                          madvise(MADV_DONTNEED)
:                                            zap_nonpresent_ptes()
:                                              swap_put_entries_direct()
:                                                swap_put_entries_cluster()
:                                                  __try_to_reclaim_swap()
:                                                    swap_cache_del_folio()
:                                                    folio_put()   <- freed
:                                          page reused, e.g. as an anon
:                                          folio of another task
:       folio_try_get(folio)               <- succeeds
:       return folio
:     swap_update_readahead()              <- may clear PG_readahead on it
:     folio_lock_or_retry()                <- locks it
:     folio_matches_swap_entry() fails, unlock and put
: 
: filemap_get_entry() does the same lockless lookup for the page cache, and
: after folio_try_get() it rechecks the xarray, because the folio may have
: been freed and reused in between.  swap_cache_get_folio() lost that
: recheck when the swap cache moved to the swap table.
: 
: So CPU 0 writes to a folio it doesn't own: it may clear PG_readahead and
: takes the folio lock of someone else's page, before
: folio_matches_swap_entry() finds out the folio is not ours.
: 
: When the race occurs, a swap fault can acquire a reference to a
: physical page that has been freed and reused for an unrelated purpose,
: potentially by another process.  The fault handler may then clear that
: page's PG_readahead/PG_reclaim flag, wait on or acquire its folio lock,
: and potentially attempt to remove it from an unrelated swap cache.
: 
: These operations can interfere with unrelated workloads and may cause
: unexpected stalls and impaired reclaim.  The subsequent swap-entry
: validation normally prevents the wrong folio from being mapped into the
: faulting process.
: 
: It's documented that caller always double checks the folio so this part
: is harmless from the aspect of data corruption or crash.
: 
: Recheck the slot after folio_try_get() and retry if it no longer holds
: this folio.