[PATCH v4] mm/memory-failure: fix folio refcount leak and locking in soft/hard offline

From: Li Youhong <[email protected]>

The min_order_for_split() function accesses folio->mapping without proper
synchronization. In memory_failure(), the folio lock is dropped before the
call, and in soft_offline_in_use_page(), the lock is not held at all. This
means that while min_order_for_split() is executing, the value of
folio->mapping may be modified by a truncate or invalidate operation,
leading to a torn read or use of a stale mapping value.

Additionally, the soft_offline_in_use_page() path fails to release the
folio reference taken by get_hwpoison_page() when new_order != 0, causing a
reference leak.

Fix these issues by:
- Moving the split operation logic into the callers and holding the folio
  lock around min_order_for_split() and split_huge_page_to_order().
- Removing the try_to_split_thp_page() helper to simplify refcount
  handling.
- Ensuring the folio reference is always dropped before returning on the
  soft-offline error path.

This refactors the code to be more maintainable, fixes the locking issue
reported by Sashiko, and also addresses the folio reference leak on the
soft-offline error path.

Fixes: 689b8986776c ("mm/memory-failure: improve large block size folio handling")
Reported-by: Sashiko <[email protected]>
Closes: https://sashiko.dev/#/patchset/[email protected]
Cc: [email protected]
Co-developed-by: David Hildenbrand <[email protected]>
Signed-off-by: David Hildenbrand <[email protected]>
Signed-off-by: Li Youhong <[email protected]>
---
v2:
- Dropped the approach of caching folio->mapping inside min_order_for_split() in favor of adding the folio lock at the callers.
- Added VM_WARN_ON_ONCE_FOLIO() in min_order_for_split().
- Updated the commit message to clarify that the real issue is the callers not holding the folio lock, rather than a TOCTOU race.
  v1: https://lore.kernel.org/all/[email protected]/

v3:
- Fix author name and Signed-off-by format as requested by Greg.
  v2: https://lore.kernel.org/all/[email protected]/

v4:
- Rebased on David Hildenbrand's refactoring patch.
- Added Co-developed-by for David Hildenbrand.
- Supersedes the standalone refcount fix:
  link: https://lore.kernel.org/all/[email protected]/
- The VM_WARN_ON_ONCE_FOLIO() check in min_order_for_split() will be sent
  as a separate follow-up patch.
  v3: https://lore.kernel.org/all/[email protected]/

---
 mm/memory-failure.c | 49 +++++++++++++++++----------------------------
 1 file changed, 18 insertions(+), 31 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 51508a55c405..665f5b444fb7 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1649,26 +1649,6 @@ static int identify_page_state(unsigned long pfn, struct page *p,
 	return page_action(ps, p, pfn);
 }
 
-/*
- * When 'release' is 'false', it means that if thp split has failed,
- * there is still more to do, hence the page refcount we took earlier
- * is still needed.
- */
-static int try_to_split_thp_page(struct page *page, unsigned int new_order,
-		bool release)
-{
-	int ret;
-
-	lock_page(page);
-	ret = split_huge_page_to_order(page, new_order);
-	unlock_page(page);
-
-	if (ret && release)
-		put_page(page);
-
-	return ret;
-}
-
 static void unmap_and_kill(struct list_head *to_kill, unsigned long pfn,
 		struct address_space *mapping, pgoff_t index, int flags)
 {
@@ -2440,7 +2420,6 @@ int memory_failure(unsigned long pfn, int flags)
 	folio_unlock(folio);
 
 	if (folio_test_large(folio)) {
-		const int new_order = min_order_for_split(folio);
 		int err;
 
 		/*
@@ -2457,24 +2436,24 @@ int memory_failure(unsigned long pfn, int flags)
 		 * page is a valid handlable page.
 		 */
 		folio_set_has_hwpoisoned(folio);
-		err = try_to_split_thp_page(p, new_order, /* release= */ false);
+
+		lock_page(p);
+		err = split_huge_page_to_order(p, min_order_for_split(folio));
+		unlock_page(p);
 		/*
 		 * If splitting a folio to order-0 fails, kill the process.
 		 * Split the folio regardless to minimize unusable pages.
 		 * Because the memory failure code cannot handle large
 		 * folios, this split is always treated as if it failed.
 		 */
-		if (err || new_order) {
-			/* get folio again in case the original one is split */
-			folio = page_folio(p);
+		folio = page_folio(p);
+		if (folio_test_large(folio)) {
 			res = -EHWPOISON;
 			kill_procs_now(p, pfn, flags, folio);
 			put_page(p);
 			action_result(pfn, MF_MSG_UNSPLIT_THP, MF_FAILED);
 			goto unlock_mutex;
 		}
-		VM_BUG_ON_PAGE(!page_count(p), p);
-		folio = page_folio(p);
 	}
 
 	/*
@@ -2776,6 +2755,9 @@ EXPORT_SYMBOL(unpoison_memory);
  * soft_offline_in_use_page handles hugetlb-pages and non-hugetlb pages.
  * If the page is a non-dirty unmapped page-cache page, it simply invalidates.
  * If the page is mapped, it migrates the contents over.
+ *
+ * The folio refcount has been incremented before entering this function.
+ * This folio reference must be released before the function returns on all paths.
  */
 static int soft_offline_in_use_page(struct page *page)
 {
@@ -2793,8 +2775,7 @@ static int soft_offline_in_use_page(struct page *page)
 	};
 
 	if (!huge && folio_test_large(folio)) {
-		const int new_order = min_order_for_split(folio);
-
+		lock_page(page);
 		/*
 		 * If new_order (target split order) is not 0, do not split the
 		 * folio at all to retain the still accessible large folio.
@@ -2802,8 +2783,14 @@ static int soft_offline_in_use_page(struct page *page)
 		 * preferred, split it to non-zero new_order like it is done in
 		 * memory_failure().
 		 */
-		if (new_order || try_to_split_thp_page(page, /* new_order= */ 0,
-						       /* release= */ true)) {
+		if (!min_order_for_split(folio))
+			ret = split_huge_page_to_order(page, 0);
+		else
+			ret = -EBUSY;
+		unlock_page(page);
+
+		if (ret) {
+			put_page(page);
 			pr_info("%#lx: thp split failed\n", pfn);
 			return -EBUSY;
 		}
-- 
2.25.1