[PATCH] mm/huge_memory: fix deferred split list corruption on partial unmap

[Mani Tofigh via B4 Relay]

From: Mani Tofigh <mt4022@xxxxxxxxxxxx>

When a THP is partially unmapped while the deferred split shrinker is
scanning it, the deferred split list can end up corrupted.  With
CONFIG_DEBUG_LIST, freeing the folio later reports:

  [  523.871401] list_del corruption. prev->next should be ffd40000005e0090, but was ffd4000000500090. (prev=ffd40000005e0090)
  [  523.871412] WARNING: lib/list_debug.c:62 at __list_del_entry_valid_or_report+0x177/0x220, CPU#0: thpsplit/63
  ...
  [  523.900715] Call Trace:
  [  523.901753]  __list_lru_del+0x5f/0x220
  [  523.902588]  __folio_unqueue_deferred_split+0x238/0x520
  ...

Without CONFIG_DEBUG_LIST, the list is left pointing at the folio
after it has been freed.

Since commit fafaeceb89a5 ("mm: switch deferred split shrinker to
list_lru"), deferred_split_scan() moves the folios it scans onto a
private list under the list_lru lock, drops the lock, and then removes
each folio from that list with list_del_init(), which sets
entry->next = entry and then entry->prev = entry.  Another task may
in the meantime partially unmap the same folio and call
deferred_split_folio(), which queues it only if list_empty() reports
it as not queued, and list_empty() reads only entry->next.  If that
happens between the two stores, the folio is queued again, and the
store to entry->prev that follows breaks the new link:

  Task A (shrinker)                     Task B (partial unmap)
  -----------------                     ----------------------
  list_del_init(entry)
    entry->next = entry
                                        deferred_split_folio()
                                          list_empty(entry) is true
                                          list_add_tail(entry)
    entry->prev = entry
    /* overwrites the prev link that
       list_add_tail() just set */

Use list_del_init_careful() instead.  It sets entry->prev first and
then entry->next with smp_store_release(), so once list_empty() sees
the entry as empty, the shrinker has finished writing to it.

The reproducer repeatedly faults in THPs and partially unmaps them
while the deferred split shrinker, run through
/proc/sys/vm/drop_caches on the same CPU, is scanning them.  This bug
was found and replayed using a deterministic hypervisor currently
under development, but the non-deterministic C reproducer can
typically trigger it in the 10-500s time range.  With the fix, nothing
is triggered after 1800 seconds.

Fixes: fafaeceb89a5 ("mm: switch deferred split shrinker to list_lru")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Mani Tofigh <mt4022@xxxxxxxxxxxx>
---
Reproducer: https://github.com/manitofigh/thpsplit-repro/blob/main/thpsplit.c
---
 mm/huge_memory.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 1e5d68acf62a..f4f7d757a310 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -4668,7 +4668,11 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
 		bool did_split = false;
 		bool underused = false;
 
-		list_del_init(&folio->_deferred_list);
+		/*
+		 * deferred_split_folio() may queue the folio again as soon as
+		 * list_empty() in __list_lru_add() sees it empty.
+		 */
+		list_del_init_careful(&folio->_deferred_list);
 
 		if (!folio_test_partially_mapped(folio)) {
 			/*

---
base-commit: e9011a65d78258901ba123c5f522bef26d29902f
change-id: 20261026-thpsplit-careful-0d24c335281e

Best regards,
--  
Mani Tofigh <mt4022@xxxxxxxxxxxx>