Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: dayou5941@163.com
To: akpm@linux-foundation.org, linmiaohe@huawei.com,
	david@kernel.org, ljs@kernel.org
Cc: nao.horiguchi@gmail.com, linux-mm@kvack.org, ziy@nvidia.com,
	Li Youhong <liyouhong@kylinos.cn>,
	Sashiko <sashiko-bot@kernel.org>,
	stable@vger.kernel.org
Subject: [PATCH v4] mm/memory-failure: fix folio refcount leak and locking in soft/hard offline
Date: Thu,  6 Aug 2026 11:19:58 +0800	[thread overview]
Message-ID: <20260806031958.677935-1-dayou5941@163.com> (raw)

From: Li Youhong <liyouhong@kylinos.cn>

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 <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260803060001.800638-1-dayou5941@163.com
Cc: stable@vger.kernel.org
Co-developed-by: David Hildenbrand <david@kernel.org>
Signed-off-by: David Hildenbrand <david@kernel.org>
Signed-off-by: Li Youhong <liyouhong@kylinos.cn>
---
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/20260804035828.2684059-1-dayou5941@163.com/

v3:
- Fix author name and Signed-off-by format as requested by Greg.
  v2: https://lore.kernel.org/all/20260805072625.2437636-1-dayou5941@163.com/

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/20260804035356.2615408-1-dayou5941@163.com/
- 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/20260805084224.2597547-1-dayou5941@163.com/

---
 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



             reply	other threads:[~2026-08-06  3:20 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-06  3:19 dayou5941 [this message]
2026-08-06  4:34 ` [PATCH v4] mm/memory-failure: fix folio refcount leak and locking in soft/hard offline Andrew Morton
2026-08-06 10:50   ` David Hildenbrand (Arm)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260806031958.677935-1-dayou5941@163.com \
    --to=dayou5941@163.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=linmiaohe@huawei.com \
    --cc=linux-mm@kvack.org \
    --cc=liyouhong@kylinos.cn \
    --cc=ljs@kernel.org \
    --cc=nao.horiguchi@gmail.com \
    --cc=sashiko-bot@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox