| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 11 Mar 2024 16:44:26 +0800
From: Huang Ying <ying.huang@...el.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-mm@...ck.org,
linux-kernel@...r.kernel.org,
Ryan Roberts <ryan.roberts@....com>,
"Huang, Ying" <ying.huang@...el.com>,
David Hildenbrand <david@...hat.com>,
stable@...r.kernel.org
Subject: [PATCH -v3] mm: swap: fix race between free_swap_and_cache() and swapoff()
From: Ryan Roberts <ryan.roberts@....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
Closes: https://lore.kernel.org/linux-mm/8734t27awd.fsf@yhuang6-desk2.ccr.corp.intel.com/
Signed-off-by: Ryan Roberts <ryan.roberts@....com>
Signed-off-by: "Huang, Ying" <ying.huang@...el.com> [patch description]
Cc: David Hildenbrand <david@...hat.com>
Cc: <stable@...r.kernel.org>
Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
---
Hi, Andrew,
If it's not too late. Please replace v2 of this patch in mm-stable
with this version.
Changes since v2:
- Remove comments for get_swap_device() because it's not correct.
- Revised patch description about the race condition description.
Changes since v1:
- Added comments for get_swap_device() as suggested by David
- Moved check that swap entry is not free from get_swap_device() to
free_swap_and_cache() since there are some paths that legitimately call with
a free offset.
Best Regards,
Huang, Ying
mm/swapfile.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 2b3a2d85e350..9e0691276f5e 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1609,13 +1609,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(data_race(!p->swap_map[swp_offset(entry)]))) {
+ put_swap_device(p);
+ return 0;
+ }
+
count = __swap_entry_free(p, entry);
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.39.2
Powered by blists - more mailing lists