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

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

On Thu, 15 Oct 2026 14:05:27 +0800 Jiayuan Chen <jiayuan.chen@linux.dev> wrote:

> From: Jiayuan Chen <jiayuan.chen@shopee.com>
> 
> 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.
> 
> Recheck the slot after folio_try_get() and retry if it no longer holds
> this folio.

LLM says to me:

: The original folio could conceivably be freed, its physical page
: reused, and the same PFN inserted into the same swap-table slot before
: the second lookup.  That would pass the check.  However, the returned
: reference would then belong to the newly inserted folio, and the later
: swap-entry validation would still be required.  I don't see that as
: invalidating this fix.

> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -92,17 +92,27 @@ void show_swap_cache_info(void)
>   */
>  struct folio *swap_cache_get_folio(swp_entry_t entry)
>  {
> +	struct swap_cluster_info *ci = __swap_entry_to_cluster(entry);
> +	unsigned int ci_off = swp_cluster_offset(entry);
>  	unsigned long swp_tb;
>  	struct folio *folio;
>  
>  	for (;;) {
> -		swp_tb = swap_table_get(__swap_entry_to_cluster(entry),
> -					swp_cluster_offset(entry));
> +		swp_tb = swap_table_get(ci, ci_off);
>  		if (!swp_tb_is_folio(swp_tb))
>  			return NULL;
>  		folio = swp_tb_to_folio(swp_tb);
> -		if (likely(folio_try_get(folio)))
> +		if (!folio_try_get(folio))
> +			continue;
> +		/*
> +		 * The folio may have been freed and its memory reused before
> +		 * we got the reference. Make sure it is still in the slot.
> +		 */
> +		swp_tb = swap_table_get(ci, ci_off);
> +		if (likely(swp_tb_is_folio(swp_tb) &&
> +			   swp_tb_to_folio(swp_tb) == folio))
>  			return folio;
> +		folio_put(folio);
>  	}
>  
>  	return NULL;

This return is now unreachable...