Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] mm/memfd: fix hugetlb reservation accounting in error paths
@ 2026-09-03  3:01 Hongfu Li
  2026-09-03 20:21 ` Andrew Morton
  0 siblings, 1 reply; 2+ messages in thread
From: Hongfu Li @ 2026-09-03  3:01 UTC (permalink / raw)
  To: hughd, baolin.wang, akpm, vivek.kasireddy
  Cc: muchun.song, osalvador, david, linux-mm, linux-kernel, hongfu.li,
	Hongfu Li

From: Hongfu Li <lihongfu@kylinos.cn>

If hugetlb_add_to_page_cache() in memfd_alloc_folio() fails with
-EEXIST, a concurrent fault has already instantiated the folio in the
page cache, and the reservation now belongs to that folio. Calling
hugetlb_unreserve_pages() in that case incorrectly removes the region
backing the cached folio. A later truncate or inode eviction then passes
a negative (chg - freed) into hugepage_subpool_put_pages(), corrupting
subpool and resv_huge_pages accounting.

Hold the hugetlb fault mutex from hugetlb_reserve_pages() until the
error-path unreserve completes to make the reserve, allocate and
instantiate steps atomic against concurrent faults.  With the mutex held
from the start, a concurrent fault can no longer consume the reservation
between reserve and allocate/instantiate. If a fault completed before the
mutex was taken, it has already added the region for that index, so
hugetlb_reserve_pages() returns 0 and the error path leaves the region
in place.

Fixes: 717cf9357325 ("mm/memfd: reserve hugetlb folios before allocation")
Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
---
v2:
- Take the hugetlb fault mutex before hugetlb_reserve_pages() and hold
  it until the error-path unreserve completes.
- Update commit message
- Link to v1: https://lore.kernel.org/all/20260831090631.29227-1-hongfu.li@linux.dev/ 
---
 mm/memfd.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/mm/memfd.c b/mm/memfd.c
index c708d92533f4..0f6fff004f5e 100644
--- a/mm/memfd.c
+++ b/mm/memfd.c
@@ -82,22 +82,31 @@ struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx)
 		struct hstate *h = hstate_file(memfd);
 		int err = -ENOMEM;
 		long nr_resv;
+		u32 hash;
 
 		gfp_mask = htlb_alloc_mask(h);
 		gfp_mask &= ~(__GFP_HIGHMEM | __GFP_MOVABLE);
 		idx >>= huge_page_order(h);
 
+		/*
+		 * Serialize hugepage allocation and instantiation to prevent
+		 * races with concurrent allocations, as required by all other
+		 * callers of hugetlb_add_to_page_cache().
+		 */
+		hash = hugetlb_fault_mutex_hash(memfd->f_mapping, idx);
+		mutex_lock(&hugetlb_fault_mutex_table[hash]);
+
 		nr_resv = hugetlb_reserve_pages(inode, idx, idx + 1, NULL, EMPTY_VMA_FLAGS);
-		if (nr_resv < 0)
-			return ERR_PTR(nr_resv);
+		if (nr_resv < 0) {
+			err = nr_resv;
+			goto out_unlock;
+		}
 
 		folio = alloc_hugetlb_folio_reserve(h,
 						    numa_node_id(),
 						    NULL,
 						    gfp_mask);
 		if (folio) {
-			u32 hash;
-
 			/*
 			 * Zero the folio to prevent information leaks to userspace.
 			 * Use folio_zero_user() which is optimized for huge/gigantic
@@ -112,20 +121,9 @@ struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx)
 			 */
 			__folio_mark_uptodate(folio);
 
-			/*
-			 * Serialize hugepage allocation and instantiation to prevent
-			 * races with concurrent allocations, as required by all other
-			 * callers of hugetlb_add_to_page_cache().
-			 */
-			hash = hugetlb_fault_mutex_hash(memfd->f_mapping, idx);
-			mutex_lock(&hugetlb_fault_mutex_table[hash]);
-
 			err = hugetlb_add_to_page_cache(folio,
 							memfd->f_mapping,
 							idx);
-
-			mutex_unlock(&hugetlb_fault_mutex_table[hash]);
-
 			if (err) {
 				folio_put(folio);
 				goto err_unresv;
@@ -133,11 +131,14 @@ struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx)
 
 			hugetlb_set_folio_subpool(folio, subpool_inode(inode));
 			folio_unlock(folio);
+			mutex_unlock(&hugetlb_fault_mutex_table[hash]);
 			return folio;
 		}
 err_unresv:
 		if (nr_resv > 0)
 			hugetlb_unreserve_pages(inode, idx, idx + 1, 0);
+out_unlock:
+		mutex_unlock(&hugetlb_fault_mutex_table[hash]);
 		return ERR_PTR(err);
 	}
 #endif
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] mm/memfd: fix hugetlb reservation accounting in error paths
  2026-09-03  3:01 [PATCH v2] mm/memfd: fix hugetlb reservation accounting in error paths Hongfu Li
@ 2026-09-03 20:21 ` Andrew Morton
  0 siblings, 0 replies; 2+ messages in thread
From: Andrew Morton @ 2026-09-03 20:21 UTC (permalink / raw)
  To: Hongfu Li
  Cc: hughd, baolin.wang, vivek.kasireddy, muchun.song, osalvador,
	david, linux-mm, linux-kernel, Hongfu Li

On Thu,  3 Sep 2026 11:01:34 +0800 Hongfu Li <hongfu.li@linux.dev> wrote:

> From: Hongfu Li <lihongfu@kylinos.cn>
> 
> If hugetlb_add_to_page_cache() in memfd_alloc_folio() fails with
> -EEXIST, a concurrent fault has already instantiated the folio in the
> page cache, and the reservation now belongs to that folio. Calling
> hugetlb_unreserve_pages() in that case incorrectly removes the region
> backing the cached folio. A later truncate or inode eviction then passes
> a negative (chg - freed) into hugepage_subpool_put_pages(), corrupting
> subpool and resv_huge_pages accounting.

Ho hum.

I've asked so many times "what are the userspace-visible runtime
effects of this bug".  Nowadays I often just ask Gemini instead.  It
told me:


  Over time, these corrupted counters would leak huge page reservations. 
  Applications using hugetlb memfds would eventually find themselves
  unable to allocate huge pages, receiving unexpected ENOMEM errors even
  though system memory and pool capacities appeared free and healthy.

and

  The corrupted accounting caused hugepage_subpool_put_pages() to
  receive a negative value during a later file truncation or inode
  eviction.

  While this typically manifests as kernel logs (WARN traces or
  badness flags regarding subpool page counts), it could cause
  misbehaved resource tracking that impacts subsequent system
  operations, unmounts, or process teardowns interacting with that
  hugetlb file descriptor.

All of which sounds rather unpleasant, so I suggest a cc:stable here.


To help people understand why we propose a backport and to help others
understand the impact the fix will have upon their system, I'll paste
the above into the changelog.  Please send any necessary corrections.


Please also update your prompts (if using them) to ensure that the
changelogging includes this info in the future.


I'll queue it for testing and shall await maintainer review.


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-03 20:22 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-03  3:01 [PATCH v2] mm/memfd: fix hugetlb reservation accounting in error paths Hongfu Li
2026-09-03 20:21 ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox