[SRU][F][PATCH 1/1] mm: swap: fix race between free_swap_and_cache() and swapoff()

Koichiro Den koichiro.den at canonical.com
Thu Sep 5 06:36:44 UTC 2024


From: Ryan Roberts <ryan.roberts at arm.com>

There was previously a theoretical window where swapoff() could run and
teardown a swap_info_struct while a call to free_swap_and_cache() was
running in another thread.  This could cause, amongst other bad
possibilities, swap_page_trans_huge_swapped() (called by
free_swap_and_cache()) to access the freed memory for swap_map.

This is a theoretical problem and I haven't been able to provoke it from a
test case.  But there has been agreement based on code review that this is
possible (see link below).

Fix it by using get_swap_device()/put_swap_device(), which will stall
swapoff().  There was an extra check in _swap_info_get() to confirm that
the swap entry was not free.  This isn't present in get_swap_device()
because it doesn't make sense in general due to the race between getting
the reference and swapoff.  So I've added an equivalent check directly in
free_swap_and_cache().

Details of how to provoke one possible issue:

--8<-----

CPU0                               CPU1
----                               ----
shmem_undo_range
  shmem_free_swap
    xa_cmpxchg_irq
    free_swap_and_cache
      __swap_entry_free
      /* swap_count() become 0 */
                                   swapoff
                                     try_to_unuse
                                       shmem_unuse /* cannot find swap entry */
                                       find_next_to_unuse
                                       filemap_get_folio
                                       folio_free_swap
                                       /* remove swap cache */
                                       /* free si->swap_map[] */
      swap_page_trans_huge_swapped <-- access freed si->swap_map !!!

--8<-----

Link: https://lkml.kernel.org/r/20240306140356.3974886-1-ryan.roberts@arm.com
Fixes: 7c00bafee87c ("mm/swap: free swap slots in batch")
Closes: https://lore.kernel.org/linux-mm/65a66eb9-41f8-4790-8db2-0c70ea15979f@redhat.com/
Signed-off-by: Ryan Roberts <ryan.roberts at arm.com>
Signed-off-by: "Huang, Ying" <ying.huang at intel.com>
Cc: David Hildenbrand <david at redhat.com>
Cc: <stable at vger.kernel.org>
Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
(backported from commit 82b1c07a0af603e3c47b906c8e991dc96f01688e)
[koichiroden: Adopted the correctly modified v3 patch:
 https://lore.kernel.org/all/20240311084426.447164-1-ying.huang@intel.com/
 Also adjusted context due to missing commits:
 commit 33e16272fe98 ("mm/swapfile.c: __swap_entry_free() always free 1 entry")
 commit c48981eeb0d5 ("include/linux/compiler.h: Introduce data_race(expr) macro")]
CVE-2024-26960
Signed-off-by: Koichiro Den <koichiro.den at canonical.com>
---
 mm/swapfile.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 5e444be061a8..9787a4155d2f 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1752,13 +1752,19 @@ int free_swap_and_cache(swp_entry_t entry)
 	if (non_swap_entry(entry))
 		return 1;
 
-	p = _swap_info_get(entry);
+	p = get_swap_device(entry);
 	if (p) {
+		if (WARN_ON(!p->swap_map[swp_offset(entry)])) {
+			put_swap_device(p);
+			return 0;
+		}
+
 		count = __swap_entry_free(p, entry, 1);
 		if (count == SWAP_HAS_CACHE &&
 		    !swap_page_trans_huge_swapped(p, entry))
 			__try_to_reclaim_swap(p, swp_offset(entry),
 					      TTRS_UNMAPPED | TTRS_FULL);
+		put_swap_device(p);
 	}
 	return p != NULL;
 }
-- 
2.43.0




More information about the kernel-team mailing list